Reduce URL size
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 11 Jul 2017 17:02:14 +0000 (17:02 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 11 Jul 2017 17:02:14 +0000 (17:02 +0000)
https://bugs.webkit.org/show_bug.cgi?id=174319

Patch by Alex Christensen <achristensen@webkit.org> on 2017-07-11
Reviewed by Andreas Kling.

m_fragmentEnd is redundant information. If a URL is valid, then it is always m_string.length().
If a URL is not valid, then it is always 0. Rather than storing additional information,
deduce the fragment end from the validity of the URL and the String's length.

No change in behavior.  This reduces sizeof(URL) from 56 to 48 and reduces operations when parsing.

* platform/URL.cpp:
(WebCore::URL::invalidate):
(WebCore::URL::fragmentIdentifier):
(WebCore::URL::hasFragmentIdentifier):
(WebCore::URL::removeFragmentIdentifier):
* platform/URL.h:
(WebCore::URL::encode):
(WebCore::URL::decode):
(WebCore::URL::hasFragment):
* platform/URLParser.cpp:
(WebCore::URLParser::urlLengthUntilPart):
(WebCore::URLParser::copyURLPartsUntil):
(WebCore::URLParser::parse):
(WebCore::URLParser::allValuesEqual):
(WebCore::URLParser::internalValuesConsistent):

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

Source/WebCore/ChangeLog
Source/WebCore/platform/URL.cpp
Source/WebCore/platform/URL.h
Source/WebCore/platform/URLParser.cpp

index 50049f0..752780b 100644 (file)
@@ -1,5 +1,34 @@
 2017-07-11  Alex Christensen  <achristensen@webkit.org>
 
 2017-07-11  Alex Christensen  <achristensen@webkit.org>
 
+        Reduce URL size
+        https://bugs.webkit.org/show_bug.cgi?id=174319
+
+        Reviewed by Andreas Kling.
+
+        m_fragmentEnd is redundant information. If a URL is valid, then it is always m_string.length().
+        If a URL is not valid, then it is always 0. Rather than storing additional information,
+        deduce the fragment end from the validity of the URL and the String's length.
+
+        No change in behavior.  This reduces sizeof(URL) from 56 to 48 and reduces operations when parsing.
+
+        * platform/URL.cpp:
+        (WebCore::URL::invalidate):
+        (WebCore::URL::fragmentIdentifier):
+        (WebCore::URL::hasFragmentIdentifier):
+        (WebCore::URL::removeFragmentIdentifier):
+        * platform/URL.h:
+        (WebCore::URL::encode):
+        (WebCore::URL::decode):
+        (WebCore::URL::hasFragment):
+        * platform/URLParser.cpp:
+        (WebCore::URLParser::urlLengthUntilPart):
+        (WebCore::URLParser::copyURLPartsUntil):
+        (WebCore::URLParser::parse):
+        (WebCore::URLParser::allValuesEqual):
+        (WebCore::URLParser::internalValuesConsistent):
+
+2017-07-11  Alex Christensen  <achristensen@webkit.org>
+
         SharedBuffer::size should return a size_t
         https://bugs.webkit.org/show_bug.cgi?id=174328
 
         SharedBuffer::size should return a size_t
         https://bugs.webkit.org/show_bug.cgi?id=174328
 
index 19db453..1ef5555 100644 (file)
@@ -355,7 +355,6 @@ void URL::invalidate()
     m_pathEnd = 0;
     m_pathAfterLastSlash = 0;
     m_queryEnd = 0;
     m_pathEnd = 0;
     m_pathAfterLastSlash = 0;
     m_queryEnd = 0;
-    m_fragmentEnd = 0;
 }
 
 URL::URL(ParsedURLStringTag, const String& url)
 }
 
 URL::URL(ParsedURLStringTag, const String& url)
