angular / angular-cli

CLI tool for Angular
https://cli.angular.io
MIT License
26.76k stars 11.97k forks source link

Rebuild is way slower and stop working after several rebuilds #20235

Closed montella1507 closed 3 years ago

montella1507 commented 3 years ago

Affected Package

Propably compiler 11.2.5

We have migrated from:

  "@angular/animations": "^11.0.9",
    "@angular/cdk": "^11.0.4",
    "@angular/common": "^11.0.9",
    "@angular/compiler": "^11.0.9",
    "@angular/core": "^11.0.9",
  "@angular/forms": "^11.0.9",
"@angular/platform-browser": "^11.0.9",
    "@angular/platform-browser-dynamic": "^11.0.9",
    "@angular/router": "^11.0.9",
    "@angular/service-worker": "^11.0.9"
  "@angular-devkit/build-angular": "^0.1100.7",
  "@angular/cli": "^11.0.7",
    "@angular/compiler-cli": "^11.0.9",
    "@angular/language-service": "^11.0.9"

TO

 "@angular/animations": "^11.2.5",
    "@angular/cdk": "^11.2.3",
    "@angular/common": "^11.2.5",
    "@angular/compiler": "^11.2.5",
    "@angular/core": "^11.2.5",
  "@angular/forms": "^11.2.5",
 "@angular/platform-browser": "^11.2.5",
    "@angular/platform-browser-dynamic": "^11.2.5",
    "@angular/router": "^11.2.5",
    "@angular/service-worker": "^11.2.5",
  "@angular-devkit/build-angular": "^0.1102.3",
"@angular/cli": "^11.2.3",
    "@angular/compiler-cli": "^11.2.5",
    "@angular/language-service": "^11.2.5"

Is this a regression?

YES

Description

We have migrated from angular 11.0.9 to 11.2.5. Our application is pretty big 1300components, 170 NgModules, 70 lazy loaded chunks.

After migrations almost all rebuild times took 6-10x more time and what is worse.. 1) Rebuild starts afer XYZms (sometimes several seconds) after save of the file 2) Rebuild stopped working after 2-3 saves (rebuilds) , even when change is a breaking change (return 0 to return 1 in component method).

I am willling to help with debug (profile node process / generate and send any log, however it may be imposible to prepare minimal repro :-/ )

montella1507 commented 3 years ago

Some screenshots from profilling + profiling profile:

vscode-profile-2021-03-10-21-07-08.zip ![Uploading image.png…]()

JoostK commented 3 years ago

Hi @montella1507,

thanks for the report. We have landed a large change in 11.2.5 with respect to incremental rebuilds, however the outcome should be that it's now faster than before, not slower. It could be that you're hitting a case where the compiler is unable to prove that work can be reused, in which case it may request more code to be regenerated than before.

I'll have a look at the profile dump, thanks for sharing πŸ‘

montella1507 commented 3 years ago

Thank you very much. I am here to help if possible.

JoostK commented 3 years ago

Ah, gotcha. You're using JIT compilation and the new Ivy Webpack integration that shipped in 11.1 is incorrectly using the AOT compiler's judgement on which files need to be emitted. But since AOT output is never requested, the AOT compiler will continue to indicate that everything needs to be emitted.

If you can enable AOT, then I suggest you do! Otherwise, I'd suggest to disable the new Ivy Webpack plugin in the CLI for now by setting environment variable NG_BUILD_IVY_LEGACY=1.

I'll transfer this to the CLI repo for now.

montella1507 commented 3 years ago

Cool :-) I will try with AOT and will come back with report :-)

montella1507 commented 3 years ago

vscode-profile-2021-03-10-21-36-15-deep-component-change.zip

