Avoid CompactPtr expansion if unnecessary

https://bugs.webkit.org/show_bug.cgi?id=249572 rdar://103506006 Reviewed by Justin Michaud. This patch adds operator== etc. for CompactPtr and adjust HashTraits for CompactPtr so that we can avoid pointer expansion while using CompactPtr in HashTable: comparison of CompactPtrs and hash computation don't need pointer expansion. * Source/WTF/wtf/CompactPtr.h: (WTF::CompactPtr::operator==): (WTF::CompactPtr::operator!=): (WTF::CompactPtr::storage const): (WTF::operator==): (WTF::operator!=): (WTF::DefaultHash<CompactPtr<P>>::hash): (WTF::DefaultHash<CompactPtr<P>>::equal): * Source/WTF/wtf/HashTraits.h: (WTF::HashTraits<CompactPtr<P>>::peek): * Source/WTF/wtf/Packed.h: (WTF::operator==): (WTF::operator!=): * Source/WTF/wtf/RefPtr.h: (WTF::operator==): (WTF::operator!=): * Source/WTF/wtf/text/AtomStringImpl.cpp: (WTF::AtomStringTableRemovalHashTranslator::equal): Canonical link: https://commits.webkit.org/258271@main
WebKit · Dec 23, 2022 · 36ff06c · 36ff06c
1 parent 95e3bdf
commit 36ff06c
Show file tree

Hide file tree

Showing 5 changed files with 102 additions and 16 deletions.
diff --git a/Source/WTF/wtf/CompactPtr.h b/Source/WTF/wtf/CompactPtr.h
@@ -201,6 +201,20 @@ class CompactPtr {
 
     bool isHashTableDeletedValue() const { return m_ptr == hashDeletedStorageValue; }
 
+    template<typename U>
+    friend bool operator==(const CompactPtr& a, const CompactPtr<U>& b)
+    {
+        return a.m_ptr == b.m_ptr;
+    }
+
+    template<typename U>
+    friend bool operator!=(const CompactPtr& a, const CompactPtr<U>& b)
+    {
+        return a.m_ptr != b.m_ptr;
+    }
+
+    StorageType storage() const { return m_ptr; }
+
 private:
     template <typename X>
     friend class CompactPtr;
@@ -212,6 +226,30 @@ class CompactPtr {
     StorageType m_ptr { 0 };
 };
 
+template<typename T, typename U>
+inline bool operator==(const CompactPtr<T>& a, U* b)
+{
+    return a.get() == b;
+}
+
+template<typename T, typename U>
+inline bool operator==(T* a, const CompactPtr<U>& b)
+{
+    return a == b.get();
+}
+
+template<typename T, typename U>
+inline bool operator!=(const CompactPtr<T>& a, U* b)
+{
+    return !(a == b);
+}
+
+template<typename T, typename U>
+inline bool operator!=(T* a, const CompactPtr<U>& b)
+{
+    return !(a == b);
+}
+
 template <typename T>
 struct GetPtrHelper<CompactPtr<T>> {
     using PtrType = T*;
@@ -244,7 +282,12 @@ struct CompactPtrTraits {
     static ALWAYS_INLINE bool isHashTableDeletedValue(const StorageType& ptr) { return ptr.isHashTableDeletedValue(); }
 };
 
-template<typename P> struct DefaultHash<CompactPtr<P>> : PtrHash<CompactPtr<P>> { };
+template<typename P> struct DefaultHash<CompactPtr<P>> {
+    using PtrType = CompactPtr<P>;
+    static unsigned hash(PtrType key) { return IntHash<typename PtrType::StorageType>::hash(key.storage()); }
+    static bool equal(PtrType a, PtrType b) { return a == b; }
+    static constexpr bool safeToCompareToEmptyOrDeleted = true;
+};
 
 } // namespace WTF
 

diff --git a/Source/WTF/wtf/HashTraits.h b/Source/WTF/wtf/HashTraits.h
@@ -248,8 +248,8 @@ template<typename P> struct HashTraits<Packed<P*>> : SimpleClassHashTraits<Packe
     static Packed<P*> emptyValue() { return nullptr; }
     static bool isEmptyValue(const TargetType& value) { return value.get() == nullptr; }
 
-    using PeekType = P*;
-    static PeekType peek(const TargetType& value) { return value.get(); }
+    using PeekType = Packed<P*>;
+    static PeekType peek(const TargetType& value) { return value; }
     static PeekType peek(P* value) { return value; }
 };
 
@@ -260,8 +260,8 @@ template<typename P> struct HashTraits<CompactPtr<P>> : SimpleClassHashTraits<Co
     static CompactPtr<P> emptyValue() { return nullptr; }
     static bool isEmptyValue(const TargetType& value) { return !value; }
 
