Skip to content

Commit

Permalink
[WTF] Use CompactPtr in AtomStringTable if it is more efficient
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=241883

Reviewed by Darin Adler.

1. We add HashTable support for CompactPtr. Correctly setting up HashTraits and Hashers so that we can have HashSet<CompactPtr<T>>.
2. Use CompactPtr in AtomStringTable if it is more efficient than PackedPtr<StringImpl>. Typically, this means we are in iOS.

* Source/WTF/wtf/CompactPtr.h:
(WTF::CompactPtr::CompactPtr):
(WTF::CompactPtr::encode):
(WTF::CompactPtr::decode):
(WTF::CompactPtr::isHashTableDeletedValue const):
(WTF::CompactPtrTraits::hashTableDeletedValue):
(WTF::CompactPtrTraits::isHashTableDeletedValue):
* Source/WTF/wtf/Forward.h:
* Source/WTF/wtf/HashTraits.h:
(WTF::HashTraits<CompactPtr<P>>::emptyValue):
(WTF::HashTraits<CompactPtr<P>>::isEmptyValue):
(WTF::HashTraits<CompactPtr<P>>::peek):
* Source/WTF/wtf/text/AtomStringImpl.cpp:
(WTF::UCharBufferTranslator::equal):
(WTF::UCharBufferTranslator::translate):
(WTF::HashAndUTF8CharactersTranslator::equal):
(WTF::HashAndUTF8CharactersTranslator::translate):
(WTF::SubstringTranslator::translate):
(WTF::SubstringTranslator8::equal):
(WTF::SubstringTranslator16::equal):
(WTF::LCharBufferTranslator::equal):
(WTF::LCharBufferTranslator::translate):
(WTF::BufferFromStaticDataTranslator::equal):
(WTF::BufferFromStaticDataTranslator::translate):
* Source/WTF/wtf/text/AtomStringTable.h:
* Tools/TestWebKitAPI/Tests/WTF/CompactPtr.cpp:
(TestWebKitAPI::TEST):

Canonical link: https://commits.webkit.org/251776@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@295771 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
Constellation committed Jun 23, 2022
1 parent e49129f commit ee26f4f
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 15 deletions.
21 changes: 18 additions & 3 deletions Source/WTF/wtf/CompactPtr.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@

#include <cstdint>
#include <utility>
#include <wtf/Forward.h>
#include <wtf/GetPtr.h>
#include <wtf/HashFunctions.h>
#include <wtf/RawPtrTraits.h>
#include <wtf/StdLibExtras.h>

