Skip to content

Commit 38bcd32

Browse files
cdumezcdumez
committed
Optimize looking up keys by raw reference in hash containers whose key is a non-nullable smart pointer type
https://bugs.webkit.org/show_bug.cgi?id=266166 Reviewed by Darin Adler. Optimize looking up keys by raw reference in hash containers whose key is a non-nullable smart pointer type (such as Ref<> / WeakRef<> / CheckedRef<>). I noticed while adopting WeakRef<> more in our containers that hash container functions can be passed a raw C++ reference. This is convenient but actually generates unnecessary refcounting churn for many of these functions because it creates a WeakRef. Creating a WeakRef is unnecessary for lookup functions such as contains() / find() and removal functions such as remove() / take(). We already had overloads in place for these functions when passing in a raw C++ pointer and the key is a smart pointer type to avoid refcounting churn. However, we didn't have such optimization when passing a raw C++ reference, which is useful when the key type is a non-nullable smart pointer. This patch adds to our hash containers function overloads that take in a raw C++ reference, similar to the ones that take in a raw C++ pointer. These overloads are only available when the key is a non-nullable smart pointer type. I also make an improvement to our existing overloads taking in a raw C++ pointer so that they no longer require a non-const pointer. There is no reason to require a non-const pointer just for pointer hashing and pointer comparison. I also noticed this issue when adopting more smart pointers in containers. * Source/JavaScriptCore/runtime/Identifier.h: (JSC::IdentifierRepHash::hash): * Source/WTF/wtf/CheckedPtr.h: * Source/WTF/wtf/CheckedRef.h: * Source/WTF/wtf/CompactPtr.h: * Source/WTF/wtf/ConcurrentPtrHashSet.h: * Source/WTF/wtf/GetPtr.h: * Source/WTF/wtf/HashCountedSet.h: (WTF::Traits>::find): (WTF::Traits>::find const): (WTF::Traits>::contains const): (WTF::Traits>::count const): (WTF::Traits>::remove): * Source/WTF/wtf/HashFunctions.h: * Source/WTF/wtf/HashMap.h: (WTF::Y>::find): (WTF::Y>::find const): (WTF::Y>::contains const): (WTF::Y>::inlineGet const): (WTF::Y>::get const): (WTF::Y>::remove): (WTF::Y>::take): * Source/WTF/wtf/HashSet.h: (WTF::TableTraits>::find const): (WTF::TableTraits>::contains const): (WTF::TableTraits>::remove): (WTF::TableTraits>::take): * Source/WTF/wtf/ListHashSet.h: (WTF::U>::find): (WTF::U>::find const): (WTF::U>::contains const): (WTF::U>::insertBefore): (WTF::U>::remove): * Source/WTF/wtf/Packed.h: * Source/WTF/wtf/Ref.h: * Source/WTF/wtf/RefPtr.h: * Source/WTF/wtf/RetainPtr.h: * Source/WTF/wtf/SchedulePair.h: (WTF::SchedulePairHash::hash): (WTF::SchedulePairHash::equal): * Source/WTF/wtf/SignedPtr.h: * Source/WTF/wtf/UniqueRef.h: * Source/WTF/wtf/WeakPtr.h: (WTF::WeakPtr::WeakPtr): * Source/WTF/wtf/WeakRef.h: (WTF::WeakRef::releaseImpl): (WTF::WeakRefHashTraits::take): * Source/WTF/wtf/glib/GRefPtr.h: * Source/WTF/wtf/text/StringHash.h: (WTF::StringHash::hash): (WTF::ASCIICaseInsensitiveHash::hash): * Source/WebCore/dom/GCReachableRef.h: * Source/WebCore/page/SecurityOriginHash.h: (WebCore::SecurityOriginHash::hash): (WebCore::SecurityOriginHash::equal): * Source/WebCore/platform/win/COMPtr.h: * Source/WebCore/rendering/FloatingObjects.h: (WebCore::FloatingObjectHashFunctions::hash): (WebCore::FloatingObjectHashFunctions::equal): * Source/WebCore/rendering/GlyphDisplayListCache.h: (WebCore::GlyphDisplayListCacheEntryHash::hash): (WebCore::GlyphDisplayListCacheEntryHash::equal): * Source/WebKit/UIProcess/API/cpp/WKRetainPtr.h: * Tools/TestWebKitAPI/Tests/WTF/WeakPtr.cpp: (TestWebKitAPI::TestType::resetDidUpdateImplRefCount): (TestWebKitAPI::TestType::didUpdateImplRefCount const): (TestWebKitAPI::TEST): Canonical link: https://commits.webkit.org/271853@main
1 parent 7ba7b1e commit 38bcd32