-    using PeekType = P*;
-    static PeekType peek(const TargetType& value) { return value.get(); }
+    using PeekType = CompactPtr<P>;
+    static PeekType peek(const TargetType& value) { return value; }
     static PeekType peek(P* value) { return value; }
 };
 

diff --git a/Source/WTF/wtf/Packed.h b/Source/WTF/wtf/Packed.h
@@ -263,6 +263,42 @@ struct IsSmartPtr<PackedPtr<T>> {
     static constexpr bool value = true;
 };
 
+template<typename T, typename U>
+inline bool operator==(const PackedPtr<T>& a, const PackedPtr<U>& b)
+{
+    return a.get() == b.get();
+}
+
+template<typename T, typename U>
+inline bool operator!=(const PackedPtr<T>& a, const PackedPtr<U>& b)
+{
+    return a.get() != b.get();
+}
+
+template<typename T, typename U>
+inline bool operator==(const PackedPtr<T>& a, U* b)
+{
+    return a.get() == b;
+}
+
+template<typename T, typename U>
+inline bool operator==(T* a, const PackedPtr<U>& b)
+{
+    return a == b.get();
+}
+
+template<typename T, typename U>
+inline bool operator!=(const PackedPtr<T>& a, U* b)
+{
+    return !(a == b);
+}
+
+template<typename T, typename U>
+inline bool operator!=(T* a, const PackedPtr<U>& b)
+{
+    return !(a == b);
+}
+
 template<typename T>
 struct PackedPtrTraits {
     template<typename U> using RebindTraits = PackedPtrTraits<U>;

diff --git a/Source/WTF/wtf/RefPtr.h b/Source/WTF/wtf/RefPtr.h
@@ -110,6 +110,13 @@ class RefPtr {
     friend RefPtr adoptRef<T, PtrTraits, RefDerefTraits>(T*);
     template<typename X, typename Y, typename Z> friend class RefPtr;
 
+    template<typename T1, typename U, typename V, typename X, typename Y, typename Z>
+    friend bool operator==(const RefPtr<T1, U, V>&, const RefPtr<X, Y, Z>&);
+    template<typename T1, typename U, typename V, typename X>
+    friend bool operator==(const RefPtr<T1, U, V>&, X*);
+    template<typename T1, typename X, typename Y, typename Z>
+    friend bool operator==(T1*, const RefPtr<X, Y, Z>&);
+
     enum AdoptTag { Adopt };
     RefPtr(T* ptr, AdoptTag) : m_ptr(ptr) { }
 
@@ -205,38 +212,38 @@ inline void swap(RefPtr<T, U, V>& a, RefPtr<T, U, V>& b)
 
 template<typename T, typename U, typename V, typename X, typename Y, typename Z>
 inline bool operator==(const RefPtr<T, U, V>& a, const RefPtr<X, Y, Z>& b)
-{ 
-    return a.get() == b.get();
+{
+    return a.m_ptr == b.m_ptr;
 }
 
 template<typename T, typename U, typename V, typename X>
 inline bool operator==(const RefPtr<T, U, V>& a, X* b)
-{ 
-    return a.get() == b; 
+{
+    return a.m_ptr == b;
 }
 
 template<typename T, typename X, typename Y, typename Z>
 inline bool operator==(T* a, const RefPtr<X, Y, Z>& b)
 {
-    return a == b.get(); 
+    return a == b.m_ptr;
 }
 
 template<typename T, typename U, typename V, typename X, typename Y, typename Z>
 inline bool operator!=(const RefPtr<T, U, V>& a, const RefPtr<X, Y, Z>& b)
-{ 
-    return a.get() != b.get(); 
+{
+    return !(a == b);
 }
 
 template<typename T, typename U, typename V, typename X>
 inline bool operator!=(const RefPtr<T, U, V>& a, X* b)
 {
-    return a.get() != b; 
+    return !(a == b);
 }
 
 template<typename T, typename X, typename Y, typename Z>
 inline bool operator!=(T* a, const RefPtr<X, Y, Z>& b)
-{ 
-    return a != b.get(); 
+{
+    return !(a == b);
 }
 
 template<typename T, typename U, typename V>

diff --git a/Source/WTF/wtf/text/AtomStringImpl.cpp b/Source/WTF/wtf/text/AtomStringImpl.cpp
@@ -445,7 +445,7 @@ Ref<AtomStringImpl> AtomStringImpl::addSlowCase(AtomStringTable& stringTable, St
 // When removing a string from the table, we know it's already the one in the table, so no need for a string equality check.
 struct AtomStringTableRemovalHashTranslator {
     static unsigned hash(AtomStringImpl* string) { return string->hash(); }
-    static bool equal(const AtomStringTable::StringEntry& a, AtomStringImpl* b) { return a.get() == b; }
+    static bool equal(const AtomStringTable::StringEntry& a, AtomStringImpl* b) { return a == b; }
 };
 
 void AtomStringImpl::remove(AtomStringImpl* string)