Title: [205812] trunk
Revision
205812
Author
achristen...@apple.com
Date
2016-09-12 11:06:59 -0700 (Mon, 12 Sep 2016)

Log Message

Optimize URLParser performance
https://bugs.webkit.org/show_bug.cgi?id=161837

Reviewed by Brady Eidson.

Source/WebCore:

No change in behavior.  Existing behavior covered by API tests and added a new API test.

* platform/URLParser.cpp:
(WebCore::isDefaultPort):
Use switch statements instead of HashMap lookups.
(WebCore::isSpecialScheme):
Use switch statements instead of repeated String comparisons.
(WebCore::URLParser::parsePort):
Reduce String allocation.

Tools:

* TestWebKitAPI/Tests/WebCore/URLParser.cpp:
(TestWebKitAPI::TEST_F):
Added a test to verify the case insensitivity of the default port checks.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (205811 => 205812)


--- trunk/Source/WebCore/ChangeLog	2016-09-12 16:52:33 UTC (rev 205811)
+++ trunk/Source/WebCore/ChangeLog	2016-09-12 18:06:59 UTC (rev 205812)
@@ -1,3 +1,20 @@
+2016-09-09  Alex Christensen  <achristen...@webkit.org>
+
+        Optimize URLParser performance
+        https://bugs.webkit.org/show_bug.cgi?id=161837
+
+        Reviewed by Brady Eidson.
+
+        No change in behavior.  Existing behavior covered by API tests and added a new API test.
+
+        * platform/URLParser.cpp:
+        (WebCore::isDefaultPort):
+        Use switch statements instead of HashMap lookups.
+        (WebCore::isSpecialScheme):
+        Use switch statements instead of repeated String comparisons.
+        (WebCore::URLParser::parsePort):
+        Reduce String allocation.
+
 2016-09-12  Simon Fraser  <simon.fra...@apple.com>
 
         Make -webkit-transition-* and -webkit-animation-* properties be pure aliases of the unprefixed ones

Modified: trunk/Source/WebCore/platform/URLParser.cpp (205811 => 205812)


--- trunk/Source/WebCore/platform/URLParser.cpp	2016-09-12 16:52:33 UTC (rev 205811)
+++ trunk/Source/WebCore/platform/URLParser.cpp	2016-09-12 18:06:59 UTC (rev 205812)
@@ -127,27 +127,118 @@
     }
 }
 
