Skip to content

Commit

Permalink
[JSC] Use simple Vector for BufferedStructures
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=273863
rdar://127713950

Reviewed by Keith Miller.

This HashMap size is relatively small (since we have upper limit for IC feedback).
We should make it Vector since HashMap for this size is costly.

* Source/JavaScriptCore/bytecode/StructureStubInfo.cpp:
(JSC::StructureStubInfo::visitAggregateImpl):
(JSC::StructureStubInfo::visitWeakReferences):
* Source/JavaScriptCore/bytecode/StructureStubInfo.h:
(JSC::StructureStubInfo::considerRepatchingCacheImpl):
(JSC::StructureStubInfo::clearBufferedStructures):

Canonical link: https://commits.webkit.org/278530@main
  • Loading branch information
Constellation committed May 8, 2024
1 parent 77eafb2 commit b8454d3
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 59 deletions.
29 changes: 21 additions & 8 deletions Source/JavaScriptCore/bytecode/StructureStubInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -336,12 +336,17 @@ void StructureStubInfo::reset(const ConcurrentJSLockerBase& locker, CodeBlock* c
template<typename Visitor>
void StructureStubInfo::visitAggregateImpl(Visitor& visitor)
{
{
if (!m_identifier) {
Locker locker { m_bufferedStructuresLock };
for (auto& bufferedStructure : m_bufferedStructures)
bufferedStructure.byValId().visitAggregate(visitor);
}
m_identifier.visitAggregate(visitor);
WTF::switchOn(m_bufferedStructures,
[&](std::monostate) { },
[&](Vector<StructureID>&) { },
[&](Vector<std::tuple<StructureID, CacheableIdentifier>>& structures) {
for (auto& [bufferedStructureID, bufferedCacheableIdentifier] : structures)
bufferedCacheableIdentifier.visitAggregate(visitor);
});
} else
m_identifier.visitAggregate(visitor);
switch (m_cacheType) {
case CacheType::Unset:
case CacheType::ArrayLength:
Expand All @@ -366,9 +371,17 @@ void StructureStubInfo::visitWeakReferences(const ConcurrentJSLockerBase& locker
VM& vm = codeBlock->vm();
{
Locker locker { m_bufferedStructuresLock };
m_bufferedStructures.removeIf(
[&] (auto& entry) -> bool {
return !vm.heap.isMarked(entry.structure());
WTF::switchOn(m_bufferedStructures,
[&](std::monostate) { },
[&](Vector<StructureID>& structures) {
structures.removeAllMatching([&](StructureID structureID) {
return !vm.heap.isMarked(structureID.decode());
});
},
[&](Vector<std::tuple<StructureID, CacheableIdentifier>>& structures) {
structures.removeAllMatching([&](auto& tuple) {
return !vm.heap.isMarked(std::get<0>(tuple).decode());
});
});
}

Expand Down
86 changes: 35 additions & 51 deletions Source/JavaScriptCore/bytecode/StructureStubInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -297,9 +297,34 @@ class StructureStubInfo {
// the base's structure. That seems unlikely for the canonical use of instanceof, where
// the prototype is fixed.
bool isNewlyAdded = false;
StructureID structureID = structure->id();
{
Locker locker { m_bufferedStructuresLock };
isNewlyAdded = m_bufferedStructures.add({ structure, impl }).isNewEntry;
if (std::holds_alternative<std::monostate>(m_bufferedStructures)) {
if (m_identifier)
m_bufferedStructures = Vector<StructureID>();
else
m_bufferedStructures = Vector<std::tuple<StructureID, CacheableIdentifier>>();
}
WTF::switchOn(m_bufferedStructures,
[&](std::monostate) { },
[&](Vector<StructureID>& structures) {
for (auto bufferedStructureID : structures) {
if (bufferedStructureID == structureID)
return;
}
structures.append(structureID);
isNewlyAdded = true;
},
[&](Vector<std::tuple<StructureID, CacheableIdentifier>>& structures) {
ASSERT(!m_identifier);
for (auto& [bufferedStructureID, bufferedCacheableIdentifier] : structures) {
if (bufferedStructureID == structureID && bufferedCacheableIdentifier == impl)
return;
}
structures.append(std::tuple { structureID, impl });
isNewlyAdded = true;
});
}
if (isNewlyAdded)
vm.writeBarrier(codeBlock);
Expand All @@ -314,57 +339,16 @@ class StructureStubInfo {
void clearBufferedStructures()
{
Locker locker { m_bufferedStructuresLock };
m_bufferedStructures.clear();
WTF::switchOn(m_bufferedStructures,
[&](std::monostate) { },
[&](Vector<StructureID>& structures) {
structures.shrink(0);
},
[&](Vector<std::tuple<StructureID, CacheableIdentifier>>& structures) {
structures.shrink(0);
});
}

class BufferedStructure {
public:
static constexpr uintptr_t hashTableDeletedValue = 0x2;
BufferedStructure() = default;
BufferedStructure(Structure* structure, CacheableIdentifier byValId)
: m_structure(structure)
, m_byValId(byValId)
{ }
BufferedStructure(WTF::HashTableDeletedValueType)
: m_structure(bitwise_cast<Structure*>(hashTableDeletedValue))
{ }

bool isHashTableDeletedValue() const { return bitwise_cast<uintptr_t>(m_structure) == hashTableDeletedValue; }

unsigned hash() const
{
unsigned hash = PtrHash<Structure*>::hash(m_structure);
if (m_byValId)
hash += m_byValId.hash();
return hash;
}

friend bool operator==(const BufferedStructure&, const BufferedStructure&) = default;

struct Hash {
static unsigned hash(const BufferedStructure& key)
{
return key.hash();
}

static bool equal(const BufferedStructure& a, const BufferedStructure& b)
{
return a == b;
}

static constexpr bool safeToCompareToEmptyOrDeleted = false;
};
using KeyTraits = SimpleClassHashTraits<BufferedStructure>;
static_assert(KeyTraits::emptyValueIsZero, "Structure* and CacheableIdentifier are empty if they are zero-initialized");

Structure* structure() const { return m_structure; }
const CacheableIdentifier& byValId() const { return m_byValId; }

private:
Structure* m_structure { nullptr };
CacheableIdentifier m_byValId;
};

void replaceHandler(CodeBlock*, Ref<InlineCacheHandler>&&);
void rewireStubAsJumpInAccess(CodeBlock*, InlineCacheHandler&);

Expand Down Expand Up @@ -431,7 +415,7 @@ class StructureStubInfo {
// Note that it's always safe to clear this. If we clear it prematurely, then if we see the same
// structure again during this buffering countdown, we will create an AccessCase object for it.
// That's not so bad - we'll get rid of the redundant ones once we regenerate.
HashSet<BufferedStructure, BufferedStructure::Hash, BufferedStructure::KeyTraits> m_bufferedStructures WTF_GUARDED_BY_LOCK(m_bufferedStructuresLock);
std::variant<std::monostate, Vector<StructureID>, Vector<std::tuple<StructureID, CacheableIdentifier>>> m_bufferedStructures WTF_GUARDED_BY_LOCK(m_bufferedStructuresLock);
public:

ScalarRegisterSet usedRegisters;
Expand Down

0 comments on commit b8454d3

Please sign in to comment.