Skip to content

Commit

Permalink
[JSC] Start using limited variant of Handler IC
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=273604
rdar://127402051

Reviewed by Keith Miller.

This patch enables limited variant of Handler IC. The limitation means,

1. Only enabled for Baseline JIT.
2. Getter and Setter are not supported yet.
3. We are caching entire code as an one handler. This is not the final form we would like to have.
   Next step is splitting them into one per AccessCase and chain them.
4. After (3) gets done, we would like to put more data into InlineCacheHandler itself so that code
   can be more and more sharable.

But even with this limited form, we are already observing good cache hit rate. So we take an approach starting with this,
and further extending Handler IC based on the above milestones.

We enable Handler IC, which is only enabled for Baseline JIT right now.
The IC is hash-consed via SharedJITStubSet. And InlineCacheCompiler first search for an already compiled stub, if it finds it,
we register watchpoint to this stub and use it without new compilation. If it is not found, we compile a new stub and register it to this table if possible.
When nobody uses this stub, then refCount becomes zero, and it automatically unregister itself from the table.
Each StructureStubInfo site's access cases is always subsumes stub's access cases. So GC will check validity via this StructureStubInfo's access cases, and
drop stub when it is no longer valid (as the same to the current IC).

* Source/JavaScriptCore/bytecode/AccessCase.cpp:
(JSC::AccessCase::canBeShared):
* Source/JavaScriptCore/bytecode/InlineCacheCompiler.cpp:
(JSC::InlineCacheCompiler::regenerate):
(JSC::InlineCacheHandler::visitWeak const):
(JSC::isMegamorphicById): Deleted.
* Source/JavaScriptCore/jit/GCAwareJITStubRoutine.cpp:
(JSC::PolymorphicAccessJITStubRoutine::addedToSharedJITStubSet):
* Source/JavaScriptCore/jit/GCAwareJITStubRoutine.h:
(JSC::PolymorphicAccessJITStubRoutine::isStillValid const):
* Source/JavaScriptCore/runtime/StructureID.h:

Canonical link: https://commits.webkit.org/278288@main
  • Loading branch information
Constellation committed May 2, 2024
1 parent d1d9950 commit e672576
Show file tree
Hide file tree
Showing 9 changed files with 139 additions and 283 deletions.
71 changes: 39 additions & 32 deletions Source/JavaScriptCore/bytecode/AccessCase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -295,14 +295,24 @@ RefPtr<AccessCase> AccessCase::fromStructureStubInfo(
}
}

bool AccessCase::hasAlternateBaseImpl() const
JSObject* AccessCase::tryGetAlternateBaseImpl() const
{
return !conditionSet().isEmpty();
}

JSObject* AccessCase::alternateBaseImpl() const
{
return conditionSet().slotBaseCondition().object();
switch (m_type) {
case AccessCase::Getter:
case AccessCase::Setter:
case AccessCase::CustomValueGetter:
case AccessCase::CustomAccessorGetter:
case AccessCase::CustomValueSetter:
case AccessCase::CustomAccessorSetter:
case AccessCase::IntrinsicGetter:
case AccessCase::Load:
case AccessCase::GetGetter:
if (!conditionSet().isEmpty())
return conditionSet().slotBaseCondition().object();
return nullptr;
default:
return nullptr;
}
}

Ref<AccessCase> AccessCase::cloneImpl() const
Expand Down Expand Up @@ -1402,12 +1412,6 @@ void AccessCase::checkConsistency(StructureStubInfo& stubInfo)