Expand All @@ -47,7 +50,6 @@ namespace WTF {
template <typename T>
class CompactPtr {
WTF_MAKE_FAST_ALLOCATED;

public:
#if HAVE(36BIT_ADDRESS)
// The CompactPtr algorithm relies on being able to shift
Expand All @@ -56,8 +58,10 @@ class CompactPtr {
// loss is if the if the address is always 16 bytes aligned i.e.
// the lower 4 bits is always 0.
using StorageType = uint32_t;
static constexpr bool is32Bit = true;
#else
using StorageType = uintptr_t;
static constexpr bool is32Bit = false;
#endif
static constexpr bool isCompactedType = true;

Expand All @@ -82,6 +86,8 @@ class CompactPtr {
std::exchange(o.m_ptr, 0);
}

ALWAYS_INLINE constexpr CompactPtr(HashTableDeletedValueType) : m_ptr(hashDeletedStorageValue) { }

ALWAYS_INLINE ~CompactPtr() = default;

T& operator*() const { return *get(); }
Expand Down Expand Up @@ -173,6 +179,7 @@ class CompactPtr {
{
uintptr_t intPtr = bitwise_cast<uintptr_t>(ptr);
#if HAVE(36BIT_ADDRESS)
static_assert(alignof(T) >= (1ULL << bitsShift));
ASSERT(!(intPtr & alignmentMask));
StorageType encoded = static_cast<StorageType>(intPtr >> bitsShift);
ASSERT(decode(encoded) == ptr);
Expand All @@ -185,18 +192,22 @@ class CompactPtr {
static ALWAYS_INLINE T* decode(StorageType ptr)
{
#if HAVE(36BIT_ADDRESS)
static_assert(alignof(T) >= (1ULL << bitsShift));
return bitwise_cast<T*>(static_cast<uintptr_t>(ptr) << bitsShift);
#else
return bitwise_cast<T*>(ptr);
#endif
}

bool isHashTableDeletedValue() const { return m_ptr == hashDeletedStorageValue; }

private:
template <typename X>
friend class CompactPtr;

static constexpr uint32_t bitsShift = 4;
static constexpr uintptr_t alignmentMask = (1ull << bitsShift) - 1;
static constexpr StorageType hashDeletedStorageValue = 1; // 0x16 (encoded as 1) is within the first unmapped page for nullptr. Thus, it never appears.

StorageType m_ptr { 0 };
};
Expand All @@ -219,6 +230,8 @@ struct CompactPtrTraits {

using StorageType = CompactPtr<T>;

static constexpr bool is32Bit = StorageType::is32Bit;

template <typename U>
static ALWAYS_INLINE T* exchange(StorageType& ptr, U&& newValue) { return ptr.exchange(newValue); }

Expand All @@ -227,10 +240,12 @@ struct CompactPtrTraits {

static ALWAYS_INLINE T* unwrap(const StorageType& ptr) { return ptr.get(); }

static StorageType hashTableDeletedValue() { return bitwise_cast<StorageType>(static_cast<StorageType>(-1)); }
static ALWAYS_INLINE bool isHashTableDeletedValue(const StorageType& ptr) { return ptr == hashTableDeletedValue(); }
static StorageType hashTableDeletedValue() { return StorageType { HashTableDeletedValue }; }
static ALWAYS_INLINE bool isHashTableDeletedValue(const StorageType& ptr) { return ptr.isHashTableDeletedValue(); }
};

template<typename P> struct DefaultHash<CompactPtr<P>> : PtrHash<CompactPtr<P>> { };

} // namespace WTF

using WTF::CompactPtr;
Expand Down
1 change: 1 addition & 0 deletions Source/WTF/wtf/Forward.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ using VectorMalloc = FastMalloc;

template<typename> struct DefaultRefDerefTraits;

template<typename> class CompactPtr;
template<typename> class CompletionHandler;
template<typename> class FixedVector;
template<typename> class Function;
Expand Down
12 changes: 12 additions & 0 deletions Source/WTF/wtf/HashTraits.h
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,18 @@ template<typename P> struct HashTraits<Packed<P*>> : SimpleClassHashTraits<Packe
static PeekType peek(P* value) { return value; }
};

template<typename P> struct HashTraits<CompactPtr<P>> : SimpleClassHashTraits<CompactPtr<P>> {
static constexpr bool hasIsEmptyValueFunction = true;
using TargetType = CompactPtr<P>;

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(); }
static PeekType peek(P* value) { return value; }
};

template<> struct HashTraits<String> : SimpleClassHashTraits<String> {
static constexpr bool hasIsEmptyValueFunction = true;
static bool isEmptyValue(const String&);
Expand Down
22 changes: 11 additions & 11 deletions Source/WTF/wtf/text/AtomStringImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -95,12 +95,12 @@ struct UCharBufferTranslator {
return buf.hash;
}

static bool equal(PackedPtr<StringImpl> const& str, const UCharBuffer& buf)
static bool equal(AtomStringTable::StringEntry const& str, const UCharBuffer& buf)
{
return WTF::equal(str.get(), buf.characters, buf.length);
}

static void translate(PackedPtr<StringImpl>& location, const UCharBuffer& buf, unsigned hash)
static void translate(AtomStringTable::StringEntry& location, const UCharBuffer& buf, unsigned hash)
{
auto* pointer = &StringImpl::create8BitIfPossible(buf.characters, buf.length).leakRef();
pointer->setHash(hash);
Expand All @@ -122,7 +122,7 @@ struct HashAndUTF8CharactersTranslator {
return buffer.hash;
}

static bool equal(PackedPtr<StringImpl> const& passedString, const HashAndUTF8Characters& buffer)
static bool equal(AtomStringTable::StringEntry const& passedString, const HashAndUTF8Characters& buffer)
{
auto* string = passedString.get();
if (buffer.utf16Length != string->length())
Expand Down Expand Up @@ -159,7 +159,7 @@ struct HashAndUTF8CharactersTranslator {
return true;
}

static void translate(PackedPtr<StringImpl>& location, const HashAndUTF8Characters& buffer, unsigned hash)
static void translate(AtomStringTable::StringEntry& location, const HashAndUTF8Characters& buffer, unsigned hash)
{
UChar* target;
auto newString = StringImpl::createUninitialized(buffer.utf16Length, target);
Expand Down Expand Up @@ -198,7 +198,7 @@ struct SubstringLocation {
};

struct SubstringTranslator {
static void translate(PackedPtr<StringImpl>& location, const SubstringLocation& buffer, unsigned hash)
static void translate(AtomStringTable::StringEntry& location, const SubstringLocation& buffer, unsigned hash)
{
auto* pointer = &StringImpl::createSubstringSharingImpl(*buffer.baseString, buffer.start, buffer.length).leakRef();
pointer->setHash(hash);
Expand All @@ -213,7 +213,7 @@ struct SubstringTranslator8 : SubstringTranslator {
return StringHasher::computeHashAndMaskTop8Bits(buffer.baseString->characters8() + buffer.start, buffer.length);
}

static bool equal(PackedPtr<StringImpl> const& string, const SubstringLocation& buffer)
static bool equal(AtomStringTable::StringEntry const& string, const SubstringLocation& buffer)
{
return WTF::equal(string.get(), buffer.baseString->characters8() + buffer.start, buffer.length);
}
Expand All @@ -225,7 +225,7 @@ struct SubstringTranslator16 : SubstringTranslator {
return StringHasher::computeHashAndMaskTop8Bits(buffer.baseString->characters16() + buffer.start, buffer.length);
}

static bool equal(PackedPtr<StringImpl> const& string, const SubstringLocation& buffer)
static bool equal(AtomStringTable::StringEntry const& string, const SubstringLocation& buffer)
{
return WTF::equal(string.get(), buffer.baseString->characters16() + buffer.start, buffer.length);
}
Expand Down Expand Up @@ -259,12 +259,12 @@ struct LCharBufferTranslator {
return buf.hash;
}

static bool equal(PackedPtr<StringImpl> const& str, const LCharBuffer& buf)
static bool equal(AtomStringTable::StringEntry const& str, const LCharBuffer& buf)
{
return WTF::equal(str.get(), buf.characters, buf.length);
}

static void translate(PackedPtr<StringImpl>& location, const LCharBuffer& buf, unsigned hash)
static void translate(AtomStringTable::StringEntry& location, const LCharBuffer& buf, unsigned hash)
{
auto* pointer = &StringImpl::create(buf.characters, buf.length).leakRef();
pointer->setHash(hash);
Expand All @@ -281,12 +281,12 @@ struct BufferFromStaticDataTranslator {
return buf.hash;
}

static bool equal(PackedPtr<StringImpl> const& str, const Buffer& buf)
static bool equal(AtomStringTable::StringEntry const& str, const Buffer& buf)
{
return WTF::equal(str.get(), buf.characters, buf.length);
}

static void translate(PackedPtr<StringImpl>& location, const Buffer& buf, unsigned hash)
static void translate(AtomStringTable::StringEntry& location, const Buffer& buf, unsigned hash)
{
auto* pointer = &StringImpl::createWithoutCopying(buf.characters, buf.length).leakRef();
pointer->setHash(hash);
Expand Down
5 changes: 4 additions & 1 deletion Source/WTF/wtf/text/AtomStringTable.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

#pragma once

#include <wtf/CompactPtr.h>
#include <wtf/HashSet.h>
#include <wtf/Packed.h>
#include <wtf/text/StringHash.h>
Expand All @@ -34,7 +35,9 @@ class StringImpl;
class AtomStringTable {
WTF_MAKE_FAST_ALLOCATED;
public:
using StringEntry = PackedPtr<StringImpl>;
// If CompactPtr is 32bit, it is more efficient than PackedPtr (6 bytes).
// We select underlying implementation based on CompactPtr's efficacy.
using StringEntry = std::conditional_t<CompactPtrTraits<StringImpl>::is32Bit, CompactPtr<StringImpl>, PackedPtr<StringImpl>>;
using StringTableImpl = HashSet<StringEntry>;

WTF_EXPORT_PRIVATE ~AtomStringTable();
Expand Down
21 changes: 21 additions & 0 deletions Tools/TestWebKitAPI/Tests/WTF/CompactPtr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

#include "AlignedRefLogger.h"
#include "Utilities.h"
#include <wtf/HashMap.h>
#include <wtf/MainThread.h>
#include <wtf/NeverDestroyed.h>
#include <wtf/RefCounted.h>
Expand Down Expand Up @@ -195,4 +196,24 @@ TEST(WTF_CompactPtr, Assignment)
}
}

struct alignas(16) AlignedPackingTarget {
unsigned m_value { 0 };
};
TEST(WTF_CompactPtr, HashMap)
{
Vector<AlignedPackingTarget> vector;
HashMap<PackedPtr<AlignedPackingTarget>, unsigned> map;
vector.reserveCapacity(10000);
for (unsigned i = 0; i < 10000; ++i)
vector.uncheckedAppend(AlignedPackingTarget { i });

for (auto& target : vector)
map.add(&target, target.m_value);

for (auto& target : vector) {
EXPECT_TRUE(map.contains(&target));
EXPECT_EQ(map.get(&target), target.m_value);
}
}

} // namespace TestWebKitAPI

0 comments on commit ee26f4f

Please sign in to comment.