Activity

1. proofyman

   changed the title [-]Perfomance issue on module init for big apps caused by noop function.[/-] [+]Perfomance issue on module init for big apps caused by registerNgModuleType function.[/+] on Oct 29, 2020
2. josephperrott

   added

   area: coreIssues related to the framework runtimeIssues related to the framework runtime

   on Oct 29, 2020
3. ngbot

   added this to the needsTriage milestone on Oct 29, 2020
4. JoostK

   added

   area: performanceIssues related to performanceIssues related to performance

   core: performance

   P3An issue that is relevant to core functions, but does not impede progress. Important, but not urgentAn issue that is relevant to core functions, but does not impede progress. Important, but not urgent

   on Oct 30, 2020
5. ngbot

   modified the milestones: needsTriage, Backlog on Oct 30, 2020
6. JoostK commented on Oct 30, 2020

   

   JoostK

   on Oct 30, 2020

   Member

   More actions

   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.
7. proofyman commented on Oct 30, 2020

   

   proofyman

   on Oct 30, 2020

   Author

   More actions

   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.
8. JoostK commented on Oct 30, 2020

   

   JoostK

   on Oct 30, 2020

   Last edited by JoostK

   Member

   More actions

   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.
9. JoostK

   added

   state: has PR

   on Oct 30, 2020
10. JoostK

    added a commit that references this issue on Oct 30, 2020

    perf(core): do not recurse into modules that have already been regist…

    ...

    This issue will close once commit dfaf450 is merged into the 'main' branch.

    dfaf450
11. josephperrott

    closed this as completedin 8317042on Nov 2, 2020
12. josephperrott

    added 2 commits that reference this issue on Nov 2, 2020

    perf(core): do not recurse into modules that have already been regist…

    ...

    This issue will close once commit 812355c is merged into the 'main' branch.

    812355c

    perf(core): do not recurse into modules that have already been regist…

    ...

    This issue will close once commit 5c13c67 is merged into the 'main' branch.

    5c13c67
13. angular-automatic-lock-bot commented on Dec 3, 2020

    

    angular-automatic-lock-botbot

    on Dec 3, 2020

    More actions

    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.}
14. angular-automatic-lock-bot

    locked and limited conversation to collaborators on Dec 3, 2020