-
Notifications
You must be signed in to change notification settings - Fork 26.2k
Perfomance issue on module init for big apps caused by registerNgModuleType function. #39487
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
Comments
Hello @proofyman, thanks for reporting! Could you check for me what the performance characteristics are when you replace export function registerNgModuleType(ngModuleType) {
recurse(ngModuleType, new Set());
function recurse(ngModuleType, visited) {
const def = ngModuleType.ɵmod;
const id = def.id;
if (id !== null) {<
8000
/span>
const existing = modules.get(id);
assertSameOrNotExisting(id, existing, ngModuleType);
modules.set(id, ngModuleType);
}
let imports = def.imports;
if (imports instanceof Function) {
imports = imports();
}
if (imports) {
for (const i of imports) {
if (!visited.has(i)) {
visited.add(i);
recurse(i, visited);
}
}
}
}
} That should avoid traversing into NgModule import graphs that have already been visited. |
I tested your code, cant find it in perfomance chart, so its good enough =). Ie11 working nice, chrome too. But why you just do not remove this code in runtime ? As i can see modern angular doesnt use module ids. |
Excellent, thank you for confirming! In that case I opened #39514 to make the necessary changes.
I believe there is still uses of module ids in the wild, so it can't be removed right away. In the future it may become possible to remove the surrounding API ( |
…ered When registering an NgModule based on its id, all transitively imported NgModules are also registered. This commit introduces a visited set to avoid traversing into NgModules that are reachable from multiple import paths multiple times. Fixes angular#39487
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
🐞 bug report
Affected Package
The issue is caused by package @angular/core
Is this a regression?
Don't know, seems like not.
Description
We have a big application with deep imports tree. There is function registerNgModuleType, that slow down application, depends on import count.
Problem is that its not depend on unique imports, just all imports, for every tree node down to leaf.
Function does nothing ( mb do something in compile time according to comment ), because it register modules, if them have ids. Modules in our app doesnt have them, no single module have id.
So function, that do nothing, slow down perfomance by 2 seconds for chrome, and 10s for ie11 on my pc. I just delete this function, and perfomance now is ok, and application fully workable.
🔬 Minimal Reproduction
Sorry, i cant add reproduction, because its requires enormous quantity of modules. I could generate them, if needed, but code clear enought to see the problem.
Anything else relevant?

Perfomance on navigation. All green fn blocks - registerNgModuleType and nested fns. There are a lot of calls, about 10k, i dont know. Each call takes 0.1ms, but all of them take 2s and do nothing.
The text was updated successfully, but these errors were encountered: