Skip to content

Commit 52a9721

Browse files
ConstellationConstellation
committed
[JSC] Avoid repeated lock contension due to NativeCalleeRegistry registration
https://bugs.webkit.org/show_bug.cgi?id=312082 rdar://174588184 Reviewed by Keith Miller. We found that IPIntCallee's registration is taking huge lock contension. But its entrypoint generation is no longer costly as we use the shared thunk. We should avoid doing these registration from the multiple thread concurrently, and do them in a batched style when finalizing all callees at the end. Also we should have Box<PCToCodeOriginMap> in OptimizingJITCallee instead of having it in NativeCalleeRegistry. * Source/JavaScriptCore/runtime/NativeCalleeRegistry.h: (JSC::NativeCalleeRegistry::registerCallees): (JSC::NativeCalleeRegistry::unregisterCallee): (JSC::NativeCalleeRegistry::addPCToCodeOriginMap): Deleted. * Source/JavaScriptCore/runtime/SamplingProfiler.cpp: (JSC::FrameWalker::recordJITFrame): * Source/JavaScriptCore/wasm/WasmBBQPlan.cpp: (JSC::Wasm::BBQPlan::work): * Source/JavaScriptCore/wasm/WasmCallee.cpp: (JSC::Wasm::Callee::pcToCodeOriginMap const): (JSC::Wasm::IPIntCallee::setEntrypointWithoutRegistration): * Source/JavaScriptCore/wasm/WasmCallee.h: (JSC::Wasm::Callee::pcToCodeOriginMapImpl const): (JSC::Wasm::OptimizingJITCallee::pcToCodeOriginMapImpl const): (JSC::Wasm::OptimizingJITCallee::setPCToCodeOriginMap): * Source/JavaScriptCore/wasm/WasmIPIntPlan.cpp: (JSC::Wasm::IPIntPlan::compileFunction): (JSC::Wasm::IPIntPlan::didCompleteCompilation): * Source/JavaScriptCore/wasm/WasmOMGPlan.cpp: (JSC::Wasm::OMGPlan::work): * Source/JavaScriptCore/wasm/WasmOSREntryPlan.cpp: (JSC::Wasm::OSREntryPlan::work): Canonical link: https://commits.webkit.org/311037@main
1 parent 203b7fc commit 52a9721

8 files changed

Lines changed: 56 additions & 38 deletions

File tree

‎Source/JavaScriptCore/runtime/NativeCalleeRegistry.h‎

Lines changed: 10 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,6 @@
2525

2626
#pragma once
2727

28-
#include <JavaScriptCore/PCToCodeOriginMap.h>
29-
#include <wtf/Box.h>
3028
#include <wtf/HashSet.h>
3129
#include <wtf/Lock.h>
3230
#include <wtf/TZoneMalloc.h>
@@ -51,13 +49,20 @@ class NativeCalleeRegistry {
5149
ASSERT_UNUSED(addResult, addResult.isNewEntry);
5250
}
5351

52+
template<typename CalleeContainer>
53+
void registerCallees(const CalleeContainer& callees)
54+
{
55+
Locker locker { m_lock };
56+
for (auto& callee : callees) {
57+
auto addResult = m_calleeSet.add(callee.ptr());
58+
ASSERT_UNUSED(addResult, addResult.isNewEntry);
59+
}
60+
}
61+
5462
void unregisterCallee(NativeCallee* callee)
5563
{
5664
Locker locker { m_lock };
5765
m_calleeSet.remove(callee);
58-
#if ENABLE(JIT)
59-
m_pcToCodeOriginMaps.remove(callee);
60-
#endif
6166
}
6267

6368
const UncheckedKeyHashSet<NativeCallee*>& allCallees() WTF_REQUIRES_LOCK(m_lock)
@@ -72,33 +77,11 @@ class NativeCalleeRegistry {
7277
return m_calleeSet.contains(callee);
7378
}
7479