-static bool isDefaultPort(const String& scheme, uint16_t port)
+static bool isDefaultPort(StringView scheme, uint16_t port)
 {
-    static NeverDestroyed<HashMap<String, uint16_t>> defaultPorts(HashMap<String, uint16_t>({
-        {"ftp", 21},
-        {"gopher", 70},
-        {"http", 80},
-        {"https", 443},
-        {"ws", 80},
-        {"wss", 443}}));
-    return defaultPorts.get().get(scheme) == port;
+    static const uint16_t ftpPort = 21;
+    static const uint16_t gopherPort = 70;
+    static const uint16_t httpPort = 80;
+    static const uint16_t httpsPort = 443;
+    static const uint16_t wsPort = 80;
+    static const uint16_t wssPort = 443;
+    
+    auto length = scheme.length();
+    if (!length)
+        return false;
+    switch (scheme[0]) {
+    case 'w':
+        switch (length) {
+        case 2:
+            return scheme[1] == 's'
+                && port == wsPort;
+        case 3:
+            return scheme[1] == 's'
+                && scheme[2] == 's'
+                && port == wssPort;
+        default:
+            return false;
+        }
+    case 'h':
+        switch (length) {
+        case 4:
+            return scheme[1] == 't'
+                && scheme[2] == 't'
+                && scheme[3] == 'p'
+                && port == httpPort;
+        case 5:
+            return scheme[1] == 't'
+                && scheme[2] == 't'
+                && scheme[3] == 'p'
+                && scheme[4] == 's'
+                && port == httpsPort;
+        default:
+            return false;
+        }
+    case 'g':
+        return length == 6
+            && scheme[1] == 'o'
+            && scheme[2] == 'p'
+            && scheme[3] == 'h'
+            && scheme[4] == 'e'
+            && scheme[5] == 'r'
+            && port == gopherPort;
+    case 'f':
+        return length == 3
+            && scheme[1] == 't'
+            && scheme[2] == 'p'
+            && port == ftpPort;
+        return false;
+    default:
+        return false;
+    }
 }
 
 static bool isSpecialScheme(StringView scheme)
 {
-    return scheme == "ftp"
-        || scheme == "file"
-        || scheme == "gopher"
-        || scheme == "http"
-        || scheme == "https"
-        || scheme == "ws"
-        || scheme == "wss";
+    auto length = scheme.length();
+    if (!length)
+        return false;
+    switch (scheme[0]) {
+    case 'f':
+        switch (length) {
+        case 3:
+            return scheme[1] == 't'
+                && scheme[2] == 'p';
+        case 4:
+            return scheme[1] == 'i'
+                && scheme[2] == 'l'
+                && scheme[3] == 'e';
+        default:
+            return false;
+        }
+    case 'g':
+        return length == 6
+            && scheme[1] == 'o'
+            && scheme[2] == 'p'
+            && scheme[3] == 'h'
+            && scheme[4] == 'e'
+            && scheme[5] == 'r';
+    case 'h':
+        switch (length) {
+        case 4:
+            return scheme[1] == 't'
+                && scheme[2] == 't'
+                && scheme[3] == 'p';
+        case 5:
+            return scheme[1] == 't'
+                && scheme[2] == 't'
+                && scheme[3] == 'p'
+                && scheme[4] == 's';
+        default:
+            return false;
+        }
+    case 'w':
+        switch (length) {
+        case 2:
+            return scheme[1] == 's';
+        case 3:
+            return scheme[1] == 's'
+                && scheme[2] == 's';
+        default:
+            return false;
+        }
+    default:
+        return false;
+    }
 }
 
 static StringView bufferView(const StringBuilder& builder, unsigned start, unsigned length)
@@ -1394,9 +1485,7 @@
             return false;
     }
     
