DocumentSharedObjectPool prevents sharing of ElementData in case of h…

…ash collision

https://bugs.webkit.org/show_bug.cgi?id=254114

Reviewed by Darin Adler.

Use a HashSet of RefPtr<ShareableElementData> for the cache, with proper traits
and hashing so that it does what we expect. This means we can let the HashSet
implementation deal with hash collision and we can still share ElementData in
the event of such collision.

The previous code was using a HashMap whose key was the hash and thus couldn't
deal with hash collision and would just not share in this case.

This is performance neutral on Speedometer on both iPhone and Mac.

* Source/WebCore/dom/DocumentSharedObjectPool.cpp:
(WebCore::DocumentSharedObjectPool::ShareableElementDataHash::hash):
(WebCore::DocumentSharedObjectPool::ShareableElementDataHash::equal):
(WebCore::AttributeSpanTranslator::hash):
(WebCore::AttributeSpanTranslator::equal):
(WebCore::AttributeSpanTranslator::translate):
(WebCore::DocumentSharedObjectPool::cachedShareableElementDataWithAttributes):
(WebCore::hasSameAttributes): Deleted.
* Source/WebCore/dom/DocumentSharedObjectPool.h:

Canonical link: https://commits.webkit.org/261893@main

Source/WebCore/dom/DocumentSharedObjectPool.cpp

@@ -59,27 +59,44 @@ ALWAYS_INLINE bool equalAttributes(const uint8_t* a, const uint8_t* b, unsigned
 }
 #endif
 
-inline bool hasSameAttributes(Span<const Attribute> attributes, ShareableElementData& elementData)
-{
-    if (attributes.size() != elementData.length())
-        return false;
-    return equalAttributes(reinterpret_cast<const uint8_t*>(attributes.data()), reinterpret_cast<const uint8_t*>(elementData.m_attributeArray), attributes.size() * sizeof(Attribute));
-}
+struct DocumentSharedObjectPool::ShareableElementDataHash {
+    static unsigned hash(const Ref<ShareableElementData>& data)
+    {
+        return computeHash(Span<const Attribute> { data->m_attributeArray, data->length() });
+    }
+    static bool equal(const Ref<ShareableElementData>& a, const Ref<ShareableElementData>& b)
+    {
+        if (a->length() != b->length())
+            return false;
+        return equalAttributes(reinterpret_cast<const uint8_t*>(a->m_attributeArray), reinterpret_cast<const uint8_t*>(b->m_attributeArray), a->length() * sizeof(Attribute));
+    }
+    static constexpr bool safeToCompareToEmptyOrDeleted = false;
+};
 
-Ref<ShareableElementData> DocumentSharedObjectPool::cachedShareableElementDataWithAttributes(Span<const Attribute> attributes)
-{
-    ASSERT(!attributes.empty());
+struct AttributeSpanTranslator {
+    static unsigned hash(Span<const Attribute> attributes)
+    {
+        return computeHash(attributes);
+    }
 
-    auto& cachedData = m_shareableElementDataCache.add(computeHash(attributes), nullptr).iterator->value;
+    static bool equal(const Ref<ShareableElementData>& a, Span<const Attribute> b)
+    {
+        if (a->length() != b.size())
+            return false;
+        return equalAttributes(reinterpret_cast<const uint8_t*>(a->m_attributeArray), reinterpret_cast<const uint8_t*>(b.data()), b.size() * sizeof(Attribute));
+    }
 
-    // FIXME: This prevents sharing when there's a hash collision.
-    if (cachedData && !hasSameAttributes(attributes, *cachedData))
-        return ShareableElementData::createWithAttributes(attributes);
+    static void translate(Ref<ShareableElementData>& location, Span<const Attribute> attributes, unsigned /*hash*/)
+    {
+        location = ShareableElementData::createWithAttributes(attributes);
+    }
+};
 
-    if (!cachedData)
-        cachedData = ShareableElementData::createWithAttributes(attributes);
+Ref<ShareableElementData> DocumentSharedObjectPool::cachedShareableElementDataWithAttributes(Span<const Attribute> attributes)
+{
+    ASSERT(!attributes.empty());
 
-    return *cachedData;
+    return m_shareableElementDataCache.add<AttributeSpanTranslator>(attributes).iterator->get();
 }
 
-}
+} // namespace WebCore

Source/WebCore/dom/DocumentSharedObjectPool.h

@@ -42,7 +42,8 @@ class DocumentSharedObjectPool {
     Ref<ShareableElementData> cachedShareableElementDataWithAttributes(Span<const Attribute>);
 
 private:
-    typedef HashMap<unsigned, RefPtr<ShareableElementData>, AlreadyHashed> ShareableElementDataCache;
+    struct ShareableElementDataHash;
+    using ShareableElementDataCache = HashSet<Ref<ShareableElementData>, ShareableElementDataHash>;
     ShareableElementDataCache m_shareableElementDataCache;
 };