Rebuild propably will not stop working, however rebuild times still super slow :-(

JoostK commented 3 years ago

Ok, thanks for the additional profile. I do indeed see a lot more emit activity than I'd expect, but I can't tell why that would be the case.

Another observation is that the cyclic import check takes 17s, so you may want to disable that in devbuilds (there's a setting in angular.json, not sure what it's called from the top of my headβ€”edit: it's showCircularDependencies).

JoostK commented 3 years ago

@montella1507 Could you also try with NG_BUILD_IVY_LEGACY=1? I doubt that it will help us diagnose the cause of the issue, but hopefully it does allow you to get back reasonable incremental rebuilds.

montella1507 commented 3 years ago

@JoostK i can confirm JIT mode stop working after few rebuilds. AOT keep working.

"showCircularDependencies": false, changes nothing i think (not sure if it is not just disabling warnings to console) , however.

When i monkeypatch isCyklic method to return directly FALSE. Rebuild times are little bit better (IVY all, AOT all, cyclic off): vscode-profile-2021-03-10-22-26-25.zip

I will try to explain our portal to provide you more information what may be the cause.

It is big CRM where you need cross references between 2 modules, for example:

OrderDetailComponent from OrderModule (lazy loaded) CustomeDetailComponent from CustomerModule (lazy loaded)

We need to show OrderDetail in CustomerDetail and vice-versa CustomerDetail in OrderDetail component. So we need:

// Customer-detail.html <OrderDetail></OrderDetail>

and vice-versa // OrderDetail <CustomerDetail></CustomerDetail>

This is NOT possible when components are not declared in same module and you cannot Import module B in module A and vice versa. And -> we want to have both modules lazy loaded.

So we usually use our custom DynamicComponent (lazy load module, compile, use component factory and attack to viewcontainerref).

So there are dependencies between modules: A -> dynamic component -> B B -> dynamic component -> A

I can imagine this may be big problem for modules buildign dependency tree because it is "lazy" cyklic dependency. However i dont think there is another solution (i simplified this example)

image

JoostK commented 3 years ago

Thanks for the additional information. I suspect that it's just Webpack's dependency graph that makes it believe that everything needs to be reemitted, which is introducing a huge bottleneck as emit is the slowest of all phases. The improvements to 11.2.5 are only concerning Angular's involvement in the process, which used to be suboptimal, but given that your JIT recompiles were also slow means that this is not the relevant here (my initial comment about JIT mode being affected by the AOT compiler turned out to be inaccurate). So that leaves a new problem to be investigated with respect to the dependency graph that Webpack is using to drive recompiles.

montella1507 commented 3 years ago
set NG_BUILD_IVY_LEGACY=1
ng serve...

mode: JIT cyclic: DISABLED

rebuild stiill like 5-6 times slower than in previous NG version (but it works atleast even after 10th change/rebuild).
Interesting thing here is, that first few (1-2) rebuilds it changes 4+ chunks, but 3rd+ rebuild correctly rebuilds only one. (so rebuild time drops from 30sec to 7sec).

However i am not able to profile (profiller / debugging and entire build will crash)

montella1507 commented 3 years ago

vscode-profile-2021-03-10-23-09-20.zip

Ah finaly: NG_BUILD_IVY_LEGACY=1 MODE: JIT cyclic: disabled

So most time takes source map generation i think.

JoostK commented 3 years ago

Thanks. Yeah profiling a long session will take down the Chrome Inspector at some point, it's not uncommon for me to see Chrome take 10GB of memory to show a profiling session.

The last profile you shared does appear to still use the new Ivy pipeline, not the legacy one. Not sure why that would be the case, your set NG_BUILD_IVY_LEGACY=1 should be adequate.

In the meantime we have an idea of what is happening; it's indeed on the CLI side where we can do some experiments to see if things can be improved.

I'm calling it a day now πŸ‘‹

montella1507 commented 3 years ago

@JoostK hi, btw i have made additional benchmarks on 11.0.9 to measure circular-dependency-plugin + sourcmap plugin effects on build time.. here are the results:

image

When circular-dependency detection is enabled, build and rebuild takes 5-8x more time. I think it has serious issues with async circular dependencies or something...

And yes, "showCircularDependencies": false is enough. Will do the same benchmarks on current version of angular / compiler later (when this issue is closed or requested by anyone explicitely)

montella1507 commented 3 years ago

image

JoostK commented 3 years ago

@montella1507 We think to have found the culprit, it was fixed on master in angular/angular-cli#20236 and will be ported to patch so should be part of next week's release. I'll share a snapshot build for patch once that's available.

montella1507 commented 3 years ago

Thank you very much. :-)

I have realized rebuild is affected by assets too - created this issue - maybe it is duplicit to this one - feel free to close please if neccessary.

https://github.com/angular/angular-cli/issues/20244

JoostK commented 3 years ago

Snapshot build is here: https://github.com/angular/ngtools-webpack-builds/tree/11.2.x (see top of readme)

clydin commented 3 years ago

That snapshot build includes fixes for the repeat JIT build crash issue (fix via #20237) and the rebuild performance (fix via #20247). If you have the opportunity, please give the snapshot build a try and let us know if you are still encountering issues.

Also, the asset issue is due to a third-party dependency used by the Angular CLI so a separate issue is appropriate. Some more details regarding the asset performance in serve can be found in a comment in that issue.

montella1507 commented 3 years ago

Thank you will try soon (1-2 days - propably weekend). BTW are you willing to consider to disable circular-dependency-plugin as default? TBH it is cause of serious DX issues on medium - large projects. I tried to disable that on medium projects without dynamic cross references (in comments above) and it affects rebuild time too (30% in medium project, 80% on large with dynamic cross-references).

Or would you accept my PR about "rebuild-time performance" article in documentation?

clydin commented 3 years ago

The circular-dependency-plugin default is definitely something the team will consider. It is, however, a breaking change to modify the default so timing of the change will need to be carefully considered.

montella1507 commented 3 years ago
npm install git+https://github.com/angular/ngtools-webpack-builds.git
+ @ngtools/webpack@12.0.0-next.4
added 6 packages from 9 contributors, removed 1 package, updated 1 package and audited 3255 packages in 27.004s
(however crashed nody-gyp build - but i think that doesnt matter)

JIT: First build: Build at: 2021-03-11T15:06:08.464Z - Hash: bf982f060453b54ec149 - Time: 262241ms Circular-dependency-plugin: disabled (JIT without that LEGACY IVY flag)

App.component.ts (it took 3.5sec on 11.0.9) rebuild time 1st 36 192ms 2nd 29 191 ms 3rd 24 857 ms

AOT: First build: Build at: 2021-03-11T15:06:08.464Z - Hash: bf982f060453b54ec149 - Time: 262241ms Circular-dependency-plugin: disabled (AOT without that LEGACY IVY flag)

App.component.ts (it took 3.5sec on 11.0.9) rebuild time 1st 36 192ms 2nd 29 191 ms 3rd 24 857 ms

So i am not sure it had any effect (can i check somehow i am actually using that snapshot build? i have that "function augmentHostWithDependencyCollection" in my node_modules so i expect it is correctly updated to snapshot build) Here is the CPU profile file (AOT, AppComponent rebuild)

First build: Build at: 2021-03-11T15:13:27.066Z - Hash: bdf37086378ab2e2bfca - Time: 179718ms Circular-dependency-plugin: disabled (AOT without that LEGACY IVY flag)

App.component.ts
rebuild time 1st 50 400ms 2nd 44 000ms 3rd 47 600ms

snapshot-profiling.zip (took so much time on pipelineEmitWithCommends and _readdirp)

clydin commented 3 years ago

Unfortunately, the @ngtools/webpack package is a direct dependency of @angular-devkit/build-angular so it won't be used if installed directly in the project. If using yarn, a resolutions addition in the project's package.json would force the version used. For npm, you could try manually removing any instances of @ngtools/webpack from node_modules that are not the snapshot version (there should be one inside the node modules directory of @angular-devkit/build-angular).

The following is the output of npm ls @ngtools/webpack in a test project:

β”œβ”€β”¬ @angular-devkit/build-angular@0.1200.0-next.3
β”‚ └── @ngtools/webpack@12.0.0-next.3
└── @ngtools/webpack@12.0.0-next.4  (git+https://github.com/angular/ngtools-webpack-builds.git#dafcd5969c31e41f7a1893ebb83cc8ebe53cf6da)
montella1507 commented 3 years ago

OK renamed @ngtools/webpack in @angular-devkit/build-angular folder, ls output:

+-- @angular-devkit/build-angular@0.1102.3
| `-- UNMET DEPENDENCY @ngtools/webpack@11.2.3
+-- @ngtools/webpack@12.0.0-next.4  (git+https://github.com/angular/ngtools-webpack-builds.git#dafcd5969c31e41f7a1893ebb83cc8ebe53cf6da)
`-- @nrwl/builders@7.8.7
  `-- @angular-devkit/build-angular@0.13.10
    `-- @ngtools/webpack@7.3.10
clydin commented 3 years ago

Just noticed that you are in a v11 project but the plugin is for a v12 prelease. For a v11 version of the snapshot with the commits you can use:

npm install git+https://github.com/angular/ngtools-webpack-builds.git#ba719cff4bab883b91684738b04bea0145ec2a81

As an aside, the v12 JIT mode should definitely be faster since support for the deprecated string format for lazy routes has been removed.

montella1507 commented 3 years ago

Results - AOT - no circular: App.component.ts rebuild time 1st 11 994ms 2nd 10 900ms 3rd 9 000ms Thats very impressive for AOT i think anyway 🀟

Results - JIT - no circular:

App.component.ts rebuild time BEST 5 500ms now / 3 600ms on 11.0.9

Col.component.ts (deep shared component) BEST 6 200 ms now / 3700md on 11.0.9

User-role.component.ts (page component) BEST 4300 ms now / 1950ms on 11.0.9

Stock.component.ts BEST 4000ms now / 2100ms on 11.0.9

So i can confirm 3 things:

Here is the profile from JIT 11.2.5-next.4

vscode-profile-2021-03-11-16-41-25.zip

JoostK commented 3 years ago

Can you share an AOT profile as well? Thanks for all the insights btw, super helpful :+1:

montella1507 commented 3 years ago

TBH if you want to, i can provide you remote desktop to my computer and you can do anything you want to πŸ˜„ (keeping that as LAST chance how to settle the problem ). Unfortunately cannot share the project files bcs of NDA.

Here is the AOT profile:

next-5-aot-col-component.zip

JoostK commented 3 years ago

Great, thanks. What version of Node are you using?

montella1507 commented 3 years ago

v14.15.1 npm 6.14.8

montella1507 commented 3 years ago

Projects are on NVMe Samsung PRO SSDs Intel(R) Core(TM) i7-10700K CPU @ 3.80GHz 3.79 GHz ( 4.7 boost stable) 32GB 3600Mhz Dual channel RAMs Windows 10 Pro 20H2 19042.867

JoostK commented 3 years ago

Cool. It doesn't get much faster than that (except for Windows, which is generally slow for I/O). There's quite some interesting things in the profile, one of which can probably be improved on our side.

montella1507 commented 3 years ago

Okey, just let me know if i can help you with anything. (remote desktop / more profiles / test / adding any CONSOLE.log() lol anywhere) :-) We will keep 11.0.9 in main branch for a while, however you made big progress here in last day, TYVM guys.

dgp1130 commented 3 years ago

We discussed this issue in our weekly triage meeting. We also did a bit of digging and found the original PR (#6813) and it seems the main motivation was to fix a compiler bug (#6309). That bug only occurred with the "magic import string" which is no longer supported. The compiler has also evolved a lot since then, so this is unlikely to still be an issue. The circular dependency check is also only a warning, so I don't think it really fixed the compiler problem, just gave a helpful error message about the problem.

General consensus is that the circular dependency checker doesn't provide a whole lot of value. While circular dependencies can cause issues, most of the Node toolchain supports them so it isn't a major problem for most web projects. Angular also supports circular dependencies[citation needed]. The compiler throws errors in specific, problematic cases but generally allows circular dependencies. This is only an issue if a project owner decides it is an issue. As a result, it is reasonable for them to integrate such a check without necessarily expecting one out of the box. ESLint has one such check, and there are likely other implementations as well.

We could just run this check for production builds, however prod builds are usually done by CI and the circular dependency check is only a warning, so it is unlikely many humans would see this log and take any meaningful action from it. Since the check provides little value now, limiting it to production builds will only reduce that value further. As a result, we're looking into removing the circular dependency check altogether. This will speed up build times (as shown above), reduce third party dependencies, and make node_modules/ a bit smaller.

Two quick things to confirm before we commit to removing the plugin:

  1. @filipesilva, you originally added the circular dependency check in #6813 (thanks for the detailed commit message!), is there any other context or considerations we should have before removing it? Any objections to these arguments?
  2. We should also include a proper circular dependency test case with our new dynamic import infrastructure if it does not already exist. This would also confirm that the previous compiler issue no longer exists and that it is safe to remove the circular dependency check.
montella1507 commented 3 years ago

We could just run this check for production builds, however prod builds are usually done by CI and the circular dependency check is only a warning, so it is unlikely many humans would see this log and take any meaningful action from it.

we have modified CI configuration that it runs build from commits WITHOUT Source-maps and WITHOUT circular dependency check + AUTO night builds (any periodic builds) runs WITH SM and WITH CDCH.

It changed time required to 1/10 (for CI commit builds + develeper experience) and still viable option to detect circular dependencies (it is very rare situation for us - we have good barel files strategies + NRWL tags + linters).

I will be honest here... FInding out that Circular dependency plugin is the cause of very big rebuild times and disabling it is ABSOLUTE GAME CHANGER for us - because if you wait 30sec every 2mins or wait 3sec every 2 mins is big difference in DX + economy of development in angular.

I think THE TIME WHEN IS DEVELOPER WAITING(during rebuilds) is little bit underestimated and may be the reason why IS / IS NOT angular selected for larger projects.

montella1507 commented 3 years ago

@JoostK @dgp1130 hi guys, should i close this issue?

clydin commented 3 years ago

@montella1507 If you have some additional time, could you try the latest snapshot build?

npm install git+https://github.com/angular/ngtools-webpack-builds.git#a2c144fbeeef9c9584b7848acdb0d1f5b4d9e20b

You'll need to do the same manual change to node modules again to force the @ngtools/webpack usage.

Angular CLI 11.2.5 will be released this week but feedback before the release would be helpful. Thank you again for all the profiling information.

JoostK commented 3 years ago

@montella1507 I'd be okay to keep it open until after the next release (likely today), so that you can check whether it's back to normal then.

montella1507 commented 3 years ago

@clydin npm ls @ngtools/webpack output +-- @angular-devkit/build-angular@0.1100.7 |-- UNMET DEPENDENCY @ngtools/webpack@11.0.7 +-- @ngtools/webpack@11.2.4 (git+https://github.com/angular/ngtools-webpack-builds.git#a2c144fbeeef9c9584b7848acdb0d1f5b4d9e20b) -- @nrwl/builders@7.8.7 -- @angular-devkit/build-angular@0.13.10 -- @ngtools/webpack@7.3.10

AOT: FALSE / CIRCULAR PLUGIN: TRUE (best rebuild times)

App.component (main chunk) - 12 156 COL.component (deep shared - main chunk) - 10 400 user-role 8 500 stock-component (routed components - lazy chunk) 9400

AOT: FALSE / CIRCULAR PLUGIN: FALSE (best rebuild times)

App.component (main chunk) - 3 500 COL.component (deep shared - main chunk) - 3300 user-role 2 000 stock-component (routed components - lazy chunk) 3000

AOT: TRUE / CIRCULAR PLUGIN: FALSE (best rebuild times)

App.component (main chunk) - 12 000 COL.component (deep shared - main chunk) - 48 000 ms (not cool :-/) user-role 11 000 stock-component (routed components - lazy chunk) 11 000

WILL check times again when you publish new version (with clean install of packages - just to be sure everything is UP TO DATE)

Here is the CPU PROFILE FROM - AOT: TRUE / CIRCULAR PLUGIN: FALSE / COL.component (48 000ms rebuild). There MAY be prblem with packages (i am not sure everything is up to date), but you would see in cpu profile: vscode-profile-2021-03-17-20-19-28.zip

EDIT: i can pull lates version of our solution, update that again to the latest angular, do clean install and use your snapshot build if you wish to - however source code is edited (it is week+ old commit - so results may be influenced by changes in our codebase - however you would see if there is problem... just dont ask me to do that without reason πŸ˜„ just if you think there is a problem in my packages. tyvm)

clydin commented 3 years ago

From the profile it looks like the legacy Ivy plugin is being used along with an older version of the @angular/compiler-cli. That would probably explain those varying timings.

The next patch release of the Angular CLI should be out today. So it may be better to wait and try a full update to the latest versions of 11.2.x.

JoostK commented 3 years ago

Another observation is that you're not yet on TypeScript 4.1. This affects source-map performance in AOT compilations, as there was a severe performance penalty for source mapping into external templates.

montella1507 commented 3 years ago

OK. Just let me know when everything is OUT. I will: 1) pull latest source code 2) ng update 3) rm package-lock 4) npm i 5) provide correct results with everything up to date
(will attach more CPU profiles when anything not cool :-) ) :-)

