Skip to content

Commit 29ac322

Browse files
kmiller68kmiller68
committed
[Wasm] Build JS Wrappers and JSToWasmCallees Lazily
https://bugs.webkit.org/show_bug.cgi?id=314910 rdar://177187656 Reviewed by Yusuke Suzuki. Wasm instantiation eagerly built two things that are commonly unused: 1. A WebAssemblyFunction (or WebAssemblyWrapperFunction) wrapper for every function listed in ModuleInformation::referencedFunctions() — the union of exports, ref.func targets, and element-segment entries. Many modules never expose most of those functions to JS. 2. A Wasm::JSToWasmCallee for every defined Wasm function, built inside IPIntPlan and stored in CalleeGroup::m_jsToWasmCallees. Most internal functions are never called from JS, so the thunk and frame-size computation were wasted. This patch makes both lazy. The resulting eagerness matrix is: | Function kind | Wrapper | JS->Wasm callee | |------------------------------------|------------------------------|-----------------| | Exported | eager (instantiation) | eager-on-create | | In an active element segment | eager (table-init at link) | eager-on-create | | Only via passive/dec element seg | lazy (table.init/table.copy) | eager-on-create | | Only via ref.func | lazy (ref.func execution) | eager-on-create | | Host import | lazy | n/a | Wrappers for non-exports are now materialized on demand by the new JSWebAssemblyInstance::ensureFunctionWrapper(FunctionSpaceIndex), which folds in the previous makeFunctionWrapper lambda from WebAssemblyModuleRecord. The eager loop over referencedFunctions() is replaced with a smaller exports-only loop in WebAssemblyModuleRecord. Active element segments still create wrappers at instantiation, but only for the functions they actually contain (via the table-init code path), not the precomputed union of all referenced functions. ref.func, table init/copy, const-expr evaluation, and the start-function path all now route through ensureFunctionWrapper. JSToWasmCallee construction is removed from IPIntPlan (takeJSToWasmCallees, ensureEntrypoint, m_entrypoints, m_jsToWasmCallees are gone). CalleeGroup::ensureJSToWasmCallee(const ModuleInformation&, FunctionSpaceIndex) creates and caches the JSToWasmCallee on first demand, guarded by a dedicated m_jsToWasmCalleesLock so it does not contend with the rest of CalleeGroup::m_lock. WebAssemblyFunction owns a RefPtr to the boxed callee and exposes ensureJSToWasmCallee, which is called from WebAssemblyFunction::create so the LLInt and JIT JS->Wasm entry trampolines (which read m_boxedJSToWasmCallee and m_frameSize directly via fixed offsets, bypassing callWebAssemblyFunction) always observe a populated value. WebAssemblyFunction is single-threaded so no extra locking is needed there. Also, remove the ModuleInformation::referencedFunctions() function and related BitVector. The referencedFunctions list was only used to determine which functions to create wrappers for. No new tests, no behavior change. Covered by existing tests. Canonical link: https://commits.webkit.org/313362@main
1 parent 0f6a356 commit 29ac322

18 files changed

Lines changed: 120 additions & 221 deletions

‎Source/JavaScriptCore/SaferCPPExpectations/MemoryUnsafeCastCheckerExpectations‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,6 @@ tools/CellProfile.h
130130
tools/HeapVerifier.cpp
131131
tools/JSDollarVM.cpp
132132
tools/VMInspector.cpp
133-
wasm/WasmCalleeGroup.cpp
134133
wasm/WasmModule.cpp
135134
wasm/WasmOperations.cpp
136135
wasm/WasmStreamingCompiler.cpp

‎Source/JavaScriptCore/SaferCPPExpectations/UncountedCallArgsCheckerExpectations‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,7 @@ wasm/js/JSWebAssemblyInstance.cpp
169169
wasm/js/JSWebAssemblyInstance.h
170170
wasm/js/JSWebAssemblyMemory.cpp
171171
wasm/js/JSWebAssemblyMemory.h
172+
wasm/js/WebAssemblyFunction.cpp
172173
wasm/js/WebAssemblyFunction.h
173174
wasm/js/WebAssemblyInstanceConstructor.cpp
174175
wasm/js/WebAssemblyModuleConstructor.cpp

