Skip to content

Commit e672576

Browse files
ConstellationConstellation
committed
[JSC] Start using limited variant of Handler IC
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
1 parent d1d9950 commit e672576

9 files changed

Lines changed: 139 additions & 283 deletions

File tree

‎Source/JavaScriptCore/bytecode/AccessCase.cpp‎

Lines changed: 39 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -295,14 +295,24 @@ RefPtr<AccessCase> AccessCase::fromStructureStubInfo(
295295
}
296296
}
297297

298-
bool AccessCase::hasAlternateBaseImpl() const
298+
JSObject* AccessCase::tryGetAlternateBaseImpl() const
299299
{
300-
return !conditionSet().isEmpty();
301-
}
302-
303-
JSObject* AccessCase::alternateBaseImpl() const
304-
{
305-
return conditionSet().slotBaseCondition().object();
300+
switch (m_type) {
301+
case AccessCase::Getter:
302+
case AccessCase::Setter:
303+
case AccessCase::CustomValueGetter:
304+
case AccessCase::CustomAccessorGetter:
305+
case AccessCase::CustomValueSetter:
306+
case AccessCase::CustomAccessorSetter:
307+
case AccessCase::IntrinsicGetter:
308+
case AccessCase::Load:
309+
case AccessCase::GetGetter:
310+
if (!conditionSet().isEmpty())
311+
return conditionSet().slotBaseCondition().object();
312+
return nullptr;
313+
default:
314+
return nullptr;
315+
}
306316
}
307317

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

