Title: [292310] trunk/Source/WebCore
Revision
292310
Author
cdu...@apple.com
Date
2022-04-04 12:25:09 -0700 (Mon, 04 Apr 2022)

Log Message

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):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (292309 => 292310)


--- trunk/Source/WebCore/ChangeLog	2022-04-04 19:06:56 UTC (rev 292309)
+++ trunk/Source/WebCore/ChangeLog	2022-04-04 19:25:09 UTC (rev 292310)
@@ -1,3 +1,30 @@
+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-04  Tim Nguyen  <n...@apple.com>
 
         Conditionally inject <attachment> styles based on runtime flag

Modified: trunk/Source/WebCore/html/parser/HTMLConstructionSite.cpp (292309 => 292310)


--- trunk/Source/WebCore/html/parser/HTMLConstructionSite.cpp	2022-04-04 19:06:56 UTC (rev 292309)
+++ trunk/Source/WebCore/html/parser/HTMLConstructionSite.cpp	2022-04-04 19:25:09 UTC (rev 292310)
@@ -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: trunk/Source/WebCore/html/parser/HTMLConstructionSite.h (292309 => 292310)


--- trunk/Source/WebCore/html/parser/HTMLConstructionSite.h	2022-04-04 19:06:56 UTC (rev 292309)
+++ trunk/Source/WebCore/html/parser/HTMLConstructionSite.h	2022-04-04 19:25:09 UTC (rev 292310)
@@ -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