75-
#if ENABLE(JIT)
76-
void addPCToCodeOriginMap(NativeCallee* callee, Box<PCToCodeOriginMap> originMap)
77-
{
78-
Locker locker { m_lock };
79-
ASSERT(isValidCallee(callee));
80-
auto addResult = m_pcToCodeOriginMaps.add(callee, WTF::move(originMap));
81-
RELEASE_ASSERT(addResult.isNewEntry);
82-
}
83-
84-
Box<PCToCodeOriginMap> codeOriginMap(NativeCallee* callee) WTF_REQUIRES_LOCK(m_lock)
85-
{
86-
ASSERT(isValidCallee(callee));
87-
auto iter = m_pcToCodeOriginMaps.find(callee);
88-
if (iter != m_pcToCodeOriginMaps.end())
89-
return iter->value;
90-
return nullptr;
91-
}
92-
#endif
93-
9480
NativeCalleeRegistry() = default;
9581

9682
private:
9783
Lock m_lock;
9884
UncheckedKeyHashSet<NativeCallee*> m_calleeSet WTF_GUARDED_BY_LOCK(m_lock);
99-
#if ENABLE(JIT)
100-
UncheckedKeyHashMap<NativeCallee*, Box<PCToCodeOriginMap>> m_pcToCodeOriginMaps WTF_GUARDED_BY_LOCK(m_lock);
101-
#endif
10285
};
10386

10487
} // namespace JSC

‎Source/JavaScriptCore/runtime/SamplingProfiler.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ class FrameWalker {
152152
if (isInlined)
153153
stackTrace[m_depth].wasmIndexOrName = origin;
154154
}
155-
stackTrace[m_depth].wasmPCMap = NativeCalleeRegistry::singleton().codeOriginMap(wasmCallee);
155+
stackTrace[m_depth].wasmPCMap = wasmCallee->pcToCodeOriginMap();
156156
#endif
157157
#endif
158158
break;

‎Source/JavaScriptCore/wasm/WasmBBQPlan.cpp‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131
#include "JITCompilation.h"
3232
#include "JSToWasm.h"
3333
#include "LinkBuffer.h"
34-
#include "NativeCalleeRegistry.h"
3534
#include "WasmBBQJIT.h"
3635
#include "WasmCallee.h"
3736
#include "WasmCalleeGroup.h"
@@ -126,8 +125,10 @@ void BBQPlan::work()
126125
callee->setEntrypoint(WTF::move(function->entrypoint), WTF::move(unlinkedWasmToWasmCalls), WTF::move(function->stackmaps), WTF::move(function->exceptionHandlers), WTF::move(exceptionHandlerLocations), WTF::move(loopEntrypointLocations), sharedLoopEntrypoint, function->osrEntryScratchBufferSize);
127126
entrypoint = callee->entrypoint();
128127

129-
if (context.pcToCodeOriginMap)
130-
NativeCalleeRegistry::singleton().addPCToCodeOriginMap(callee.ptr(), WTF::move(context.pcToCodeOriginMap));
128+
if (context.pcToCodeOriginMap) {
129+
WTF::storeStoreFence();
130+
callee->setPCToCodeOriginMap(WTF::move(context.pcToCodeOriginMap));
131+
}
131132

132133
{
133134
Locker locker { m_calleeGroup->m_lock };

‎Source/JavaScriptCore/wasm/WasmCallee.cpp‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,17 @@ std::tuple<void*, void*> Callee::range() const
183183
return result;
184184
}
185185

186+
#if ENABLE(JIT)
187+
Box<PCToCodeOriginMap> Callee::pcToCodeOriginMap() const
188+
{
189+
Box<PCToCodeOriginMap> result;
190+
runWithDowncast([&](auto* derived) {
191+
result = derived->pcToCodeOriginMapImpl();
192+
});
193+
return result;
194+
}
195+
#endif
196+
186197
const RegisterAtOffsetList* Callee::calleeSaveRegisters()
187198
{
188199
const RegisterAtOffsetList* result = nullptr;
@@ -304,6 +315,12 @@ void IPIntCallee::setEntrypoint(CodePtr<WasmEntryPtrTag> entrypoint)
304315
NativeCalleeRegistry::singleton().registerCallee(this);
305316
}
306317