14031413
bool AccessCase::canBeShared(const AccessCase& lhs, const AccessCase& rhs)
14041414
{
1405-
// And we say "false" if either of them have m_polyProtoAccessChain.
1406-
if (lhs.m_polyProtoAccessChain || rhs.m_polyProtoAccessChain)
1407-
return false;
1408-
if (lhs.additionalSet() || rhs.additionalSet())
1409-
return false;
1410-
14111415
if (lhs.m_type != rhs.m_type)
14121416
return false;
14131417
if (lhs.m_offset != rhs.m_offset)
@@ -1420,6 +1424,17 @@ bool AccessCase::canBeShared(const AccessCase& lhs, const AccessCase& rhs)
14201424
return false;
14211425
if (lhs.m_conditionSet != rhs.m_conditionSet)
14221426
return false;
1427+
if (lhs.additionalSet() != rhs.additionalSet())
1428+
return false;
1429+
if (lhs.m_polyProtoAccessChain || rhs.m_polyProtoAccessChain) {
1430+
if (!lhs.m_polyProtoAccessChain || !rhs.m_polyProtoAccessChain)
1431+
return false;
1432+
if (*lhs.m_polyProtoAccessChain != *rhs.m_polyProtoAccessChain)
1433+
return false;
1434+
}
1435+
1436+
if (lhs.tryGetAlternateBase() != rhs.tryGetAlternateBase())
1437+
return false;
14231438

14241439
switch (lhs.m_type) {
14251440
case Load:
@@ -1521,6 +1536,15 @@ bool AccessCase::canBeShared(const AccessCase& lhs, const AccessCase& rhs)
15211536
case InstanceOfGeneric:
15221537
return true;
15231538

1539+
case CustomValueGetter:
1540+
case CustomAccessorGetter:
1541+
case CustomValueSetter:
1542+
case CustomAccessorSetter: {
1543+
auto& lhsd = lhs.as<GetterSetterAccessCase>();
1544+
auto& rhsd = rhs.as<GetterSetterAccessCase>();
1545+
return lhsd.m_customAccessor == rhsd.m_customAccessor;
1546+
}
1547+
15241548
case Getter:
15251549
case Setter:
15261550
case ProxyObjectHas:
@@ -1531,14 +1555,6 @@ bool AccessCase::canBeShared(const AccessCase& lhs, const AccessCase& rhs)
15311555
return false;
15321556
}
15331557

1534-
case CustomValueGetter:
1535-
case CustomAccessorGetter:
1536-
case CustomValueSetter:
1537-
case CustomAccessorSetter: {
1538-
// They are embedding JSGlobalObject that are not tied to sharing JITStubRoutine.
1539-
return false;
1540-
}
1541-
15421558
case IntrinsicGetter: {
15431559
auto& lhsd = lhs.as<IntrinsicGetterAccessCase>();
15441560
auto& rhsd = rhs.as<IntrinsicGetterAccessCase>();
@@ -1590,20 +1606,11 @@ WatchpointSet* AccessCase::additionalSet() const
15901606
return result;
15911607
}
15921608

1593-
bool AccessCase::hasAlternateBase() const
1594-
{
1595-
bool result = false;
1596-
const_cast<AccessCase*>(this)->runWithDowncast([&](auto* accessCase) {
1597-
result = accessCase->hasAlternateBaseImpl();
1598-
});
1599-
return result;
1600-
}
1601-
1602-
JSObject* AccessCase::alternateBase() const
1609+
JSObject* AccessCase::tryGetAlternateBase() const
16031610
{
16041611
JSObject* result = nullptr;
16051612
const_cast<AccessCase*>(this)->runWithDowncast([&](auto* accessCase) {
1606-
result = accessCase->alternateBaseImpl();
1613+
result = accessCase->tryGetAlternateBaseImpl();
16071614
});
16081615
return result;
16091616
}

‎Source/JavaScriptCore/bytecode/AccessCase.h‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -219,9 +219,8 @@ class AccessCase : public ThreadSafeRefCounted<AccessCase> {
219219

220220
ObjectPropertyConditionSet conditionSet() const { return m_conditionSet; }
221221

222-
bool hasAlternateBase() const;
223-
JSObject* alternateBase() const;
224-
222+
JSObject* tryGetAlternateBase() const;
223+
225224
WatchpointSet* additionalSet() const;
226225
bool viaGlobalProxy() const { return m_viaGlobalProxy; }
227226

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

282281
UniquedStringImpl* uid() const { return m_identifier.uid(); }
283282
CacheableIdentifier identifier() const { return m_identifier; }
284-
void updateIdentifier(CacheableIdentifier identifier)
285-
{
286-
m_identifier = identifier;
287-
}
288283

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

331326
Ref<AccessCase> cloneImpl() const;
332327
WatchpointSet* additionalSetImpl() const { return nullptr; }
333-
bool hasAlternateBaseImpl() const;
328+
JSObject* tryGetAlternateBaseImpl() const;
334329
void dumpImpl(PrintStream&, CommaPrinter&, Indenter&) const { }
335-
JSObject* alternateBaseImpl() const;
336330

337331
bool guardedByStructureCheckSkippingConstantIdentifierCheck() const;
338332

‎Source/JavaScriptCore/bytecode/GetByStatus.cpp‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,9 @@ GetByStatus GetByStatus::computeForStubInfoWithoutExitSiteFeedback(const Concurr
332332
if (!conditionSet.isStillValid())
333333
continue;
334334

335-
Structure* currStructure = access.hasAlternateBase() ? access.alternateBase()->structure() : access.structure();
335+
Structure* currStructure = access.structure();
336+
if (auto* object = access.tryGetAlternateBase())
337+
currStructure = object->structure();
336338
// For now, we only support cases which JSGlobalObject is the same to the currently profiledBlock.
337339
if (currStructure->globalObject() != profiledBlock->globalObject())
338340
return GetByStatus(JSC::slowVersion(summary), stubInfo);

‎Source/JavaScriptCore/bytecode/GetterSetterAccessCase.cpp‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -75,18 +75,11 @@ Ref<AccessCase> GetterSetterAccessCase::cloneImpl() const
7575
return adoptRef(*new GetterSetterAccessCase(*this));
7676
}
7777

78-
bool GetterSetterAccessCase::hasAlternateBaseImpl() const
78+
JSObject* GetterSetterAccessCase::tryGetAlternateBaseImpl() const
7979
{
80-
if (customSlotBase())
81-
return true;
82-
return Base::hasAlternateBaseImpl();
83-
}
84-
85-
JSObject* GetterSetterAccessCase::alternateBaseImpl() const
86-
{
87-
if (customSlotBase())
88-
return customSlotBase();
89-
return Base::alternateBaseImpl();
80+
if (auto* object = customSlotBase())
81+
return object;
82+
return Base::tryGetAlternateBaseImpl();
9083
}
9184

9285
void GetterSetterAccessCase::dumpImpl(PrintStream& out, CommaPrinter& comma, Indenter& indent) const

‎Source/JavaScriptCore/bytecode/GetterSetterAccessCase.h‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,7 @@ class GetterSetterAccessCase final : public ProxyableAccessCase {
6565

6666
GetterSetterAccessCase(const GetterSetterAccessCase&);
6767

68-
bool hasAlternateBaseImpl() const;
69-
JSObject* alternateBaseImpl() const;
68+
JSObject* tryGetAlternateBaseImpl() const;
7069
void dumpImpl(PrintStream&, CommaPrinter&, Indenter&) const;
7170
Ref<AccessCase> cloneImpl() const;
7271

0 commit comments

Comments
 (0)