‎Source/JavaScriptCore/SaferCPPExpectations/UncountedLocalVarsCheckerExpectations‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ runtime/TypeSet.cpp
102102
runtime/VMTraps.cpp
103103
runtime/WaiterListManager.h
104104
tools/JSDollarVM.cpp
105+
wasm/WasmCalleeGroup.cpp
105106
wasm/WasmBBQJIT.cpp
106107
wasm/WasmBBQJIT64.cpp
107108
wasm/WasmBBQPlan.cpp
@@ -125,4 +126,5 @@ wasm/js/JSWebAssembly.cpp
125126
wasm/js/JSWebAssemblyHelpers.h
126127
wasm/js/JSWebAssemblyInstance.cpp
127128
wasm/js/WasmToJS.cpp
129+
wasm/js/WebAssemblyFunction.cpp
128130
wasm/js/WebAssemblyModuleRecord.cpp

‎Source/JavaScriptCore/runtime/NativeCallee.h‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,14 +65,21 @@ class BoxedNativeCalleePtrTraits {
6565
using StorageType = CalleeBits;
6666
// Use an intermediate cast to uintptr_t to silence unsafe casting warning. It's locally "obvious" (other than the fact that RefPtr uses StorageType's constructor instead of a wrap) the
6767
// return value is of type T.
68-
static T* unwrap(const CalleeBits& calleeBits) { return reinterpret_cast<T*>(reinterpret_cast<uintptr_t>(calleeBits.asNativeCallee())); }
68+
static T* unwrap(const CalleeBits& calleeBits)
69+
{
70+
if (!calleeBits)
71+
return nullptr;
72+
return reinterpret_cast<T*>(reinterpret_cast<uintptr_t>(calleeBits.asNativeCallee()));
73+
}
6974
static T* exchange(CalleeBits& calleeBits, T* newCallee)
7075
{
7176
T* result = unwrap(calleeBits);
7277
calleeBits = newCallee;
7378
return result;
7479
}
7580

81+
static void swap(CalleeBits& a, CalleeBits& b) { std::swap(a, b); }
82+
7683
// FIXME: This isn't hashable since we don't have hashTableDeletedValue() or isHashTableDeletedValue()
7784
// but those probably shouldn't be hard to add if needed.
7885
};

‎Source/JavaScriptCore/wasm/WasmCalleeGroup.cpp‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
#include "WasmCallee.h"
3434
#include "WasmIPIntPlan.h"
3535
#include "WasmMachineThreads.h"
36+
#include "WasmModuleInformation.h"
3637
#include "WasmWorklist.h"
3738

