Optimize URLParser performance
authorachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 12 Sep 2016 18:06:59 +0000 (18:06 +0000)
committerachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 12 Sep 2016 18:06:59 +0000 (18:06 +0000)
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.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@205812 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/platform/URLParser.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp

index b5da379..03f2456 100644 (file)
@@ -1,3 +1,20 @@
+2016-09-09  Alex Christensen  <achristensen@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.fraser@apple.com>
 
         Make -webkit-transition-* and -webkit-animation-* properties be pure aliases of the unprefixed ones
index e2dc2e1..68f4faa 100644 (file)
@@ -127,27 +127,118 @@ static void encodeQuery(const StringBuilder& source, StringBuilder& destination,
     }
 }
 
-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 @@ bool URLParser::parsePort(StringView::CodePoints::Iterator& iterator, const Stri
             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
index da450f1..055a8f7 100644 (file)
@@ -1,3 +1,14 @@
+2016-09-10  Alex Christensen  <achristensen@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  <cdumez@apple.com>
 
         parseHTMLInteger() should take a StringView in parameter
index e9903b0..ed33344 100644 (file)
@@ -466,6 +466,7 @@ TEST_F(URLParserTest, ParserDifferences)
 
 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 @@ TEST_F(URLParserTest, DefaultPort)
         {"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 @@ TEST_F(URLParserTest, DefaultPort)
         {"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 @@ TEST_F(URLParserTest, DefaultPort)
         {"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 @@ TEST_F(URLParserTest, DefaultPort)
         {"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 @@ TEST_F(URLParserTest, DefaultPort)
         {"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",