Skip to content

perf(JSC): O(E+N) GetModuleNamespace; iterative InnerModuleLoading #200

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Jarred-Sumner merged 3 commits into from
Apr 26, 2026
Merged

perf(JSC): O(E+N) GetModuleNamespace; iterative InnerModuleLoading #200

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
013a712
perf(JSC): O(E+N) GetModuleNamespace; iterative InnerModuleLoading
Jarred-Sumner Apr 26, 2026
143fb22
drop unconditional resolveExportImpl cache read
Jarred-Sumner Apr 26, 2026
28f2acb
drop loadRequestedModules memoization
Jarred-Sumner Apr 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 76 additions & 34 deletions Source/JavaScriptCore/runtime/AbstractModuleRecord.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -802,36 +802,6 @@ auto AbstractModuleRecord::resolveExport(JSGlobalObject* globalObject, const Ide
return resolveExportImpl(globalObject, ResolveQuery(this, exportName.impl()));
}

static void getExportedNames(JSGlobalObject* globalObject, AbstractModuleRecord* root, IdentifierSet& exportedNames)
{
VM& vm = globalObject->vm();
auto scope = DECLARE_THROW_SCOPE(vm);

UncheckedKeyHashSet<AbstractModuleRecord*> exportStarSet;
Vector<AbstractModuleRecord*, 8> pendingModules;

pendingModules.append(root);

while (!pendingModules.isEmpty()) {
AbstractModuleRecord* moduleRecord = pendingModules.takeLast();
if (exportStarSet.contains(moduleRecord))
continue;
exportStarSet.add(moduleRecord);

for (const auto& pair : moduleRecord->exportEntries()) {
const AbstractModuleRecord::ExportEntry& exportEntry = pair.value;
if (moduleRecord == root || vm.propertyNames->defaultKeyword != exportEntry.exportName)
exportedNames.add(exportEntry.exportName.impl());
}

for (const auto& starModuleName : moduleRecord->starExportEntries()) {
AbstractModuleRecord* requestedModuleRecord = moduleRecord->hostResolveImportedModule(globalObject, Identifier::fromUid(vm, starModuleName.get()));
RETURN_IF_EXCEPTION(scope, void());
pendingModules.append(requestedModuleRecord);
}
}
}

