Skip to content
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.

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

importNowHook optimizations #2466

Open
boneskull opened this issue Sep 20, 2024 · 1 comment
Open

importNowHook optimizations #2466

boneskull opened this issue Sep 20, 2024 · 1 comment

Comments

@boneskull
Copy link
Contributor

per @naugtur's comment:

not all of these must make sense, I'm "thinking aloud":

compartmentDescriptors are not changing between calls to importNowHook, so most of this work could be hoisted out to the topmost maker function and only a lookup in a reindexed structure would be needed. On the other hand, the memory needed for that would be wasted most of the time as it's a very rare situation

I'd consider extracting it to a separate function findCompartmentContainingAbsolutePath

since this is not causing a module to be loaded, only redirecting - the further steps are going through a policy check anyway so skipping attenuators compartment in the lookup should be unnecessary as if any attenuators exist, a policy exists as well to prevent their code from being loaded by the compartment.

I'm not sure if this makes sense, but could policy be used to narrow down the search here? Take only the packages allowed by policy and look in their descriptors only.


This concerns the logic in makeImportNowHook, 'round about here:

const importNowHook = moduleSpecifier => {
if (isAbsolute(moduleSpecifier)) {
const record = findRedirect({
compartmentDescriptor,
compartmentDescriptors,
compartments,
absoluteModuleSpecifier: moduleSpecifier,
packageLocation,
});
if (record) {
return record;
}
}

@boneskull boneskull changed the title importHook optimizations importNowHook optimizations Sep 20, 2024
@boneskull
Copy link
Contributor Author

To be addressed sometime after #2310 is merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant