Changeset 156902 in webkit

Timestamp:

Oct 4, 2013, 11:41:09 AM (12 years ago)

Author:

ap@apple.com

Message:

Optimize strings copies in srcset parser
​https://bugs.webkit.org/show_bug.cgi?id=121899

Patch by Romain Perier <Romain Perier> on 2013-10-04
Reviewed by Alexey Proskuryakov.

No new tests, covered by existing ones.

• html/parser/HTMLParserIdioms.cpp:

(WebCore::parseImagesWithScaleFromSrcsetAttribute): Don't copy
image.imageURL at each loop iteration, save indexes instead.
(WebCore::bestFitSourceForImageAttributes): Make a String for
the URL only when the corresponding candidate is chosen
by the selection algorithm. It reduces the number of copies
significantly and improves performance
(around 30% with the "Release" profile and 60% with the "Debug" one).

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• html/parser/HTMLParserIdioms.cpp (modified) (4 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-10-04  Romain Perier  <romain.perier@gmail.com>
+
+        Optimize strings copies in srcset parser
+        https://bugs.webkit.org/show_bug.cgi?id=121899
+
+        Reviewed by Alexey Proskuryakov.
+
+        No new tests, covered by existing ones.
+
+        * html/parser/HTMLParserIdioms.cpp:
+        (WebCore::parseImagesWithScaleFromSrcsetAttribute): Don't copy
+        image.imageURL at each loop iteration, save indexes instead.
+        (WebCore::bestFitSourceForImageAttributes): Make a String for
+        the URL only when the corresponding candidate is chosen
+        by the selection algorithm. It reduces the number of copies
+        significantly and improves performance
+        (around 30% with the "Release" profile and 60% with the "Debug" one).
+
 2013-10-04  Alexey Proskuryakov  <ap@apple.com>
 

trunk/Source/WebCore/html/parser/HTMLParserIdioms.cpp

@@ -301 +301 @@
 
 struct ImageWithScale {
-    String imageURL;
+    unsigned imageURLStart;
+    unsigned imageURLLength;
     float scaleFactor;
-    bool operator==(const ImageWithScale& image) const
+
+    ImageWithScale()
+        : imageURLStart(0)
+        , imageURLLength(0)
+        , scaleFactor(1)
+    { 
+    }
+
+    bool hasImageURL() const
     {
-        return scaleFactor == image.scaleFactor && imageURL == image.imageURL;
+        return imageURLLength;
     }
 };
@@ -391 +400 @@
         }
         ImageWithScale image;
-        image.imageURL = String(srcsetAttribute.characters() + imageURLStart, imageURLEnd - imageURLStart);
+        image.imageURLStart = imageURLStart;
+        image.imageURLLength = imageURLEnd - imageURLStart;
         image.scaleFactor = imageScaleFactor;
 
@@ -407 +417 @@
 
     if (!srcAttribute.isEmpty()) {
-        ImageWithScale image;
-        image.imageURL = srcAttribute;
-        image.scaleFactor = 1.0;
-
-        imageCandidates.append(image);
+        ImageWithScale srcPlaceholderImage;
+        imageCandidates.append(srcPlaceholderImage);
     }
 
@@ -421 +428 @@
     for (size_t i = 0; i < imageCandidates.size() - 1; ++i) {
         if (imageCandidates[i].scaleFactor >= deviceScaleFactor)
-            return String(imageCandidates[i].imageURL);
-    }
-    return String(imageCandidates.last().imageURL);
-}
-
-}
+            return imageCandidates[i].hasImageURL() ? srcsetAttribute.substringSharingImpl(imageCandidates[i].imageURLStart, imageCandidates[i].imageURLLength) : srcAttribute;
+    }
+    const ImageWithScale& lastCandidate = imageCandidates.last();
+    return lastCandidate.hasImageURL() ? srcsetAttribute.substringSharingImpl(lastCandidate.imageURLStart, lastCandidate.imageURLLength) : srcAttribute;
+}
+
+}