Title: [292985] branches/safari-613-branch/Source/WebCore
Revision
292985
Author
alanc...@apple.com
Date
2022-04-18 17:49:23 -0700 (Mon, 18 Apr 2022)

Log Message

Cherry-pick r292310. rdar://problem/91255370

    Simplify / Optimize the whitespace cache implementation
    https://bugs.webkit.org/show_bug.cgi?id=238736

    Reviewed by Sam Weinig.

    Instead of using 2 C arrays of size maximumCachedStringLength + 1 Vector with an inline
    buffer of size maximumCachedStringLength, we now used a single FixedVector of size
    maximumCachedStringLength.

    Because the Vector has an inline buffer whose size is the max size of the cache, using
    a FixedVector is just more efficient. It also means we don't need to store indexes in
    that Vector in a separate C array. Finally, I used a struct named AtomStringWithCode to
    store { AtomString, uint64 code } so we don't need separate containers for the AtomString
    and the code.

    Note that I added VectorTraits for the new AtomStringWithCode struct to make sure it can
    get initialized via a simple memset.

    This is a 0.25-0.3% progression on Speedometer according to A/B bots.

    * html/parser/HTMLConstructionSite.cpp:
    (WebCore::WhitespaceCache::lookup):
    * html/parser/HTMLConstructionSite.h:
    (WebCore::WhitespaceCache::WhitespaceCache):

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

Modified Paths

Diff

Modified: branches/safari-613-branch/Source/WebCore/ChangeLog (292984 => 292985)


--- branches/safari-613-branch/Source/WebCore/ChangeLog	2022-04-19 00:49:20 UTC (rev 292984)
+++ branches/safari-613-branch/Source/WebCore/ChangeLog	2022-04-19 00:49:23 UTC (rev 292985)
@@ -1,5 +1,64 @@
 2022-04-18  Kocsen Chung  <kocsen_ch...@apple.com>
 
+        Cherry-pick r292310. rdar://problem/91255370
+
+    Simplify / Optimize the whitespace cache implementation
+    https://bugs.webkit.org/show_bug.cgi?id=238736
+    
+    Reviewed by Sam Weinig.
+    
+    Instead of using 2 C arrays of size maximumCachedStringLength + 1 Vector with an inline
+    buffer of size maximumCachedStringLength, we now used a single FixedVector of size
+    maximumCachedStringLength.
+    
+    Because the Vector has an inline buffer whose size is the max size of the cache, using
+    a FixedVector is just more efficient. It also means we don't need to store indexes in
+    that Vector in a separate C array. Finally, I used a struct named AtomStringWithCode to
+    store { AtomString, uint64 code } so we don't need separate containers for the AtomString
+    and the code.
+    
+    Note that I added VectorTraits for the new AtomStringWithCode struct to make sure it can
+    get initialized via a simple memset.
+    
+    This is a 0.25-0.3% progression on Speedometer according to A/B bots.
+    
+    * html/parser/HTMLConstructionSite.cpp:
+    (WebCore::WhitespaceCache::lookup):
+    * html/parser/HTMLConstructionSite.h:
+    (WebCore::WhitespaceCache::WhitespaceCache):
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@292310 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2022-04-04  Chris Dumez  <cdu...@apple.com>
+
+            Simplify / Optimize the whitespace cache implementation
+            https://bugs.webkit.org/show_bug.cgi?id=238736
+
+            Reviewed by Sam Weinig.
+
+            Instead of using 2 C arrays of size maximumCachedStringLength + 1 Vector with an inline
+            buffer of size maximumCachedStringLength, we now used a single FixedVector of size
+            maximumCachedStringLength.
+
+            Because the Vector has an inline buffer whose size is the max size of the cache, using
+            a FixedVector is just more efficient. It also means we don't need to store indexes in
+            that Vector in a separate C array. Finally, I used a struct named AtomStringWithCode to
+            store { AtomString, uint64 code } so we don't need separate containers for the AtomString
+            and the code.
+
+            Note that I added VectorTraits for the new AtomStringWithCode struct to make sure it can
+            get initialized via a simple memset.
+
+            This is a 0.25-0.3% progression on Speedometer according to A/B bots.
+
+            * html/parser/HTMLConstructionSite.cpp:
+            (WebCore::WhitespaceCache::lookup):
+            * html/parser/HTMLConstructionSite.h:
+            (WebCore::WhitespaceCache::WhitespaceCache):
+
+2022-04-18  Kocsen Chung  <kocsen_ch...@apple.com>
+
         Cherry-pick r291852. rdar://problem/83235846
 
     Improve rebuilding of ruby subtrees