@@ -493,15 +492,15 @@ String URL::encodedPass() const
 
 String URL::fragmentIdentifier() const
 {
 
 String URL::fragmentIdentifier() const
 {
-    if (m_fragmentEnd == m_queryEnd)
+    if (!m_isValid || m_queryEnd == m_string.length())
         return String();
 
         return String();
 
-    return m_string.substring(m_queryEnd + 1, m_fragmentEnd - (m_queryEnd + 1));
+    return m_string.substring(m_queryEnd + 1);
 }
 
 bool URL::hasFragmentIdentifier() const
 {
 }
 
 bool URL::hasFragmentIdentifier() const
 {
-    return m_fragmentEnd != m_queryEnd;
+    return m_isValid && m_string.length() != m_queryEnd;
 }
 
 String URL::baseAsString() const
 }
 
 String URL::baseAsString() const
@@ -864,13 +863,11 @@ void URL::setFragmentIdentifier(StringView identifier)
 void URL::removeFragmentIdentifier()
 {
     if (!m_isValid) {
 void URL::removeFragmentIdentifier()
 {
     if (!m_isValid) {
-        ASSERT(!m_fragmentEnd);
         ASSERT(!m_queryEnd);
         return;
     }
         ASSERT(!m_queryEnd);
         return;
     }
-    if (m_fragmentEnd > m_queryEnd)
+    if (m_isValid && m_string.length() > m_queryEnd)
         m_string = m_string.left(m_queryEnd);
         m_string = m_string.left(m_queryEnd);
-    m_fragmentEnd = m_queryEnd;
 }
     
 void URL::setQuery(const String& query)
 }
     
 void URL::setQuery(const String& query)
index 68d6285..37f050f 100644 (file)
@@ -231,7 +231,6 @@ private:
     unsigned m_pathAfterLastSlash;
     unsigned m_pathEnd;
     unsigned m_queryEnd;
     unsigned m_pathAfterLastSlash;
     unsigned m_pathEnd;
     unsigned m_queryEnd;
-    unsigned m_fragmentEnd;
 };
 
 template <class Encoder>
 };
 
 template <class Encoder>
@@ -251,7 +250,6 @@ void URL::encode(Encoder& encoder) const
     encoder << m_pathAfterLastSlash;
     encoder << m_pathEnd;
     encoder << m_queryEnd;
     encoder << m_pathAfterLastSlash;
     encoder << m_pathEnd;
     encoder << m_queryEnd;
-    encoder << m_fragmentEnd;
 }
 
 template <class Decoder>
 }
 
 template <class Decoder>
@@ -287,8 +285,6 @@ bool URL::decode(Decoder& decoder, URL& url)
         return false;
     if (!decoder.decode(url.m_queryEnd))
         return false;
         return false;
     if (!decoder.decode(url.m_queryEnd))
         return false;
-    if (!decoder.decode(url.m_fragmentEnd))
-        return false;
     return true;
 }
 
     return true;
 }
 
@@ -412,7 +408,7 @@ inline bool URL::hasQuery() const
 
 inline bool URL::hasFragment() const
 {
 
 inline bool URL::hasFragment() const
 {
-    return m_fragmentEnd > m_queryEnd;
+    return m_isValid && m_string.length() > m_queryEnd;
 }
 
 inline bool URL::protocolIsInHTTPFamily() const
 }
 
 inline bool URL::protocolIsInHTTPFamily() const
