Perfomance issue on module init for big apps caused by registerNgModuleType function.

[proofyman]

🐞 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.

[JoostK]

Hello @proofyman, thanks for reporting!

Could you check for me what the performance characteristics are when you replace registerNgModuleType with the following implementation:

export function registerNgModuleType(ngModuleType) {
    recurse(ngModuleType, new Set());
    function recurse(ngModuleType, visited) {
        const def = ngModuleType.ɵmod;
        const id = def.id;
        if (id !== null) {
            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.

[proofyman]

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.

[JoostK]

I tested your code, cant find it in perfomance chart, so its good enough =). Ie11 working nice, chrome too.

Excellent, thank you for confirming! In that case I opened #39514 to make the necessary changes.

But why you just do not remove this code in runtime ? As i can see modern angular doesnt use module ids.

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 (getModuleFactory) which should indeed allow this registration business to be removed.

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}