3839
namespace JSC { namespace Wasm {
@@ -51,12 +52,15 @@ CalleeGroup::CalleeGroup(MemoryMode mode, const CalleeGroup& other)
5152
: m_calleeCount(other.m_calleeCount)
5253
, m_mode(mode)
5354
, m_ipintCallees(other.m_ipintCallees)
54-
, m_jsToWasmCallees(other.m_jsToWasmCallees)
5555
, m_callers(m_calleeCount)
5656
, m_wasmIndirectCallEntrypoints(other.m_wasmIndirectCallEntrypoints)
5757
, m_wasmIndirectCallWasmCallees(other.m_wasmIndirectCallWasmCallees)
5858
, m_wasmToWasmExitStubs(other.m_wasmToWasmExitStubs)
5959
{
60+
{
61+
Locker otherLocker { other.m_jsToWasmCalleesLock };
62+
m_jsToWasmCallees = other.m_jsToWasmCallees;
63+
}
6064
Locker locker { m_lock };
6165
setCompilationFinished();
6266
}
@@ -85,7 +89,6 @@ CalleeGroup::CalleeGroup(VM& vm, MemoryMode mode, ModuleInformation& moduleInfor
8589
}
8690

8791
m_wasmToWasmExitStubs = m_plan->takeWasmToWasmExitStubs();
88-
m_jsToWasmCallees = static_cast<IPIntPlan*>(m_plan.get())->takeJSToWasmCallees();
8992

9093
setCompilationFinished();
9194
})));
@@ -103,6 +106,23 @@ CalleeGroup::CalleeGroup(VM& vm, MemoryMode mode, ModuleInformation& moduleInfor
103106

104107
CalleeGroup::~CalleeGroup() = default;
105108

109+
JSToWasmCallee& CalleeGroup::ensureJSToWasmCallee(const ModuleInformation& moduleInformation, FunctionSpaceIndex functionIndexSpace)
110+
{
111+
ASSERT(runnable());
112+
ASSERT(functionIndexSpace >= functionImportCount());
113+
unsigned calleeIndex = functionIndexSpace - functionImportCount();
114+
115+
Locker locker { m_jsToWasmCalleesLock };
116+
auto addResult = m_jsToWasmCallees.ensure(calleeIndex, [&] {
117+
auto& ipintCallee = m_ipintCallees->at(calleeIndex).get();
118+
bool usesSIMD = moduleInformation.usesSIMD(FunctionCodeIndex(calleeIndex));
119+
auto callee = JSToWasmCallee::create(Ref<const RTT> { ipintCallee.signatureRTT() }, usesSIMD);
120+
callee->setWasmCallee(CalleeBits::encodeNativeCallee(&ipintCallee));
121+
return callee;
122+
});
123+
return *addResult.iterator->value;
124+
}
125+
106126
void CalleeGroup::waitUntilFinished()
107127
{
108128
RefPtr<Plan> plan;

‎Source/JavaScriptCore/wasm/WasmCalleeGroup.h‎

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -101,18 +101,7 @@ class CalleeGroup final : public ThreadSafeRefCounted<CalleeGroup> {
101101
return FunctionCodeIndex(spaceIndex - functionImportCount());
102102
}
103103

104-
// These two callee getters are only valid once the callees have been populated.
105-
106-
JSToWasmCallee& jsToWasmCalleeFromFunctionIndexSpace(FunctionSpaceIndex functionIndexSpace)
107-
{
108-
ASSERT(runnable());
109-
ASSERT(functionIndexSpace >= functionImportCount());
110-
unsigned calleeIndex = functionIndexSpace - functionImportCount();
111-
112-
auto callee = m_jsToWasmCallees.get(calleeIndex);
113-
RELEASE_ASSERT(callee);
114-
return *callee;
115-
}
104+
JSToWasmCallee& ensureJSToWasmCallee(const ModuleInformation&, FunctionSpaceIndex functionIndexSpace);
116105

117106
RefPtr<JITCallee> replacement(const AbstractLocker& locker, FunctionSpaceIndex functionIndexSpace) WTF_REQUIRES_LOCK(m_lock)
118107
{
@@ -266,7 +255,8 @@ class CalleeGroup final : public ThreadSafeRefCounted<CalleeGroup> {
266255
OptimizedCallees m_currentlyInstallingOptimizedCallees WTF_GUARDED_BY_LOCK(m_lock) { };
267256
FixedVector<OptimizedCallees> m_optimizedCallees WTF_GUARDED_BY_LOCK(m_lock);
268257
const Ref<IPIntCallees> m_ipintCallees;
269-
UncheckedKeyHashMap<uint32_t, RefPtr<JSToWasmCallee>, DefaultHash<uint32_t>, WTF::UnsignedWithZeroKeyHashTraits<uint32_t>> m_jsToWasmCallees;
258+
UncheckedKeyHashMap<uint32_t, RefPtr<JSToWasmCallee>, DefaultHash<uint32_t>, WTF::UnsignedWithZeroKeyHashTraits<uint32_t>> m_jsToWasmCallees WTF_GUARDED_BY_LOCK(m_jsToWasmCalleesLock);
259+
mutable Lock m_jsToWasmCalleesLock;
270260
#if ENABLE(WEBASSEMBLY_BBQJIT) || ENABLE(WEBASSEMBLY_OMGJIT)
271261
// FIXME: We should probably find some way to prune dead entries periodically.
272262
UncheckedKeyHashMap<uint32_t, ThreadSafeWeakPtr<OMGOSREntryCallee>, DefaultHash<uint32_t>, WTF::UnsignedWithZeroKeyHashTraits<uint32_t>> m_osrEntryCallees WTF_GUARDED_BY_LOCK(m_lock);

‎Source/JavaScriptCore/wasm/WasmConstExprGenerator.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -623,7 +623,7 @@ class ConstExprGenerator {
623623
[[nodiscard]] PartialResult addRefFunc(FunctionSpaceIndex index, ExpressionType& result)
624624
{
625625
if (m_mode == Mode::Evaluate) {
626-
JSValue wrapper = m_instance->getFunctionWrapper(index);
626+
JSValue wrapper = m_instance->ensureFunctionWrapper(index);
627627
ASSERT(!wrapper.isNull());
628628
ASSERT(wrapper.isObject());
629629
m_keepAlive.appendWithCrashOnOverflow(asObject(wrapper));

‎Source/JavaScriptCore/wasm/WasmFunctionParser.h‎

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -386,7 +386,6 @@ class FunctionParser : public Parser<void>, public FunctionParserTypes<typename
386386

387387
// FIXME add a macro as above for WASM_TRY_APPEND_TO_CONTROL_STACK https://bugs.webkit.org/show_bug.cgi?id=165862
388388

389-
void addReferencedFunctions(const Element&);
390389
[[nodiscard]] PartialResult parseArrayTypeDefinition(ASCIILiteral, bool, TypeSignatureIndex&, FieldType&, Type&);
391390
[[nodiscard]] PartialResult parseBlockSignatureAndNotifySIMDUseIfNeeded(BlockSignature&);
392391

@@ -1907,20 +1906,6 @@ auto FunctionParser<Context>::checkExpressionStack(const ControlType& controlDat
19071906
return { };
19081907
}
19091908

1910-
template<typename Context>
1911-
void FunctionParser<Context>::addReferencedFunctions(const Element& segment)
1912-
{
1913-
if (!isSubtype(segment.elementType, funcrefType()))
1914-
return;
1915-
1916-
// Add each function index as a referenced function. This ensures that
1917-
// wrappers will be created for GC.
1918-
for (uint32_t i = 0; i < segment.length(); i++) {
1919-
if (segment.initTypes[i] == Element::InitializationType::FromRefFunc)
1920-
m_info.addReferencedFunction(FunctionSpaceIndex(segment.initialBitsOrIndices[i]));
1921-
}
1922-
}
1923-
19241909
template<typename Context>
19251910
auto FunctionParser<Context>::parseArrayTypeDefinition(ASCIILiteral operation, bool isNullable, TypeSignatureIndex& typeIndex, FieldType& elementType, Type& arrayRefType) -> PartialResult
19261911
{
@@ -2608,10 +2593,6 @@ FOR_EACH_WASM_MEMORY_STORE_OP(CREATE_CASE)
26082593
WASM_VALIDATOR_FAIL_IF(storageType.is<PackedType>(), "type mismatch in array.new_elem: expected `funcref` or `externref`");
26092594

26102595
WASM_VALIDATOR_FAIL_IF(!isSubtype(elementsSegment.elementType, storageType.unpacked()), "type mismatch in array.new_elem: segment elements have type ", elementsSegment.elementType, " but array.new_elem operation expects elements of type ", storageType.unpacked());
2611-
// Create function wrappers for any functions in this element segment.
2612-
// We conservatively assume that the `array.new_canon_elem` instruction will be executed.
2613-
// An optimization would be to lazily create the wrappers when the array is initialized.
2614-
addReferencedFunctions(elementsSegment);
26152596

26162597
// Get the array size
26172598
TypedExpression size;
@@ -2770,7 +2751,6 @@ FOR_EACH_WASM_MEMORY_STORE_OP(CREATE_CASE)
27702751
WASM_VALIDATOR_FAIL_IF(dstFieldType.mutability != Mutability::Mutable, "array.init_elem index ", dstTypeIndex, " does not reference a mutable array definition");
27712752

27722753
WASM_VALIDATOR_FAIL_IF(!isSubtype(segmentElementType, unpackedElementType), "type mismatch in array.init_elem: segment elements have type ", segmentElementType, " but array.init_elem operation expects elements of type ", unpackedElementType);
2773-
addReferencedFunctions(elementsSegment);
27742754

27752755
TypedExpression dst, dstOffset, srcOffset, size;
27762756
WASM_TRY_POP_EXPRESSION_STACK_INTO(size, "array.init_elem"_s);
@@ -3117,7 +3097,6 @@ FOR_EACH_WASM_MEMORY_STORE_OP(CREATE_CASE)
31173097
// Function references don't need to be declared in constant expression contexts.
31183098
if constexpr (!std::is_same<Context, ConstExprGenerator>())
31193099
WASM_VALIDATOR_FAIL_IF(!m_info.isDeclaredFunction(index), "ref.func index "_s, index, " isn't declared"_s);
3120-
m_info.addReferencedFunction(index);
31213100

31223101
ExpressionType result;
31233102
WASM_TRY_ADD_TO_CONTEXT(addRefFunc(index, result));

‎Source/JavaScriptCore/wasm/WasmIPIntPlan.cpp‎

Lines changed: 0 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -78,10 +78,6 @@ bool IPIntPlan::prepareImpl()
7878
return false;
7979
m_wasmInternalFunctions.resize(functions.size());
8080

81-
if (!tryReserveCapacity(m_entrypoints, functions.size(), " WebAssembly functions"_s))
82-
return false;
83-
m_entrypoints.resize(functions.size());
84-
8581
if (!m_ipintCallees)
8682
m_ipintCallees = IPIntCallees::create(functions.size());
8783
return true;
@@ -112,7 +108,6 @@ void IPIntPlan::compileFunction(FunctionCodeIndex functionIndex)
112108

113109
m_wasmInternalFunctions[functionIndex] = WTF::move(*parseAndCompileResult);
114110

115-
IPIntCallee* ipintCallee = nullptr;
116111
{
117112
auto callee = IPIntCallee::create(*m_wasmInternalFunctions[functionIndex], functionIndexSpace, signature, m_moduleInformation->nameSection->get(functionIndexSpace));
118113
ASSERT(!callee->entrypoint());
@@ -136,30 +131,8 @@ void IPIntPlan::compileFunction(FunctionCodeIndex functionIndex)
136131
entrypoint = LLInt::getCodeFunctionPtr<CFunctionPtrTag>(ipint_trampoline);
137132

138133
callee->setEntrypointWithoutRegistration(entrypoint);
139-
ipintCallee = callee.ptr();
140134
m_ipintCallees->at(functionIndex) = WTF::move(callee);
141135
}
142-
143-
// If the function is exported via module, then we ensure JSToWasm entrypoint.
144-
if (m_compilerMode != CompilerMode::Validation) {
145-
if (m_exportedFunctionIndices.contains(functionIndex)) {
146-
if (!ensureEntrypoint(*ipintCallee, functionIndex)) {
147-
Locker locker { m_lock };
148-
Base::fail(makeString("JIT is disabled, but the entrypoint for "_s, functionIndex.rawIndex(), " requires JIT"_s));
149-
return;
150-
}
151-
}
152-
}
153-
154-
}
155-
156-
bool IPIntPlan::ensureEntrypoint(IPIntCallee&, FunctionCodeIndex functionIndex)
157-
{
158-
if (m_entrypoints[functionIndex])
159-
return true;
160-
161-
m_entrypoints[functionIndex] = JSToWasmCallee::create(Ref { m_moduleInformation->rtt(m_moduleInformation->internalFunctionTypeSignatureIndices[functionIndex]) }, m_moduleInformation->usesSIMD(functionIndex));
162-
return true;
163136
}
164137

165138
void IPIntPlan::didCompleteCompilation()
@@ -176,22 +149,6 @@ void IPIntPlan::didCompleteCompilation()
176149
if (m_compilerMode == CompilerMode::Validation)
177150
return;
178151

179-
for (uint32_t functionIndex = 0; functionIndex < m_moduleInformation->functions.size(); functionIndex++) {
180-
if (!m_entrypoints[functionIndex]) {
181-
const FunctionSpaceIndex functionIndexSpace = FunctionSpaceIndex(functionIndex + m_moduleInformation->importFunctionCount());
182-
if (m_exportedFunctionIndices.contains(functionIndex) || m_moduleInformation->hasReferencedFunction(functionIndexSpace)) {
183-
if (!ensureEntrypoint(m_ipintCallees->at(functionIndex).get(), FunctionCodeIndex(functionIndex))) {
184-
Base::fail(makeString("JIT is disabled, but the entrypoint for "_s, functionIndex, " requires JIT"_s));
185-
return;
186-
}
187-
}
188-
}
189-
if (auto& callee = m_entrypoints[functionIndex]) {
190-
callee->setWasmCallee(CalleeBits::encodeNativeCallee(&m_ipintCallees->at(functionIndex).get()));
191-
m_jsToWasmCallees.add(functionIndex, callee);
192-
}
193-
}
194-
195152
for (auto& unlinked : m_unlinkedWasmToWasmCalls) {
196153
for (auto& call : unlinked) {
197154
CodePtr<WasmEntryPtrTag> executableAddress;

‎Source/JavaScriptCore/wasm/WasmIPIntPlan.h‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,6 @@ namespace Wasm {
3737

3838
class IPIntCallee;
3939

40-
using JSToWasmCalleeMap = UncheckedKeyHashMap<uint32_t, RefPtr<JSToWasmCallee>, DefaultHash<uint32_t>, WTF::UnsignedWithZeroKeyHashTraits<uint32_t>>;
41-
42-
4340
class IPIntPlan final : public EntryPlan {
4441
using Base = EntryPlan;
4542

@@ -54,12 +51,6 @@ class IPIntPlan final : public EntryPlan {
5451
return m_ipintCallees.releaseNonNull();
5552
}
5653

57-
JSToWasmCalleeMap&& takeJSToWasmCallees()
58-
{
59-
RELEASE_ASSERT(!failed() && !hasWork());
60-
return WTF::move(m_jsToWasmCallees);
61-
}
62-
6354
bool hasWork() const final
6455
{
6556
// We'll use "Compiled" here to signify that IPInt has finished parsing and
@@ -81,13 +72,9 @@ class IPIntPlan final : public EntryPlan {
8172
bool prepareImpl() final;
8273
void didCompleteCompilation() WTF_REQUIRES_LOCK(m_lock) final;
8374

84-
bool ensureEntrypoint(IPIntCallee&, FunctionCodeIndex functionIndex);
85-
8675
Vector<std::unique_ptr<FunctionIPIntMetadataGenerator>> m_wasmInternalFunctions;
8776
RefPtr<IPIntCallees> m_ipintCallees;
8877
bool m_calleesAlreadyRegistered { false };
89-
Vector<RefPtr<JSToWasmCallee>> m_entrypoints;
90-
JSToWasmCalleeMap m_jsToWasmCallees;
9178
};
9279

9380
} } // namespace JSC::Wasm

0 commit comments

Comments
 (0)