Modified: branches/safari-613-branch/Source/WebCore/html/parser/HTMLConstructionSite.cpp (292984 => 292985)


--- branches/safari-613-branch/Source/WebCore/html/parser/HTMLConstructionSite.cpp	2022-04-19 00:49:20 UTC (rev 292984)
+++ branches/safari-613-branch/Source/WebCore/html/parser/HTMLConstructionSite.cpp	2022-04-19 00:49:23 UTC (rev 292985)
@@ -887,35 +887,26 @@
         return whitespaceMode == AllWhitespace || isAllWhitespace(string) ? AtomString(string) : AtomString();
 
     uint64_t code;
-    if (whitespaceMode == AllWhitespace)
+    if (whitespaceMode == AllWhitespace) {
         code = codeForString<AllWhitespace>(string);
-    else
+        ASSERT(code);
+    } else {
         code = codeForString<WhitespaceUnknown>(string);
+        if (!code)
+            return AtomString();
+    }
 
-    if (!code)
-        return AtomString();
-
-    size_t lengthIndex = length - 1;
-    if (m_codes[lengthIndex] == code) {
-        ASSERT(m_atoms[m_indexes[lengthIndex]] == string);
-        return m_atoms[m_indexes[lengthIndex]];
+    auto& existingAtom = m_atoms[length - 1];
+    if (existingAtom.code == code) {
+        ASSERT(existingAtom.string == string);
+        return existingAtom.string;
     }
 
     if (code == overflowWhitespaceCode)
         return AtomString(string);
 
-    if (m_codes[lengthIndex]) {
-        AtomString whitespaceAtom(string);
-        m_codes[lengthIndex] = code;
-        m_atoms[m_indexes[lengthIndex]] = whitespaceAtom;
-        return whitespaceAtom;
-    }
-
-    AtomString whitespaceAtom(string);
-    m_codes[lengthIndex] = code;
-    m_indexes[lengthIndex] = m_atoms.size();
-    m_atoms.append(whitespaceAtom);
-    return whitespaceAtom;
+    existingAtom = { AtomString { string }, code };
+    return existingAtom.string;
 }
 
 }

Modified: branches/safari-613-branch/Source/WebCore/html/parser/HTMLConstructionSite.h (292984 => 292985)


--- branches/safari-613-branch/Source/WebCore/html/parser/HTMLConstructionSite.h	2022-04-19 00:49:20 UTC (rev 292984)
+++ branches/safari-613-branch/Source/WebCore/html/parser/HTMLConstructionSite.h	2022-04-19 00:49:23 UTC (rev 292985)
@@ -30,6 +30,7 @@
 #include "FragmentScriptingPermission.h"
 #include "HTMLElementStack.h"
 #include "HTMLFormattingElementList.h"
+#include <wtf/FixedVector.h>
 #include <wtf/Noncopyable.h>
 #include <wtf/RefPtr.h>
 #include <wtf/SetForScope.h>
@@ -37,6 +38,17 @@
 
 namespace WebCore {
 
+struct AtomStringWithCode {
+    AtomString string;
+    uint64_t code { 0 };
+};
+}
+
+namespace WTF {
+template<> struct VectorTraits<WebCore::AtomStringWithCode> : SimpleClassVectorTraits { };
+}
+
+namespace WebCore {
 struct HTMLConstructionSiteTask {
     enum Operation {
         Insert,
@@ -228,7 +240,10 @@
 class WhitespaceCache {
     WTF_MAKE_FAST_ALLOCATED;
 public:
-    WhitespaceCache() = default;
+    WhitespaceCache()
+        : m_atoms(maximumCachedStringLength)
+    {
+    }
 
     AtomString lookup(const String&, WhitespaceMode);
 
@@ -238,14 +253,7 @@
     constexpr static uint64_t overflowWhitespaceCode = static_cast<uint64_t>(-1);
     constexpr static size_t maximumCachedStringLength = 128;
 
-    // Parallel arrays storing a 64 bit code and an index into m_atoms for the
-    // most recently atomized whitespace-only string of a given length. The
-    // indices into these two arrays are the string length minus 1, so the code
-    // for a whitespace-only string of length 2 is stored at m_codes[1], etc.
-    uint64_t m_codes[maximumCachedStringLength] { 0 };
-    uint8_t m_indexes[maximumCachedStringLength] { 0 };
-
-    Vector<AtomString, maximumCachedStringLength> m_atoms;
+    FixedVector<AtomStringWithCode> m_atoms;
 };
 
 } // namespace WebCore
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to