-    // FIXME: This shouldn't need a String allocation.
-    String scheme = m_buffer.toStringPreserveCapacity().substring(0, m_url.m_schemeEnd);
-    if (isDefaultPort(scheme, port)) {
+    if (isDefaultPort(bufferView(m_buffer, 0, m_url.m_schemeEnd), port)) {
         ASSERT(m_buffer[m_buffer.length() - 1] == ':');
         m_buffer.resize(m_buffer.length() - 1);
     } else

Modified: trunk/Tools/ChangeLog (205811 => 205812)


--- trunk/Tools/ChangeLog	2016-09-12 16:52:33 UTC (rev 205811)
+++ trunk/Tools/ChangeLog	2016-09-12 18:06:59 UTC (rev 205812)
@@ -1,3 +1,14 @@
+2016-09-10  Alex Christensen  <achristen...@webkit.org>
+
+        Optimize URLParser performance
+        https://bugs.webkit.org/show_bug.cgi?id=161837
+
+        Reviewed by Brady Eidson.
+
+        * TestWebKitAPI/Tests/WebCore/URLParser.cpp:
+        (TestWebKitAPI::TEST_F):
+        Added a test to verify the case insensitivity of the default port checks.
+
 2016-09-10  Chris Dumez  <cdu...@apple.com>
 
         parseHTMLInteger() should take a StringView in parameter

Modified: trunk/Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp (205811 => 205812)


--- trunk/Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp	2016-09-12 16:52:33 UTC (rev 205811)
+++ trunk/Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp	2016-09-12 18:06:59 UTC (rev 205812)
@@ -466,6 +466,7 @@
 
 TEST_F(URLParserTest, DefaultPort)
 {
+    checkURL("FtP://host:21/", {"ftp", "", "", "host", 0, "/", "", "", "ftp://host/"});
     checkURL("ftp://host:21/", {"ftp", "", "", "host", 0, "/", "", "", "ftp://host/"});
     checkURL("ftp://host:22/", {"ftp", "", "", "host", 22, "/", "", "", "ftp://host:22/"});
     checkURLDifferences("ftp://host:21",
@@ -475,6 +476,7 @@
         {"ftp", "", "", "host", 22, "/", "", "", "ftp://host:22/"},
         {"ftp", "", "", "host", 22, "", "", "", "ftp://host:22"});
     
+    checkURL("gOpHeR://host:70/", {"gopher", "", "", "host", 0, "/", "", "", "gopher://host/"});
     checkURL("gopher://host:70/", {"gopher", "", "", "host", 0, "/", "", "", "gopher://host/"});
     checkURL("gopher://host:71/", {"gopher", "", "", "host", 71, "/", "", "", "gopher://host:71/"});
     // Spec, Chrome, Firefox, and URLParser have "/", URL::parse does not.
@@ -486,16 +488,19 @@
         {"gopher", "", "", "host", 71, "/", "", "", "gopher://host:71/"},
         {"gopher", "", "", "host", 71, "", "", "", "gopher://host:71"});
     
+    checkURL("hTtP://host:80", {"http", "", "", "host", 0, "/", "", "", "http://host/"});
     checkURL("http://host:80", {"http", "", "", "host", 0, "/", "", "", "http://host/"});
     checkURL("http://host:80/", {"http", "", "", "host", 0, "/", "", "", "http://host/"});
     checkURL("http://host:81", {"http", "", "", "host", 81, "/", "", "", "http://host:81/"});
     checkURL("http://host:81/", {"http", "", "", "host", 81, "/", "", "", "http://host:81/"});
     
+    checkURL("hTtPs://host:443", {"https", "", "", "host", 0, "/", "", "", "https://host/"});
     checkURL("https://host:443", {"https", "", "", "host", 0, "/", "", "", "https://host/"});
     checkURL("https://host:443/", {"https", "", "", "host", 0, "/", "", "", "https://host/"});
     checkURL("https://host:444", {"https", "", "", "host", 444, "/", "", "", "https://host:444/"});
     checkURL("https://host:444/", {"https", "", "", "host", 444, "/", "", "", "https://host:444/"});
     
+    checkURL("wS://host:80/", {"ws", "", "", "host", 0, "/", "", "", "ws://host/"});
     checkURL("ws://host:80/", {"ws", "", "", "host", 0, "/", "", "", "ws://host/"});
     checkURL("ws://host:81/", {"ws", "", "", "host", 81, "/", "", "", "ws://host:81/"});
     // URLParser matches Chrome and Firefox, but not URL::parse
@@ -506,6 +511,7 @@
         {"ws", "", "", "host", 81, "/", "", "", "ws://host:81/"},
         {"ws", "", "", "host", 81, "", "", "", "ws://host:81"});
     
+    checkURL("WsS://host:443/", {"wss", "", "", "host", 0, "/", "", "", "wss://host/"});
     checkURL("wss://host:443/", {"wss", "", "", "host", 0, "/", "", "", "wss://host/"});
     checkURL("wss://host:444/", {"wss", "", "", "host", 444, "/", "", "", "wss://host:444/"});
     // URLParser matches Chrome and Firefox, but not URL::parse
@@ -517,6 +523,7 @@
         {"wss", "", "", "host", 444, "", "", "", "wss://host:444"});
 
     // 990 is the default ftps port in URL::parse, but it's not in the URL spec. Maybe it should be.
+    checkURL("fTpS://host:990/", {"ftps", "", "", "host", 990, "/", "", "", "ftps://host:990/"});
     checkURL("ftps://host:990/", {"ftps", "", "", "host", 990, "/", "", "", "ftps://host:990/"});
     checkURL("ftps://host:991/", {"ftps", "", "", "host", 991, "/", "", "", "ftps://host:991/"});
     checkURLDifferences("ftps://host:990",
@@ -526,6 +533,7 @@
         {"ftps", "", "", "host", 991, "/", "", "", "ftps://host:991/"},
         {"ftps", "", "", "host", 991, "", "", "", "ftps://host:991"});
 
+    checkURL("uNkNoWn://host:80/", {"unknown", "", "", "host", 80, "/", "", "", "unknown://host:80/"});
     checkURL("unknown://host:80/", {"unknown", "", "", "host", 80, "/", "", "", "unknown://host:80/"});
     checkURL("unknown://host:81/", {"unknown", "", "", "host", 81, "/", "", "", "unknown://host:81/"});
     checkURLDifferences("unknown://host:80",
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to