JSModuleNamespaceObject* AbstractModuleRecord::getModuleNamespace(JSGlobalObject* globalObject, bool shouldPreventExtensions)
{
VM& vm = globalObject->vm();
Expand All @@ -846,12 +816,84 @@ JSModuleNamespaceObject* AbstractModuleRecord::getModuleNamespace(JSGlobalObject
if (m_moduleNamespaceObject)
return m_moduleNamespaceObject.get();

IdentifierSet exportedNames;
getExportedNames(globalObject, this, exportedNames);
RETURN_IF_EXCEPTION(scope, nullptr);
// GetModuleNamespace = GetExportedNames + ResolveExport per name. The naive
// implementation walks the star-export graph once per name. With N names and
// E star edges that is O(N * E), and large re-export trees (barrel files,
// `export * from`) make N and E both grow with the graph.
//
// Observation: ResolveExport(root, n, []) is fully determined by the set of
// explicit (Local / Namespace) bindings for n in the star-reachable graph.
// If exactly one module T binds n, the answer is always Resolved(T, ...).
// The spec's resolveSet only ever turns *extra* paths to T into null, and
// merge(Resolved(T), null) = Resolved(T). Only when two distinct bindings
// exist (potential shadowing / ambiguity) or an Indirect entry is involved
// does the path-sensitive walk matter.
//
// So: do a single traversal that (a) collects exported names and (b) records
// each name's unique binding. Names with one binding go straight into the
// namespace; the rest fall back to resolveExportImpl. This is O(E + N) for
// the common case.

Vector<std::pair<Identifier, Resolution>> resolutions;
for (auto& name : exportedNames) {
IdentifierSet slowPathNames;
{
UncheckedKeyHashSet<AbstractModuleRecord*> exportStarSet;
Vector<AbstractModuleRecord*, 8> pendingModules;
Resolutions uniqueBindings;

exportStarSet.add(this);
pendingModules.append(this);

while (!pendingModules.isEmpty()) {
AbstractModuleRecord* moduleRecord = pendingModules.takeLast();

for (const auto& pair : moduleRecord->exportEntries()) {
const ExportEntry& exportEntry = pair.value;
if (moduleRecord != this && vm.propertyNames->defaultKeyword == exportEntry.exportName)
continue;
RefPtr<UniquedStringImpl> name = pair.key;
if (slowPathNames.contains(name))
continue;

Resolution candidate;
if (exportEntry.type == ExportEntry::Type::Local)
candidate = Resolution { Resolution::Type::Resolved, moduleRecord, exportEntry.localName };
else if (exportEntry.type == ExportEntry::Type::Namespace) {
AbstractModuleRecord* importedModuleRecord = moduleRecord->hostResolveImportedModule(globalObject, exportEntry.moduleName);
RETURN_IF_EXCEPTION(scope, nullptr);
candidate = Resolution { Resolution::Type::Resolved, importedModuleRecord, vm.propertyNames->starNamespacePrivateName };
} else {
slowPathNames.add(WTF::move(name));
uniqueBindings.remove(pair.key);
continue;
}

auto addResult = uniqueBindings.add(name, candidate);
if (!addResult.isNewEntry) {
const Resolution& existing = addResult.iterator->value;
if (existing.moduleRecord != candidate.moduleRecord || existing.localName != candidate.localName) {
slowPathNames.add(WTF::move(name));
uniqueBindings.remove(pair.key);
}
}
}

for (const auto& starModuleName : moduleRecord->starExportEntries()) {
AbstractModuleRecord* requestedModuleRecord = moduleRecord->hostResolveImportedModule(globalObject, Identifier::fromUid(vm, starModuleName.get()));
RETURN_IF_EXCEPTION(scope, nullptr);
if (exportStarSet.add(requestedModuleRecord).isNewEntry)
pendingModules.append(requestedModuleRecord);
}
}

resolutions.reserveInitialCapacity(uniqueBindings.size() + slowPathNames.size());
for (auto& pair : uniqueBindings) {
cacheResolution(pair.key.get(), pair.value);
resolutions.append({ Identifier::fromUid(vm, pair.key.get()), pair.value });
}
}

for (auto& name : slowPathNames) {
Identifier ident = Identifier::fromUid(vm, name.get());
const Resolution resolution = resolveExport(globalObject, ident);
RETURN_IF_EXCEPTION(scope, nullptr);
Expand Down
171 changes: 112 additions & 59 deletions Source/JavaScriptCore/runtime/JSModuleLoader.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -792,74 +792,110 @@ JSPromise* JSModuleLoader::loadModule(JSGlobalObject* globalObject, const Module
return resultPromise;
}

static void checkSafeToRecurse(JSGlobalObject* globalObject, ThrowScope& scope)
{
if (!globalObject->vm().isSafeToRecurse())
throwRangeError(globalObject, scope, "Maximum call stack size exceeded"_s);
}

void JSModuleLoader::innerModuleLoading(JSGlobalObject* globalObject, ModuleGraphLoadingState *state, AbstractModuleRecord* module)
void JSModuleLoader::innerModuleLoading(JSGlobalObject* globalObject, ModuleGraphLoadingState *state, AbstractModuleRecord* startModule)
{
// InnerModuleLoading(state, module)
// https://tc39.es/ecma262/#sec-InnerModuleLoading
//
// The spec phrases this recursively, with HostLoadImportedModule re-entering
// via FinishLoadingImportedModule -> ContinueModuleLoading. That re-entry
// happens once per import edge, but only one-per-module actually enters the
// step-2 loop body — the rest hit the visited check, decrement the counter
// and return. With dense `export * from` graphs (E >> V) the per-edge
// function entry / throw-scope / dynamicDowncast dominates. We instead drain
// a worklist on the state: ContinueModuleLoading enqueues, and the outermost
// call drains. Same observable [[Visited]] / [[PendingModulesCount]].

VM& vm = globalObject->vm();
auto scope = DECLARE_THROW_SCOPE(vm);

// 1. Assert: state.[[IsLoading]] is true.
ASSERT(state->isLoading());
// 2. If module is a Cyclic Module Record, module.[[Status]] is NEW, and state.[[Visited]] does not contain module, then
if (auto* cyclic = dynamicDowncast<CyclicModuleRecord>(module); cyclic && cyclic->status() == CyclicModuleRecord::Status::New && !state->containsVisited(cyclic)) {
// 2.a. Append module to state.[[Visited]].
state->appendVisited(vm, cyclic);
// 2.b. Let requestedModulesCount be the number of elements in module.[[RequestedModules]].
size_t requestedModulesCount = module->requestedModules().size();
// 2.c. Set state.[[PendingModulesCount]] to state.[[PendingModulesCount]] + requestedModulesCount.
state->setPendingModulesCount(state->pendingModulesCount() + requestedModulesCount);
// 2.d. For each ModuleRequest Record request of module.[[RequestedModules]], do
for (const AbstractModuleRecord::ModuleRequest& request : module->requestedModules()) {
// 2.d.i. If AllImportAttributesSupported(request.[[Attributes]]) is false, then
// 2.d.i.1. Let error be ThrowCompletion(a newly created SyntaxError object).
// 2.d.i.2. Perform ContinueModuleLoading(state, error).
// (Not possible.)
// 2.d.ii. Else if module.[[LoadedModules]] contains a LoadedModuleRequest Record record such that ModuleRequestsEqual(record, request) is true, then
if (auto iter = module->loadedModules().find(ModuleMapKey { request.m_specifier.impl(), request.type() }); iter != module->loadedModules().end()) {
checkSafeToRecurse(globalObject, scope);
RETURN_IF_EXCEPTION(scope, void());
// 2.d.ii.1. Perform InnerModuleLoading(state, record.[[Module]]).
innerModuleLoading(globalObject, state, iter->value.m_module.get());
RETURN_IF_EXCEPTION(scope, void());
// 2.d.iii. Else,
} else {
// 2.d.iii.1. Perform HostLoadImportedModule(module, request, state.[[HostDefined]], state).
JSPromise* promise = hostLoadImportedModule(globalObject, cyclic, request, ModuleLoaderPayload::create(vm, state), state->scriptFetcher(), true);
RETURN_IF_EXCEPTION(scope, void());
promise->performPromiseThenWithInternalMicrotask(vm, globalObject, InternalMicrotask::ModuleGraphLoadingError, jsUndefined(), state);
// 2.d.iii.2. NOTE: HostLoadImportedModule will call FinishLoadingImportedModule, which re-enters the graph loading process through ContinueModuleLoading.
state->enqueueInnerLoad(startModule);
if (state->drainingInnerLoad())
RELEASE_AND_RETURN(scope, void());
state->setDrainingInnerLoad(true);

ModuleLoaderPayload* payload = nullptr;
AbstractModuleRecord* module;
while ((module = state->takeInnerLoad())) {
// 1. Assert: state.[[IsLoading]] is true.
ASSERT(state->isLoading());
// 2. If module is a Cyclic Module Record, module.[[Status]] is NEW, and state.[[Visited]] does not contain module, then
// (containsVisited first — it's the cheap pointer-hash check that fails
// fast for the common already-seen case before we pay for the dyncast.)
if (!state->containsVisited(module)) {
if (auto* cyclic = dynamicDowncast<CyclicModuleRecord>(module); cyclic && cyclic->status() == CyclicModuleRecord::Status::New) {
// 2.a. Append module to state.[[Visited]].
state->appendVisited(vm, cyclic);
// 2.b. Let requestedModulesCount be the number of elements in module.[[RequestedModules]].
size_t requestedModulesCount = module->requestedModules().size();
// 2.c. Set state.[[PendingModulesCount]] to state.[[PendingModulesCount]] + requestedModulesCount.
state->setPendingModulesCount(state->pendingModulesCount() + requestedModulesCount);
bool loadedModulesEmpty = module->loadedModules().isEmpty();
// 2.d. For each ModuleRequest Record request of module.[[RequestedModules]], do
for (const AbstractModuleRecord::ModuleRequest& request : module->requestedModules()) {
// 2.d.i. If AllImportAttributesSupported(request.[[Attributes]]) is false, then
// 2.d.i.1. Let error be ThrowCompletion(a newly created SyntaxError object).
// 2.d.i.2. Perform ContinueModuleLoading(state, error).
// (Not possible.)
// 2.d.ii. Else if module.[[LoadedModules]] contains a LoadedModuleRequest Record record such that ModuleRequestsEqual(record, request) is true, then
if (!loadedModulesEmpty) {
if (auto iter = module->loadedModules().find(ModuleMapKey { request.m_specifier.impl(), request.type() }); iter != module->loadedModules().end()) {
// 2.d.ii.1. Perform InnerModuleLoading(state, record.[[Module]]).
AbstractModuleRecord* loaded = iter->value.m_module.get();
if (state->containsVisited(loaded))
state->setPendingModulesCount(state->pendingModulesCount() - 1);
else
state->enqueueInnerLoad(loaded);
if (!state->isLoading())
break;
continue;
}
}
// 2.d.iii. Else,
// 2.d.iii.1. Perform HostLoadImportedModule(module, request, state.[[HostDefined]], state).
// The payload only carries `state`; one instance is sufficient for every
// request originating from this drain. Async loads stash it in their
// ModuleLoadingContext, but they all want the same state back.
if (!payload)
payload = ModuleLoaderPayload::create(vm, state);
JSPromise* promise = hostLoadImportedModule(globalObject, cyclic, request, payload, state->scriptFetcher(), true);
Comment on lines +841 to +861

@coderabbitai coderabbitai Bot Apr 26, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Keep [[LoadedModules]] descent depth-first.

This path now enqueues an already-loaded child and keeps iterating later sibling requests. Step 2.d.ii.1 used to descend into that child immediately, so this reorders descendant HostLoadImportedModule calls and synchronous failures relative to later siblings. A graph like [A_loaded, B] can now start B before a synchronous failure in A_loaded's subtree is observed, which changes host side effects and potentially first-error precedence. Use an explicit stack/frame so cached children are processed before advancing to the next sibling.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@Source/JavaScriptCore/runtime/JSModuleLoader.cpp` around lines 841 - 861, The
current code enqueues an already-loaded child via
state->enqueueInnerLoad(loaded) and then continues to the next sibling, which
breaks the required depth-first descent; change the logic in the block that
checks module->loadedModules() (ModuleMapKey lookup) to process cached children
immediately using an explicit stack or frame instead of enqueueing them: when
you find iter->value.m_module (loaded) and state->containsVisited(loaded) is
false, push or process that loaded AbstractModuleRecord* (the same work that
enqueueInnerLoad would trigger) before advancing to the next request so
descendant HostLoadImportedModule calls (hostLoadImportedModule) and synchronous
failures occur before siblings; preserve the state->isLoading() checks and the
existing payload creation (ModuleLoaderPayload::create) and only call
hostLoadImportedModule for requests after the cached-child stack/frame has been
drained.

if (scope.exception()) [[unlikely]] {
state->setDrainingInnerLoad(false);
return;
}
promise->performPromiseThenWithInternalMicrotask(vm, globalObject, InternalMicrotask::ModuleGraphLoadingError, jsUndefined(), state);
// 2.d.iii.2. NOTE: HostLoadImportedModule will call FinishLoadingImportedModule, which re-enters the graph loading process through ContinueModuleLoading.
// 2.d.iv. If state.[[IsLoading]] is false, return UNUSED.
if (!state->isLoading())
break;
}
if (!state->isLoading()) {
state->setDrainingInnerLoad(false);
RELEASE_AND_RETURN(scope, void());
}
}
// 2.d.iv. If state.[[IsLoading]] is false, return UNUSED.
if (!state->isLoading())
RELEASE_AND_RETURN(scope, void());
}
}
// 3. Assert: state.[[PendingModulesCount]] ≥ 1.
ASSERT(state->pendingModulesCount() >= 1);
// 4. Set state.[[PendingModulesCount]] to state.[[PendingModulesCount]] - 1.
state->setPendingModulesCount(state->pendingModulesCount() - 1);
// 5. If state.[[PendingModulesCount]] = 0, then
if (!state->pendingModulesCount()) {
// 5.a. Set state.[[IsLoading]] to false.
state->setIsLoading(false);
// 5.b. For each Cyclic Module Record loaded of state.[[Visited]], do
state->iterateVisited([](CyclicModuleRecord* loaded) {
// 5.b.i. If loaded.[[Status]] is NEW, set loaded.[[Status]] to UNLINKED.
if (loaded->status() == CyclicModuleRecord::Status::New)
loaded->status(CyclicModuleRecord::Status::Unlinked);
});
// 5.c. Perform ! Call(state.[[PromiseCapability]].[[Resolve]], undefined, « undefined »).
state->promise()->fulfill(vm, globalObject, module);
// 3. Assert: state.[[PendingModulesCount]] ≥ 1.
ASSERT(state->pendingModulesCount() >= 1);
// 4. Set state.[[PendingModulesCount]] to state.[[PendingModulesCount]] - 1.
state->setPendingModulesCount(state->pendingModulesCount() - 1);
// 5. If state.[[PendingModulesCount]] = 0, then
if (!state->pendingModulesCount()) {
// 5.a. Set state.[[IsLoading]] to false.
state->setIsLoading(false);
// 5.b. For each Cyclic Module Record loaded of state.[[Visited]], do
state->iterateVisited([](CyclicModuleRecord* loaded) {
// 5.b.i. If loaded.[[Status]] is NEW, set loaded.[[Status]] to UNLINKED.
if (loaded->status() == CyclicModuleRecord::Status::New)
loaded->status(CyclicModuleRecord::Status::Unlinked);
});
// 5.c. Perform ! Call(state.[[PromiseCapability]].[[Resolve]], undefined, « undefined »).
state->promise()->fulfill(vm, globalObject, module);
break;
}
}
// 6. Return UNUSED.
state->setDrainingInnerLoad(false);
scope.release();
}

Expand Down Expand Up @@ -932,8 +968,25 @@ void JSModuleLoader::continueModuleLoading(JSGlobalObject* globalObject, ModuleG
// 2. If moduleCompletion is a normal completion, then
if (auto* module = std::get_if<AbstractModuleRecord*>(&moduleCompletion)) {
// 2.a. Perform InnerModuleLoading(state, moduleCompletion.[[Value]]).
innerModuleLoading(globalObject, state, *module);
RETURN_IF_EXCEPTION(scope, void());
// Per-edge fast path: most calls land here with a module that's already
// visited (E >> V); skip the dyncast+enqueue and just decrement.
if (state->containsVisited(*module)) {
ASSERT(state->pendingModulesCount() > 1 || !state->drainingInnerLoad());
state->setPendingModulesCount(state->pendingModulesCount() - 1);
if (!state->pendingModulesCount()) {
state->setIsLoading(false);
state->iterateVisited([](CyclicModuleRecord* loaded) {
if (loaded->status() == CyclicModuleRecord::Status::New)
loaded->status(CyclicModuleRecord::Status::Unlinked);
});
state->promise()->fulfill(vm, globalObject, *module);
}
} else if (state->drainingInnerLoad())
state->enqueueInnerLoad(*module);
else {
innerModuleLoading(globalObject, state, *module);
RETURN_IF_EXCEPTION(scope, void());
}
// 3. Else,
} else {
// 3.a. Set state.[[IsLoading]] to false.
Expand Down
35 changes: 0 additions & 35 deletions Source/JavaScriptCore/runtime/ModuleGraphLoadingState.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,46 +73,11 @@ ModuleGraphLoadingState* ModuleGraphLoadingState::create(VM& vm, JSPromise* prom
return instance;
}

JSPromise* ModuleGraphLoadingState::promise() const
{
return m_promise.get();
}

unsigned ModuleGraphLoadingState::pendingModulesCount() const
{
return m_pendingModulesCount;
}

bool ModuleGraphLoadingState::isLoading() const
{
return m_isLoading;
}

ScriptFetcher* ModuleGraphLoadingState::scriptFetcher() const
{
return m_scriptFetcher.get();
}

void ModuleGraphLoadingState::setPendingModulesCount(unsigned count)
{
m_pendingModulesCount = count;
}

void ModuleGraphLoadingState::setIsLoading(bool loading)
{
m_isLoading = loading;
}

void ModuleGraphLoadingState::appendVisited(VM& vm, CyclicModuleRecord* cyclic)
{
Locker locker { cellLock() };
m_visited.append(WriteBarrier(vm, this, cyclic));
m_visitedSet.add(cyclic);
}

bool ModuleGraphLoadingState::containsVisited(CyclicModuleRecord* cyclic) const
{
return m_visitedSet.contains(cyclic);
}

} // namespace JSC
Loading
Loading