318+
void IPIntCallee::setEntrypointWithoutRegistration(CodePtr<WasmEntryPtrTag> entrypoint)
319+
{
320+
ASSERT(!m_entrypoint);
321+
m_entrypoint = entrypoint;
322+
}
323+
307324
const RegisterAtOffsetList* IPIntCallee::calleeSaveRegistersImpl()
308325
{
309326
ASSERT(RegisterAtOffsetList::ipintCalleeSaveRegisters().registerCount() == numberOfIPIntCalleeSaveRegisters);

‎Source/JavaScriptCore/wasm/WasmCallee.h‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,10 @@ class Callee : public NativeCallee {
7676
// Used by Wasm's fault signal handler to determine if the fault came from Wasm.
7777
std::tuple<void*, void*> range() const;
7878

79+
#if ENABLE(JIT)
80+
Box<PCToCodeOriginMap> pcToCodeOriginMap() const;
81+
#endif
82+
7983
const HandlerInfo* handlerForIndex(JSWebAssemblyInstance&, unsigned, const Tag*);
8084

8185
bool hasExceptionHandlers() const { return !m_exceptionHandlers.isEmpty(); }
@@ -93,6 +97,10 @@ class Callee : public NativeCallee {
9397
return 0;
9498
}
9599

100+
#if ENABLE(JIT)
101+
Box<PCToCodeOriginMap> pcToCodeOriginMapImpl() const { return nullptr; }
102+
#endif
103+
96104
protected:
97105
JS_EXPORT_PRIVATE Callee(Wasm::CompilationMode);
98106
JS_EXPORT_PRIVATE Callee(Wasm::CompilationMode, FunctionSpaceIndex, std::pair<const Name*, RefPtr<NameSection>>&&);
@@ -258,6 +266,9 @@ class OptimizingJITCallee : public JITCallee {
258266

259267
Box<PCToCodeOriginMap> materializePCToOriginMap(B3::PCToOriginMap&&, LinkBuffer&);
260268

269+
Box<PCToCodeOriginMap> pcToCodeOriginMapImpl() const { return m_pcToCodeOriginMap; }
270+
void setPCToCodeOriginMap(Box<PCToCodeOriginMap>&& map) { m_pcToCodeOriginMap = WTF::move(map); }
271+
261272
unsigned computeCodeHashImpl() const;
262273

263274
protected:
@@ -283,6 +294,7 @@ class OptimizingJITCallee : public JITCallee {
283294
Vector<WasmCodeOrigin, 0> codeOrigins;
284295
Vector<Ref<NameSection>, 0> nameSections;
285296
Box<PCToCodeOriginMap> m_callSiteIndexMap;
297+
Box<PCToCodeOriginMap> m_pcToCodeOriginMap;
286298
const Ref<IPIntCallee> m_profiledCallee;
287299
};
288300

@@ -445,6 +457,7 @@ class IPIntCallee final : public Callee {
445457

446458
FunctionCodeIndex functionIndex() const { return m_functionIndex; }
447459
void setEntrypoint(CodePtr<WasmEntryPtrTag>);
460+
void setEntrypointWithoutRegistration(CodePtr<WasmEntryPtrTag>);
448461
const uint8_t* bytecode() const { return m_bytecode; }
449462
const uint8_t* bytecodeEnd() const { return m_bytecodeEnd; }
450463
const uint8_t* metadata() const LIFETIME_BOUND { return m_metadata.span().data(); }

‎Source/JavaScriptCore/wasm/WasmIPIntPlan.cpp‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
#include "LLIntData.h"
3737
#include "LLIntThunks.h"
3838
#include "LinkBuffer.h"
39+
#include "NativeCalleeRegistry.h"
3940
#include "WasmCallee.h"
4041
#include "WasmFunctionIPIntMetadataGenerator.h"
4142
#include "WasmIPIntGenerator.h"
@@ -149,7 +150,7 @@ void IPIntPlan::compileFunction(FunctionCodeIndex functionIndex)
149150
if (!entrypoint)
150151
entrypoint = LLInt::getCodeFunctionPtr<CFunctionPtrTag>(ipint_trampoline);
151152

152-
callee->setEntrypoint(entrypoint);
153+
callee->setEntrypointWithoutRegistration(entrypoint);
153154
ipintCallee = callee.ptr();
154155
m_calleesVector[functionIndex] = WTF::move(callee);
155156
} else
@@ -184,6 +185,7 @@ void IPIntPlan::didCompleteCompilation()
184185
unsigned functionCount = m_wasmInternalFunctions.size();
185186
if (!m_callees && functionCount) {
186187
m_callees = m_calleesVector.span().data();
188+
NativeCalleeRegistry::singleton().registerCallees(m_calleesVector);
187189
if (!m_moduleInformation->clobberingTailCalls().isEmpty())
188190
computeTransitiveTailCalls();
189191
}

‎Source/JavaScriptCore/wasm/WasmOMGPlan.cpp‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131

3232
#include "JITCompilation.h"
3333
#include "LinkBuffer.h"
34-
#include "NativeCalleeRegistry.h"
3534
#include "WasmCallee.h"
3635
#include "WasmFaultSignalHandler.h"
3736
#include "WasmIRGeneratorHelpers.h"
@@ -175,8 +174,10 @@ void OMGPlan::work()
175174
callee->setEntrypoint(WTF::move(omgEntrypoint), WTF::move(unlinkedCalls), WTF::move(internalFunction->stackmaps), WTF::move(internalFunction->exceptionHandlers), WTF::move(exceptionHandlerLocations));
176175
entrypoint = callee->entrypoint();
177176

178-
if (samplingProfilerMap)
179-
NativeCalleeRegistry::singleton().addPCToCodeOriginMap(callee.ptr(), WTF::move(samplingProfilerMap));
177+
if (samplingProfilerMap) {
178+
WTF::storeStoreFence();
179+
callee->setPCToCodeOriginMap(WTF::move(samplingProfilerMap));
180+
}
180181

181182
Locker locker { m_calleeGroup->m_lock };
182183
newlyInstalled = m_calleeGroup->installOptimizedCallee(locker, m_moduleInformation, m_functionIndex, callee.copyRef(), internalFunction->outgoingJITDirectCallees);

‎Source/JavaScriptCore/wasm/WasmOSREntryPlan.cpp‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030

3131
#include "JITCompilation.h"
3232
#include "LinkBuffer.h"
33-
#include "NativeCalleeRegistry.h"
3433
#include "WasmCallee.h"
3534
#include "WasmFaultSignalHandler.h"
3635
#include "WasmIRGeneratorHelpers.h"
@@ -144,8 +143,10 @@ void OSREntryPlan::work()
144143
ASSERT(m_calleeGroup.ptr() == m_module->calleeGroupFor(mode()));
145144
callee->setEntrypoint(WTF::move(omgEntrypoint), internalFunction->osrEntryScratchBufferSize, WTF::move(unlinkedCalls), WTF::move(internalFunction->stackmaps), WTF::move(internalFunction->exceptionHandlers), WTF::move(exceptionHandlerLocations));
146145

147-
if (samplingProfilerMap)
148-
NativeCalleeRegistry::singleton().addPCToCodeOriginMap(callee.ptr(), WTF::move(samplingProfilerMap));
146+
if (samplingProfilerMap) {
147+
WTF::storeStoreFence();
148+
callee->setPCToCodeOriginMap(WTF::move(samplingProfilerMap));
149+
}
149150

150151
{
151152
Locker locker { m_calleeGroup->m_lock };

0 commit comments

Comments
 (0)