bool AccessCase::canBeShared(const AccessCase& lhs, const AccessCase& rhs)
{
// And we say "false" if either of them have m_polyProtoAccessChain.
if (lhs.m_polyProtoAccessChain || rhs.m_polyProtoAccessChain)
return false;
if (lhs.additionalSet() || rhs.additionalSet())
return false;

if (lhs.m_type != rhs.m_type)
return false;
if (lhs.m_offset != rhs.m_offset)
Expand All @@ -1420,6 +1424,17 @@ bool AccessCase::canBeShared(const AccessCase& lhs, const AccessCase& rhs)
return false;
if (lhs.m_conditionSet != rhs.m_conditionSet)
return false;
if (lhs.additionalSet() != rhs.additionalSet())
return false;
if (lhs.m_polyProtoAccessChain || rhs.m_polyProtoAccessChain) {
if (!lhs.m_polyProtoAccessChain || !rhs.m_polyProtoAccessChain)
return false;
if (*lhs.m_polyProtoAccessChain != *rhs.m_polyProtoAccessChain)
return false;
}

if (lhs.tryGetAlternateBase() != rhs.tryGetAlternateBase())
return false;

switch (lhs.m_type) {
case Load:
Expand Down Expand Up @@ -1521,6 +1536,15 @@ bool AccessCase::canBeShared(const AccessCase& lhs, const AccessCase& rhs)
case InstanceOfGeneric:
return true;

case CustomValueGetter:
case CustomAccessorGetter:
case CustomValueSetter:
case CustomAccessorSetter: {
auto& lhsd = lhs.as<GetterSetterAccessCase>();
auto& rhsd = rhs.as<GetterSetterAccessCase>();
return lhsd.m_customAccessor == rhsd.m_customAccessor;
}

case Getter:
case Setter:
case ProxyObjectHas:
Expand All @@ -1531,14 +1555,6 @@ bool AccessCase::canBeShared(const AccessCase& lhs, const AccessCase& rhs)
return false;
}

case CustomValueGetter:
case CustomAccessorGetter:
case CustomValueSetter:
case CustomAccessorSetter: {
// They are embedding JSGlobalObject that are not tied to sharing JITStubRoutine.
return false;
}

case IntrinsicGetter: {
auto& lhsd = lhs.as<IntrinsicGetterAccessCase>();
auto& rhsd = rhs.as<IntrinsicGetterAccessCase>();
Expand Down Expand Up @@ -1590,20 +1606,11 @@ WatchpointSet* AccessCase::additionalSet() const
return result;
}

bool AccessCase::hasAlternateBase() const
{
bool result = false;
const_cast<AccessCase*>(this)->runWithDowncast([&](auto* accessCase) {
result = accessCase->hasAlternateBaseImpl();
});
return result;
}

JSObject* AccessCase::alternateBase() const
JSObject* AccessCase::tryGetAlternateBase() const
{
JSObject* result = nullptr;
const_cast<AccessCase*>(this)->runWithDowncast([&](auto* accessCase) {
result = accessCase->alternateBaseImpl();
result = accessCase->tryGetAlternateBaseImpl();
});
return result;
}
Expand Down
12 changes: 3 additions & 9 deletions Source/JavaScriptCore/bytecode/AccessCase.h
Original file line number Diff line number Diff line change
Expand Up @@ -219,9 +219,8 @@ class AccessCase : public ThreadSafeRefCounted<AccessCase> {

ObjectPropertyConditionSet conditionSet() const { return m_conditionSet; }

bool hasAlternateBase() const;
JSObject* alternateBase() const;

JSObject* tryGetAlternateBase() const;

WatchpointSet* additionalSet() const;
bool viaGlobalProxy() const { return m_viaGlobalProxy; }

Expand Down Expand Up @@ -281,10 +280,6 @@ class AccessCase : public ThreadSafeRefCounted<AccessCase> {

UniquedStringImpl* uid() const { return m_identifier.uid(); }
CacheableIdentifier identifier() const { return m_identifier; }
void updateIdentifier(CacheableIdentifier identifier)
{
m_identifier = identifier;
}

#if ASSERT_ENABLED
void checkConsistency(StructureStubInfo&);
Expand Down Expand Up @@ -330,9 +325,8 @@ class AccessCase : public ThreadSafeRefCounted<AccessCase> {

Ref<AccessCase> cloneImpl() const;
WatchpointSet* additionalSetImpl() const { return nullptr; }
bool hasAlternateBaseImpl() const;
JSObject* tryGetAlternateBaseImpl() const;
void dumpImpl(PrintStream&, CommaPrinter&, Indenter&) const { }
JSObject* alternateBaseImpl() const;

bool guardedByStructureCheckSkippingConstantIdentifierCheck() const;

Expand Down
4 changes: 3 additions & 1 deletion Source/JavaScriptCore/bytecode/GetByStatus.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,9 @@ GetByStatus GetByStatus::computeForStubInfoWithoutExitSiteFeedback(const Concurr
if (!conditionSet.isStillValid())
continue;

Structure* currStructure = access.hasAlternateBase() ? access.alternateBase()->structure() : access.structure();
Structure* currStructure = access.structure();
if (auto* object = access.tryGetAlternateBase())
currStructure = object->structure();
// For now, we only support cases which JSGlobalObject is the same to the currently profiledBlock.
if (currStructure->globalObject() != profiledBlock->globalObject())
return GetByStatus(JSC::slowVersion(summary), stubInfo);
Expand Down
15 changes: 4 additions & 11 deletions Source/JavaScriptCore/bytecode/GetterSetterAccessCase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,18 +75,11 @@ Ref<AccessCase> GetterSetterAccessCase::cloneImpl() const
return adoptRef(*new GetterSetterAccessCase(*this));
}

bool GetterSetterAccessCase::hasAlternateBaseImpl() const
JSObject* GetterSetterAccessCase::tryGetAlternateBaseImpl() const
{
if (customSlotBase())
return true;
return Base::hasAlternateBaseImpl();
}

JSObject* GetterSetterAccessCase::alternateBaseImpl() const
{
if (customSlotBase())
return customSlotBase();
return Base::alternateBaseImpl();
if (auto* object = customSlotBase())
return object;
return Base::tryGetAlternateBaseImpl();
}

void GetterSetterAccessCase::dumpImpl(PrintStream& out, CommaPrinter& comma, Indenter& indent) const
Expand Down
3 changes: 1 addition & 2 deletions Source/JavaScriptCore/bytecode/GetterSetterAccessCase.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,7 @@ class GetterSetterAccessCase final : public ProxyableAccessCase {

GetterSetterAccessCase(const GetterSetterAccessCase&);

bool hasAlternateBaseImpl() const;
JSObject* alternateBaseImpl() const;
JSObject* tryGetAlternateBaseImpl() const;
void dumpImpl(PrintStream&, CommaPrinter&, Indenter&) const;
Ref<AccessCase> cloneImpl() const;

Expand Down
Loading

0 comments on commit e672576

Please sign in to comment.