index 1e3d465..472ba76 100644 (file)
@@ -829,14 +829,11 @@ enum class URLParser::URLPart {
     PathAfterLastSlash,
     PathEnd,
     QueryEnd,
     PathAfterLastSlash,
     PathEnd,
     QueryEnd,
-    FragmentEnd,
 };
 
 size_t URLParser::urlLengthUntilPart(const URL& url, URLPart part)
 {
     switch (part) {
 };
 
 size_t URLParser::urlLengthUntilPart(const URL& url, URLPart part)
 {
     switch (part) {
-    case URLPart::FragmentEnd:
-        return url.m_fragmentEnd;
     case URLPart::QueryEnd:
         return url.m_queryEnd;
     case URLPart::PathEnd:
     case URLPart::QueryEnd:
         return url.m_queryEnd;
     case URLPart::PathEnd:
@@ -886,8 +883,6 @@ void URLParser::copyURLPartsUntil(const URL& base, URLPart part, const CodePoint
     m_asciiBuffer.clear();
     copyASCIIStringUntil(base.m_string, urlLengthUntilPart(base, part));
     switch (part) {
     m_asciiBuffer.clear();
     copyASCIIStringUntil(base.m_string, urlLengthUntilPart(base, part));
     switch (part) {
-    case URLPart::FragmentEnd:
-        RELEASE_ASSERT_NOT_REACHED();
     case URLPart::QueryEnd:
         m_url.m_queryEnd = base.m_queryEnd;
         FALLTHROUGH;
     case URLPart::QueryEnd:
         m_url.m_queryEnd = base.m_queryEnd;
         FALLTHROUGH;
@@ -1861,7 +1856,6 @@ void URLParser::parse(const CharacterType* input, const unsigned length, const U
     case State::SpecialRelativeOrAuthority:
         LOG_FINAL_STATE("SpecialRelativeOrAuthority");
         copyURLPartsUntil(base, URLPart::QueryEnd, c, isUTF8Encoding);
     case State::SpecialRelativeOrAuthority:
         LOG_FINAL_STATE("SpecialRelativeOrAuthority");
         copyURLPartsUntil(base, URLPart::QueryEnd, c, isUTF8Encoding);
-        m_url.m_fragmentEnd = m_url.m_queryEnd;
         break;
     case State::PathOrAuthority:
         LOG_FINAL_STATE("PathOrAuthority");
         break;
     case State::PathOrAuthority:
         LOG_FINAL_STATE("PathOrAuthority");
@@ -1876,7 +1870,6 @@ void URLParser::parse(const CharacterType* input, const unsigned length, const U
         m_url.m_pathAfterLastSlash = m_url.m_userStart + 1;
         m_url.m_pathEnd = m_url.m_pathAfterLastSlash;
         m_url.m_queryEnd = m_url.m_pathAfterLastSlash;
         m_url.m_pathAfterLastSlash = m_url.m_userStart + 1;
         m_url.m_pathEnd = m_url.m_pathAfterLastSlash;
         m_url.m_queryEnd = m_url.m_pathAfterLastSlash;
-        m_url.m_fragmentEnd = m_url.m_pathAfterLastSlash;
         break;
     case State::Relative:
         LOG_FINAL_STATE("Relative");
         break;
     case State::Relative:
         LOG_FINAL_STATE("Relative");
@@ -1888,7 +1881,6 @@ void URLParser::parse(const CharacterType* input, const unsigned length, const U
         m_url.m_pathAfterLastSlash = base.m_portEnd + 1;
         m_url.m_pathEnd = m_url.m_pathAfterLastSlash;
         m_url.m_queryEnd = m_url.m_pathAfterLastSlash;
         m_url.m_pathAfterLastSlash = base.m_portEnd + 1;
         m_url.m_pathEnd = m_url.m_pathAfterLastSlash;
         m_url.m_queryEnd = m_url.m_pathAfterLastSlash;
-        m_url.m_fragmentEnd = m_url.m_pathAfterLastSlash;
         break;
     case State::SpecialAuthoritySlashes:
         LOG_FINAL_STATE("SpecialAuthoritySlashes");
         break;
     case State::SpecialAuthoritySlashes:
         LOG_FINAL_STATE("SpecialAuthoritySlashes");
@@ -1900,7 +1892,6 @@ void URLParser::parse(const CharacterType* input, const unsigned length, const U
         m_url.m_pathAfterLastSlash = m_url.m_userStart;
         m_url.m_pathEnd = m_url.m_userStart;
         m_url.m_queryEnd = m_url.m_userStart;
         m_url.m_pathAfterLastSlash = m_url.m_userStart;
         m_url.m_pathEnd = m_url.m_userStart;
         m_url.m_queryEnd = m_url.m_userStart;
-        m_url.m_fragmentEnd = m_url.m_userStart;
         break;
     case State::SpecialAuthorityIgnoreSlashes:
         LOG_FINAL_STATE("SpecialAuthorityIgnoreSlashes");
         break;
     case State::SpecialAuthorityIgnoreSlashes:
         LOG_FINAL_STATE("SpecialAuthorityIgnoreSlashes");
@@ -1929,7 +1920,6 @@ void URLParser::parse(const CharacterType* input, const unsigned length, const U
         }
         m_url.m_pathAfterLastSlash = m_url.m_pathEnd;
         m_url.m_queryEnd = m_url.m_pathEnd;
         }
         m_url.m_pathAfterLastSlash = m_url.m_pathEnd;
         m_url.m_queryEnd = m_url.m_pathEnd;
-        m_url.m_fragmentEnd = m_url.m_pathEnd;
         break;
     case State::Host:
         LOG_FINAL_STATE("Host");
         break;
     case State::Host:
         LOG_FINAL_STATE("Host");
@@ -1945,13 +1935,11 @@ void URLParser::parse(const CharacterType* input, const unsigned length, const U
             m_url.m_pathEnd = m_url.m_portEnd;
         m_url.m_pathAfterLastSlash = m_url.m_pathEnd;
         m_url.m_queryEnd = m_url.m_pathEnd;
             m_url.m_pathEnd = m_url.m_portEnd;
         m_url.m_pathAfterLastSlash = m_url.m_pathEnd;
         m_url.m_queryEnd = m_url.m_pathEnd;
-        m_url.m_fragmentEnd = m_url.m_pathEnd;
         break;
     case State::File:
         LOG_FINAL_STATE("File");
         if (base.isValid() && base.protocolIs("file")) {
             copyURLPartsUntil(base, URLPart::QueryEnd, c, isUTF8Encoding);
         break;
     case State::File:
         LOG_FINAL_STATE("File");
         if (base.isValid() && base.protocolIs("file")) {
             copyURLPartsUntil(base, URLPart::QueryEnd, c, isUTF8Encoding);
-            m_url.m_fragmentEnd = m_url.m_queryEnd;
             break;
         }
         syntaxViolation(c);
             break;
         }
         syntaxViolation(c);
@@ -1964,7 +1952,6 @@ void URLParser::parse(const CharacterType* input, const unsigned length, const U
         m_url.m_pathAfterLastSlash = m_url.m_userStart + 1;
         m_url.m_pathEnd = m_url.m_pathAfterLastSlash;
         m_url.m_queryEnd = m_url.m_pathAfterLastSlash;
         m_url.m_pathAfterLastSlash = m_url.m_userStart + 1;
         m_url.m_pathEnd = m_url.m_pathAfterLastSlash;
         m_url.m_queryEnd = m_url.m_pathAfterLastSlash;
-        m_url.m_fragmentEnd = m_url.m_pathAfterLastSlash;
         break;
     case State::FileSlash:
         LOG_FINAL_STATE("FileSlash");
         break;
     case State::FileSlash:
         LOG_FINAL_STATE("FileSlash");
@@ -1982,7 +1969,6 @@ void URLParser::parse(const CharacterType* input, const unsigned length, const U
             m_url.m_pathAfterLastSlash = m_url.m_userStart + 1;
         m_url.m_pathEnd = m_url.m_pathAfterLastSlash;
         m_url.m_queryEnd = m_url.m_pathAfterLastSlash;
             m_url.m_pathAfterLastSlash = m_url.m_userStart + 1;
         m_url.m_pathEnd = m_url.m_pathAfterLastSlash;
         m_url.m_queryEnd = m_url.m_pathAfterLastSlash;
-        m_url.m_fragmentEnd = m_url.m_pathAfterLastSlash;
         break;
     case State::FileHost:
         LOG_FINAL_STATE("FileHost");
         break;
     case State::FileHost:
         LOG_FINAL_STATE("FileHost");
@@ -1994,7 +1980,6 @@ void URLParser::parse(const CharacterType* input, const unsigned length, const U
             m_url.m_pathAfterLastSlash = currentPosition(c);
             m_url.m_pathEnd = m_url.m_pathAfterLastSlash;
             m_url.m_queryEnd = m_url.m_pathAfterLastSlash;
             m_url.m_pathAfterLastSlash = currentPosition(c);
             m_url.m_pathEnd = m_url.m_pathAfterLastSlash;
             m_url.m_queryEnd = m_url.m_pathAfterLastSlash;
-            m_url.m_fragmentEnd = m_url.m_pathAfterLastSlash;
             break;
         }
         
             break;
         }
         
@@ -2009,7 +1994,6 @@ void URLParser::parse(const CharacterType* input, const unsigned length, const U
             m_url.m_pathAfterLastSlash = m_url.m_userStart + 1;
             m_url.m_pathEnd = m_url.m_pathAfterLastSlash;
             m_url.m_queryEnd = m_url.m_pathAfterLastSlash;
             m_url.m_pathAfterLastSlash = m_url.m_userStart + 1;
             m_url.m_pathEnd = m_url.m_pathAfterLastSlash;
             m_url.m_queryEnd = m_url.m_pathAfterLastSlash;
-            m_url.m_fragmentEnd = m_url.m_pathAfterLastSlash;
             break;
         }
 
             break;
         }
 
@@ -2028,7 +2012,6 @@ void URLParser::parse(const CharacterType* input, const unsigned length, const U
         m_url.m_pathAfterLastSlash = m_url.m_portEnd + 1;
         m_url.m_pathEnd = m_url.m_pathAfterLastSlash;
         m_url.m_queryEnd = m_url.m_pathAfterLastSlash;
         m_url.m_pathAfterLastSlash = m_url.m_portEnd + 1;
         m_url.m_pathEnd = m_url.m_pathAfterLastSlash;
         m_url.m_queryEnd = m_url.m_pathAfterLastSlash;
-        m_url.m_fragmentEnd = m_url.m_pathAfterLastSlash;
         break;
     case State::PathStart:
         LOG_FINAL_STATE("PathStart");
         break;
     case State::PathStart:
         LOG_FINAL_STATE("PathStart");
@@ -2037,30 +2020,25 @@ void URLParser::parse(const CharacterType* input, const unsigned length, const U
         LOG_FINAL_STATE("Path");
         m_url.m_pathEnd = currentPosition(c);
         m_url.m_queryEnd = m_url.m_pathEnd;
         LOG_FINAL_STATE("Path");
         m_url.m_pathEnd = currentPosition(c);
         m_url.m_queryEnd = m_url.m_pathEnd;
-        m_url.m_fragmentEnd = m_url.m_pathEnd;
         break;
     case State::CannotBeABaseURLPath:
         LOG_FINAL_STATE("CannotBeABaseURLPath");
         m_url.m_pathEnd = currentPosition(c);
         m_url.m_queryEnd = m_url.m_pathEnd;
         break;
     case State::CannotBeABaseURLPath:
         LOG_FINAL_STATE("CannotBeABaseURLPath");
         m_url.m_pathEnd = currentPosition(c);
         m_url.m_queryEnd = m_url.m_pathEnd;
-        m_url.m_fragmentEnd = m_url.m_pathEnd;
         break;
     case State::UTF8Query:
         LOG_FINAL_STATE("UTF8Query");
         ASSERT(queryBegin == CodePointIterator<CharacterType>());
         m_url.m_queryEnd = currentPosition(c);
         break;
     case State::UTF8Query:
         LOG_FINAL_STATE("UTF8Query");
         ASSERT(queryBegin == CodePointIterator<CharacterType>());
         m_url.m_queryEnd = currentPosition(c);
-        m_url.m_fragmentEnd = m_url.m_queryEnd;
         break;
     case State::NonUTF8Query:
         LOG_FINAL_STATE("NonUTF8Query");
         ASSERT(queryBegin != CodePointIterator<CharacterType>());
         encodeQuery(queryBuffer, encoding, CodePointIterator<CharacterType>(queryBegin, c));
         m_url.m_queryEnd = currentPosition(c);
         break;
     case State::NonUTF8Query:
         LOG_FINAL_STATE("NonUTF8Query");
         ASSERT(queryBegin != CodePointIterator<CharacterType>());
         encodeQuery(queryBuffer, encoding, CodePointIterator<CharacterType>(queryBegin, c));
         m_url.m_queryEnd = currentPosition(c);
-        m_url.m_fragmentEnd = m_url.m_queryEnd;
         break;
     case State::Fragment:
         LOG_FINAL_STATE("Fragment");
         break;
     case State::Fragment:
         LOG_FINAL_STATE("Fragment");
-        m_url.m_fragmentEnd = currentPosition(c);
         break;
     }
 
         break;
     }
 
@@ -2887,7 +2865,7 @@ bool URLParser::allValuesEqual(const URL& a, const URL& b)
 {
     // FIXME: m_cannotBeABaseURL is not compared because the old URL::parse did not use it,
     // but once we get rid of URL::parse its value should be tested.
 {
     // FIXME: m_cannotBeABaseURL is not compared because the old URL::parse did not use it,
     // but once we get rid of URL::parse its value should be tested.
-    URL_PARSER_LOG("%d %d %d %d %d %d %d %d %d %d %d %d %s\n%d %d %d %d %d %d %d %d %d %d %d %d %s",
+    URL_PARSER_LOG("%d %d %d %d %d %d %d %d %d %d %d %s\n%d %d %d %d %d %d %d %d %d %d %d %s",
         a.m_isValid,
         a.m_protocolIsInHTTPFamily,
         a.m_schemeEnd,
         a.m_isValid,
         a.m_protocolIsInHTTPFamily,
         a.m_schemeEnd,
@@ -2899,7 +2877,6 @@ bool URLParser::allValuesEqual(const URL& a, const URL& b)
         a.m_pathAfterLastSlash,
         a.m_pathEnd,
         a.m_queryEnd,
         a.m_pathAfterLastSlash,
         a.m_pathEnd,
         a.m_queryEnd,
-        a.m_fragmentEnd,
         a.m_string.utf8().data(),
         b.m_isValid,
         b.m_protocolIsInHTTPFamily,
         a.m_string.utf8().data(),
         b.m_isValid,
         b.m_protocolIsInHTTPFamily,
@@ -2912,7 +2889,6 @@ bool URLParser::allValuesEqual(const URL& a, const URL& b)
         b.m_pathAfterLastSlash,
         b.m_pathEnd,
         b.m_queryEnd,
         b.m_pathAfterLastSlash,
         b.m_pathEnd,
         b.m_queryEnd,
-        b.m_fragmentEnd,
         b.m_string.utf8().data());
 
     return a.m_string == b.m_string
         b.m_string.utf8().data());
 
     return a.m_string == b.m_string
@@ -2926,8 +2902,7 @@ bool URLParser::allValuesEqual(const URL& a, const URL& b)
         && a.m_portEnd == b.m_portEnd
         && a.m_pathAfterLastSlash == b.m_pathAfterLastSlash
         && a.m_pathEnd == b.m_pathEnd
         && a.m_portEnd == b.m_portEnd
         && a.m_pathAfterLastSlash == b.m_pathAfterLastSlash
         && a.m_pathEnd == b.m_pathEnd
-        && a.m_queryEnd == b.m_queryEnd
-        && a.m_fragmentEnd == b.m_fragmentEnd;
+        && a.m_queryEnd == b.m_queryEnd;
 }
 
 bool URLParser::internalValuesConsistent(const URL& url)
 }
 
 bool URLParser::internalValuesConsistent(const URL& url)
@@ -2940,10 +2915,7 @@ bool URLParser::internalValuesConsistent(const URL& url)
         && url.m_portEnd <= url.m_pathAfterLastSlash
         && url.m_pathAfterLastSlash <= url.m_pathEnd
         && url.m_pathEnd <= url.m_queryEnd
         && url.m_portEnd <= url.m_pathAfterLastSlash
         && url.m_pathAfterLastSlash <= url.m_pathEnd
         && url.m_pathEnd <= url.m_queryEnd
-        && url.m_queryEnd <= url.m_fragmentEnd
-        && (url.m_isValid ? url.m_fragmentEnd == url.m_string.length() : !url.m_fragmentEnd);
-    // FIXME: Why do we even store m_fragmentEnd?
-    // It should be able to be deduced from m_isValid and m_string.length() to save memory.
+        && url.m_queryEnd <= url.m_string.length();
 }
 
 } // namespace WebCore
 }
 
 } // namespace WebCore