34 files changed

Lines changed: 481 additions & 91 deletions

‎Source/JavaScriptCore/bytecode/StructureStubInfo.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -663,12 +663,12 @@ class SharedJITStubSet {
663663
};
664664

665665
struct PointerTranslator {
666-
static unsigned hash(PolymorphicAccessJITStubRoutine* stub)
666+
static unsigned hash(const PolymorphicAccessJITStubRoutine* stub)
667667
{
668668
return stub->hash();
669669
}
670670

671-
static bool equal(const Hash::Key& key, PolymorphicAccessJITStubRoutine* stub)
671+
static bool equal(const Hash::Key& key, const PolymorphicAccessJITStubRoutine* stub)
672672
{
673673
return key.m_wrapped == stub;
674674
}

‎Source/JavaScriptCore/jit/JITThunks.cpp‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ static inline NativeExecutable& getMayBeDyingNativeExecutable(const Weak<NativeE
6969
return *executable;
7070
}
7171

72-
inline unsigned JITThunks::WeakNativeExecutableHash::hash(NativeExecutable* executable)
72+
inline unsigned JITThunks::WeakNativeExecutableHash::hash(const NativeExecutable* executable)
7373
{
7474
return hash(executable->function(), executable->constructor(), executable->implementationVisibility(), executable->name());
7575
}
@@ -79,7 +79,7 @@ inline unsigned JITThunks::WeakNativeExecutableHash::hash(const Weak<NativeExecu
7979
return hash(&getMayBeDyingNativeExecutable(key));
8080
}
8181

82-
inline bool JITThunks::WeakNativeExecutableHash::equal(NativeExecutable& a, NativeExecutable& b)
82+
inline bool JITThunks::WeakNativeExecutableHash::equal(const NativeExecutable& a, const NativeExecutable& b)
8383
{
8484
if (&a == &b)
8585
return true;
@@ -91,7 +91,7 @@ inline bool JITThunks::WeakNativeExecutableHash::equal(const Weak<NativeExecutab
9191
return equal(getMayBeDyingNativeExecutable(a), getMayBeDyingNativeExecutable(b));
9292
}
9393

94-
inline bool JITThunks::WeakNativeExecutableHash::equal(const Weak<NativeExecutable>& a, NativeExecutable* bExecutable)
94+
inline bool JITThunks::WeakNativeExecutableHash::equal(const Weak<NativeExecutable>& a, const NativeExecutable* bExecutable)
9595
{
9696
return equal(getMayBeDyingNativeExecutable(a), *bExecutable);
9797
}
@@ -213,8 +213,8 @@ struct JITThunks::HostKeySearcher {
213213
};
214214

215215
struct JITThunks::NativeExecutableTranslator {
216-
static unsigned hash(NativeExecutable* key) { return WeakNativeExecutableHash::hash(key); }
217-
static bool equal(const Weak<NativeExecutable>& a, NativeExecutable* b) { return WeakNativeExecutableHash::equal(a, b); }
216+
static unsigned hash(const NativeExecutable* key) { return WeakNativeExecutableHash::hash(key); }
217+
static bool equal(const Weak<NativeExecutable>& a, const NativeExecutable* b) { return WeakNativeExecutableHash::equal(a, b); }
218218
static void translate(Weak<NativeExecutable>& location, NativeExecutable* executable, unsigned)
219219
{
220220
location = Weak<NativeExecutable>(executable, executable->vm().jitStubs.get());

‎Source/JavaScriptCore/jit/JITThunks.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -121,16 +121,16 @@ class JITThunks final : private WeakHandleOwner {
121121

122122
struct WeakNativeExecutableHash {
123123
static inline unsigned hash(const Weak<NativeExecutable>&);
124-
static inline unsigned hash(NativeExecutable*);
124+
static inline unsigned hash(const NativeExecutable*);
125125
static unsigned hash(const HostFunctionKey& key)
126126
{
127127
return hash(std::get<0>(key), std::get<1>(key), std::get<2>(key), std::get<3>(key));
128128
}
129129

130130
static inline bool equal(const Weak<NativeExecutable>&, const Weak<NativeExecutable>&);
131131
static inline bool equal(const Weak<NativeExecutable>&, const HostFunctionKey&);
132-
static inline bool equal(const Weak<NativeExecutable>&, NativeExecutable*);
133-
static inline bool equal(NativeExecutable&, NativeExecutable&);
132+
static inline bool equal(const Weak<NativeExecutable>&, const NativeExecutable*);
133+
static inline bool equal(const NativeExecutable&, const NativeExecutable&);
134134
static constexpr bool safeToCompareToEmptyOrDeleted = false;
135135

136136
private:

‎Source/JavaScriptCore/runtime/Identifier.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ JSValue identifierToSafePublicJSValue(VM&, const Identifier&);
277277
// https://bugs.webkit.org/show_bug.cgi?id=150137
278278
struct IdentifierRepHash : PtrHash<RefPtr<UniquedStringImpl>> {
279279
static unsigned hash(const RefPtr<UniquedStringImpl>& key) { return key->existingSymbolAwareHash(); }
280-
static unsigned hash(UniquedStringImpl* key) { return key->existingSymbolAwareHash(); }
280+
static unsigned hash(const UniquedStringImpl* key) { return key->existingSymbolAwareHash(); }
281281
static constexpr bool hasHashInValue = true;
282282
};
283283

‎Source/WTF/wtf/CheckedPtr.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,8 @@ class CheckedPtr {
263263

264264
template <typename T, typename PtrTraits>
265265
struct GetPtrHelper<CheckedPtr<T, PtrTraits>> {
266-
typedef T* PtrType;
266+
using PtrType = T*;
267+
using UnderlyingType = T;
267268
static T* getPtr(const CheckedPtr<T, PtrTraits>& p) { return const_cast<T*>(p.get()); }
268269
};
269270

‎Source/WTF/wtf/CheckedRef.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -237,13 +237,15 @@ class CheckedRef {
237237

238238
template <typename T, typename PtrTraits>
239239
struct GetPtrHelper<CheckedRef<T, PtrTraits>> {
240-
typedef T* PtrType;
240+
using PtrType = T*;
241+
using UnderlyingType = T;
241242
static T* getPtr(const CheckedRef<T, PtrTraits>& p) { return const_cast<T*>(p.ptr()); }
242243
};
243244

244245
template <typename T, typename U>
245246
struct IsSmartPtr<CheckedRef<T, U>> {
246247
static constexpr bool value = true;
248+
static constexpr bool isNullable = true;
247249
};
248250

249251
template<typename ExpectedType, typename ArgType, typename ArgPtrTraits>

‎Source/WTF/wtf/CompactPtr.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,12 +237,14 @@ inline bool operator==(T* a, const CompactPtr<U>& b)
237237
template <typename T>
238238
struct GetPtrHelper<CompactPtr<T>> {
239239
using PtrType = T*;
240+
using UnderlyingType = T;
240241
static T* getPtr(const CompactPtr<T>& p) { return const_cast<T*>(p.get()); }
241242
};
242243

243244
template <typename T>
244245
struct IsSmartPtr<CompactPtr<T>> {
245246
static constexpr bool value = true;
247+
static constexpr bool isNullable = true;
246248
};
247249

248250
template <typename T>

‎Source/WTF/wtf/ConcurrentPtrHashSet.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,9 +108,9 @@ class ConcurrentPtrHashSet final {
108108
Atomic<void*> array[1];
109109
};
110110

111-
static unsigned hash(void* ptr)
111+
static unsigned hash(const void* ptr)
112112
{
113-
return PtrHash<void*>::hash(ptr);
113+
return PtrHash<const void*>::hash(ptr);
114114
}
115115

116116
void initialize();

‎Source/WTF/wtf/GetPtr.h‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,20 +31,23 @@ template <typename T> inline T* getPtr(T* p) { return p; }
3131

3232
template <typename T> struct IsSmartPtr {
3333
static constexpr bool value = false;
34+
static constexpr bool isNullable = true;
3435
};
3536

3637
template <typename T, bool isSmartPtr>
3738
struct GetPtrHelperBase;
3839

3940
template <typename T>
4041
struct GetPtrHelperBase<T, false /* isSmartPtr */> {
41-
typedef T* PtrType;
42+
using PtrType = T*;
43+
using UnderlyingType = T;
4244
static T* getPtr(T& p) { return std::addressof(p); }
4345
};
4446

4547
template <typename T>
4648
struct GetPtrHelperBase<T, true /* isSmartPtr */> {
47-
typedef typename T::PtrType PtrType;
49+
using PtrType = typename T::PtrType;
50+
using UnderlyingType = typename T::ValueType;
4851
static PtrType getPtr(const T& p) { return p.get(); }
4952
};
5053

@@ -68,11 +71,13 @@ inline typename GetPtrHelper<T>::PtrType getPtr(const T& p)
6871

6972
template <typename T, typename Deleter> struct IsSmartPtr<std::unique_ptr<T, Deleter>> {
7073
static constexpr bool value = true;
74+
static constexpr bool isNullable = true;
7175
};
7276

7377
template <typename T, typename Deleter>
7478
struct GetPtrHelper<std::unique_ptr<T, Deleter>> {
75-
typedef T* PtrType;
79+
using PtrType = T*;
80+
using UnderlyingType = T;
7681
static T* getPtr(const std::unique_ptr<T, Deleter>& p) { return p.get(); }
7782
};
7883

‎Source/WTF/wtf/HashCountedSet.h‎

Lines changed: 52 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -104,11 +104,18 @@ class HashCountedSet final {
104104
void clear();
105105

106106
// Overloads for smart pointer keys that take the raw pointer type as the parameter.
107-
template<typename V = ValueType> typename std::enable_if<IsSmartPtr<V>::value, iterator>::type find(typename GetPtrHelper<V>::PtrType);
108-
template<typename V = ValueType> typename std::enable_if<IsSmartPtr<V>::value, const_iterator>::type find(typename GetPtrHelper<V>::PtrType) const;
109-
template<typename V = ValueType> typename std::enable_if<IsSmartPtr<V>::value, bool>::type contains(typename GetPtrHelper<V>::PtrType) const;
110-
template<typename V = ValueType> typename std::enable_if<IsSmartPtr<V>::value, unsigned>::type count(typename GetPtrHelper<V>::PtrType) const;
111-
template<typename V = ValueType> typename std::enable_if<IsSmartPtr<V>::value, bool>::type remove(typename GetPtrHelper<V>::PtrType);
107+
template<typename V = ValueType> typename std::enable_if<IsSmartPtr<V>::value, iterator>::type find(std::add_const_t<typename GetPtrHelper<V>::UnderlyingType>*);
108+
template<typename V = ValueType> typename std::enable_if<IsSmartPtr<V>::value, const_iterator>::type find(std::add_const_t<typename GetPtrHelper<V>::UnderlyingType>*) const;
109+
template<typename V = ValueType> typename std::enable_if<IsSmartPtr<V>::value, bool>::type contains(std::add_const_t<typename GetPtrHelper<V>::UnderlyingType>*) const;
110+
template<typename V = ValueType> typename std::enable_if<IsSmartPtr<V>::value, unsigned>::type count(std::add_const_t<typename GetPtrHelper<V>::UnderlyingType>*) const;
111+
template<typename V = ValueType> typename std::enable_if<IsSmartPtr<V>::value, bool>::type remove(std::add_const_t<typename GetPtrHelper<V>::UnderlyingType>*);
112+
113+
// Overloads for non-nullable smart pointer values that take the raw reference type as the parameter.
114+
template<typename V = ValueType> typename std::enable_if<IsSmartPtr<V>::value && !IsSmartPtr<V>::isNullable, iterator>::type find(std::add_const_t<typename GetPtrHelper<V>::UnderlyingType>&);
115+
template<typename V = ValueType> typename std::enable_if<IsSmartPtr<V>::value && !IsSmartPtr<V>::isNullable, const_iterator>::type find(std::add_const_t<typename GetPtrHelper<V>::UnderlyingType>&) const;
116+
template<typename V = ValueType> typename std::enable_if<IsSmartPtr<V>::value && !IsSmartPtr<V>::isNullable, bool>::type contains(std::add_const_t<typename GetPtrHelper<V>::UnderlyingType>&) const;
117+
template<typename V = ValueType> typename std::enable_if<IsSmartPtr<V>::value && !IsSmartPtr<V>::isNullable, unsigned>::type count(std::add_const_t<typename GetPtrHelper<V>::UnderlyingType>&) const;
118+
template<typename V = ValueType> typename std::enable_if<IsSmartPtr<V>::value && !IsSmartPtr<V>::isNullable, bool>::type remove(std::add_const_t<typename GetPtrHelper<V>::UnderlyingType>&);
112119

113120
static bool isValidValue(const ValueType& value) { return ImplType::isValidValue(value); }
114121

@@ -281,39 +288,74 @@ inline void HashCountedSet<Value, HashFunctions, Traits>::clear()
281288

282289
template<typename Value, typename HashFunctions, typename Traits>
283290
template<typename V>
284-
inline auto HashCountedSet<Value, HashFunctions, Traits>::find(typename GetPtrHelper<V>::PtrType value) -> typename std::enable_if<IsSmartPtr<V>::value, iterator>::type
291+
inline auto HashCountedSet<Value, HashFunctions, Traits>::find(std::add_const_t<typename GetPtrHelper<V>::UnderlyingType>* value) -> typename std::enable_if<IsSmartPtr<V>::value, iterator>::type
285292
{
286293
return m_impl.find(value);
287294
}
288295

289296
template<typename Value, typename HashFunctions, typename Traits>
290297
template<typename V>
291-
inline auto HashCountedSet<Value, HashFunctions, Traits>::find(typename GetPtrHelper<V>::PtrType value) const -> typename std::enable_if<IsSmartPtr<V>::value, const_iterator>::type
298+
inline auto HashCountedSet<Value, HashFunctions, Traits>::find(std::add_const_t<typename GetPtrHelper<V>::UnderlyingType>* value) const -> typename std::enable_if<IsSmartPtr<V>::value, const_iterator>::type
292299
{
293300
return m_impl.find(value);
294301
}
295302

296303
template<typename Value, typename HashFunctions, typename Traits>
297304
template<typename V>
298-
inline auto HashCountedSet<Value, HashFunctions, Traits>::contains(typename GetPtrHelper<V>::PtrType value) const -> typename std::enable_if<IsSmartPtr<V>::value, bool>::type
305+
inline auto HashCountedSet<Value, HashFunctions, Traits>::contains(std::add_const_t<typename GetPtrHelper<V>::UnderlyingType>* value) const -> typename std::enable_if<IsSmartPtr<V>::value, bool>::type
299306
{
300307
return m_impl.contains(value);
301308
}
302309

303310
template<typename Value, typename HashFunctions, typename Traits>
304311
template<typename V>
305-
inline auto HashCountedSet<Value, HashFunctions, Traits>::count(typename GetPtrHelper<V>::PtrType value) const -> typename std::enable_if<IsSmartPtr<V>::value, unsigned>::type
312+
inline auto HashCountedSet<Value, HashFunctions, Traits>::count(std::add_const_t<typename GetPtrHelper<V>::UnderlyingType>* value) const -> typename std::enable_if<IsSmartPtr<V>::value, unsigned>::type
306313
{
307314
return m_impl.get(value);
308315
}
309316

310317
template<typename Value, typename HashFunctions, typename Traits>
311318
template<typename V>
312-
inline auto HashCountedSet<Value, HashFunctions, Traits>::remove(typename GetPtrHelper<V>::PtrType value) -> typename std::enable_if<IsSmartPtr<V>::value, bool>::type
319+
inline auto HashCountedSet<Value, HashFunctions, Traits>::remove(std::add_const_t<typename GetPtrHelper<V>::UnderlyingType>* value) -> typename std::enable_if<IsSmartPtr<V>::value, bool>::type
313320
{
314321
return remove(find(value));
315322
}
316323

324+
template<typename Value, typename HashFunctions, typename Traits>
325+
template<typename V>
326+
inline auto HashCountedSet<Value, HashFunctions, Traits>::find(std::add_const_t<typename GetPtrHelper<V>::UnderlyingType>& value) -> typename std::enable_if<IsSmartPtr<V>::value && !IsSmartPtr<V>::isNullable, iterator>::type
327+
{
328+
return find(&value);
329+
}
330+
331+
template<typename Value, typename HashFunctions, typename Traits>
332+
template<typename V>
333+
inline auto HashCountedSet<Value, HashFunctions, Traits>::find(std::add_const_t<typename GetPtrHelper<V>::UnderlyingType>& value) const -> typename std::enable_if<IsSmartPtr<V>::value && !IsSmartPtr<V>::isNullable, const_iterator>::type
334+
{
335+
return find(&value);
336+
}
337+
338+
template<typename Value, typename HashFunctions, typename Traits>
339+
template<typename V>
340+
inline auto HashCountedSet<Value, HashFunctions, Traits>::contains(std::add_const_t<typename GetPtrHelper<V>::UnderlyingType>& value) const -> typename std::enable_if<IsSmartPtr<V>::value && !IsSmartPtr<V>::isNullable, bool>::type
341+
{
342+
return contains(&value);
343+
}
344+
345+
template<typename Value, typename HashFunctions, typename Traits>
346+
template<typename V>
347+
inline auto HashCountedSet<Value, HashFunctions, Traits>::count(std::add_const_t<typename GetPtrHelper<V>::UnderlyingType>& value) const -> typename std::enable_if<IsSmartPtr<V>::value && !IsSmartPtr<V>::isNullable, unsigned>::type
348+
{
349+
return count(&value);
350+
}
351+
352+
template<typename Value, typename HashFunctions, typename Traits>
353+
template<typename V>
354+
inline auto HashCountedSet<Value, HashFunctions, Traits>::remove(std::add_const_t<typename GetPtrHelper<V>::UnderlyingType>& value) -> typename std::enable_if<IsSmartPtr<V>::value && !IsSmartPtr<V>::isNullable, bool>::type
355+
{
356+
return remove(&value);
357+
}
358+
317359
} // namespace WTF
318360

319361
using WTF::HashCountedSet;

0 commit comments

Comments
 (0)