(fury refreshing :-) ) image

JoostK commented 3 years ago

@montella1507 Refresh once more πŸ˜„

So just to summarize: you'll need @angular/compiler-cli (and its other framework friends) at version >= 11.2.5, with 11.2.6 being today's release. Additionally, you'll need @angular/cli 11.2.5, i.e. today's release. Furthermore, it would be beneficial to use TS 4.1 if combined with AOT, but I don't expect this to matter much in incremental rebuilds (only initial builds).

dgp1130 commented 3 years ago

@montella1507 CLI 11.2.5 just released, so you should be unblocked there.

montella1507 commented 3 years ago

I have problem with updating to TS 4.1, because:

or 4.1.0-dev.20200930 ?
so what should i change to? 4.2.3 ?

TBH i cant install 4.2.3 what is .. strange.

JoostK commented 3 years ago

Don't worry too much about the TS update. I meant TS 4.1.x in general, so that would be the latest 4.1.5. You can use ~4.1.5 as version specifier to get the latest 4.1.x but not 4.2.x (TS does not follow semver). The ~4.2.3 dependency version is for the prereleases of Angular 12.

montella1507 commented 3 years ago

ok thanks :-) npm install typescript@4.1.5 --skip-cache

will paste results soon. (had to install and do some manual work, because ng update auto-migrations failed - because of older NRWL / NX - no tsconfig.json (because it is tsconfig.base.json) so i have not updated package.json and packages-lock.json (but correct packages are installed.)

JoostK commented 3 years ago

It could be that the TS update introduces some new compilation errors, so you can choose to leave that one for now and just update to latest Angular. The effect of the TS improvements are mostly noticeable on initial builds.

montella1507 commented 3 years ago

1) maybe it is bug that i see 0 bytes in rebuilt chunk size ? :-)

Lazy Chunk Files                                        | Names                                                |    Size      
components-src-lib-user-management-components-module.js | components-src-lib-user-management-components-module | 0 bytes  

2) Let me know when you got to Czech Republic, i owe you a beer

image image

NOTES:

JoostK commented 3 years ago

That's more like it :-)

The AOT improvement to deep-shared component is due to angular/angular#40947. The general AOT decrease from ~10s to ~4s is likely due to angular/angular#40948, although angular/angular#40947 also helps.

Which version of TS was this? And could you share an AOT / NO CIRC / NO SM profile as well?