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

Huge performance drop after upgrade to Angular 11 #41038

Closed
galczo5 opened this issue Mar 1, 2021 · 64 comments
Closed

Huge performance drop after upgrade to Angular 11 #41038

galczo5 opened this issue Mar 1, 2021 · 64 comments
Labels
area: core Issues related to the framework runtime area: performance needs: clarification This issue needs additional clarification from the reporter before the team can investigate.
Milestone

Comments

@galczo5
Copy link

galczo5 commented Mar 1, 2021

🐞 bug report

Affected Package

Hard to point, one of:

"@angular/animations": "^11.1.2",
"@angular/common": "^11.1.2",
"@angular/compiler": "^11.1.2",
"@angular/core": "^11.1.2",
"@angular/forms": "^11.1.2",
"@angular/platform-browser": "^11.1.2",
"@angular/platform-browser-dynamic": "^11.1.2",
"@angular/router": "^11.1.2",
"@angular/upgrade": "11.1.2",

Is this a regression?

Yes

Description

After the upgrade from Angular 9 to 11 we find out that performance of component creation has declined. In our automated tests we get creation times doubled in almost every complex component. Results can be found on the image below. As you can see the main problem here is scripting time.

image

For testing purposes we use puppeteer to get performance profiles from Google Chrome. Every component is created dynamically. Tracing data is collected using the Tracing class from the Puppeteer library (https://pptr.dev/#?product=Puppeteer&version=v2.0.0&show=api-class-tracing). The start method is called at the beginning of each iteration of the test and the stop method is called after the test completes. Then the tracing data is parsed to determine the scripting, rendering and painting times.

Problem exists in both JIT and AOT builds with and without prod mode enabled.

Even when we tried to investigate this using a manually generated profile we found exact same results. It’s not only about numbers, we feel that our app is much slower.

This problem and really slow build time (compared to Angular 9) was the reason to downgrade back to Angular 9.

🔬 Minimal Reproduction

It’s hard to reproduce this problem in minimal env. Our example is a huge, enterprise scale app. We have a lot of performance tweaks, like:
“On Push strategy everywhere”
Noop NgZone provided
We don’t use styles encapsulation

Additionally it is good to point that we use tones of dynamically generated components.

I’ve tried to investigate the problem on a clean angular/cli project. In both (9 and 11) times of component creation are the same (https://github.com/galczo5/angular-11-rendering). Maybe it’s related to the scale of our app.

We can try to expose our whole app in prod mode for you for investigation. Please let us know if this is necessary.

If you want we can start with performance profiles.
Again please let us know as soon as possible.

🔥 Exception or Error

No exception or error.

🌍 Your Environment

Angular Version:
Angular 9:



     _                      _                 ____ _     ___
    / \   _ __   __ _ _   _| | __ _ _ __     / ___| |   |_ _|
   / △ \ | '_ \ / _` | | | | |/ _` | '__|   | |   | |    | |
  / ___ \| | | | (_| | |_| | | (_| | |      | |___| |___ | |
 /_/   \_\_| |_|\__, |\__,_|_|\__,_|_|       \____|_____|___|
                |___/


Angular CLI: 9.1.14
Node: 10.16.3
OS: darwin x64

Angular: 9.1.13
... animations, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... platform-server, router, upgrade
Ivy Workspace: Yes

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.901.14
@angular-devkit/build-angular     0.901.14
@angular-devkit/build-optimizer   0.901.14
@angular-devkit/build-webpack     0.901.14
@angular-devkit/core              9.1.14
@angular-devkit/schematics        9.1.14
@angular/cdk                      7.3.7
@angular/cli                      9.1.14
@ngtools/json-schema              1.1.0
@ngtools/webpack                  9.1.14
@schematics/angular               0.5.9
@schematics/update                0.901.14
rxjs                              6.6.3
typescript                        3.8.3
webpack                           4.42.0


Angular 11:



     _                      _                 ____ _     ___
    / \   _ __   __ _ _   _| | __ _ _ __     / ___| |   |_ _|
   / △ \ | '_ \ / _` | | | | |/ _` | '__|   | |   | |    | |
  / ___ \| | | | (_| | |_| | | (_| | |      | |___| |___ | |
 /_/   \_\_| |_|\__, |\__,_|_|\__,_|_|       \____|_____|___|
                |___/


Angular CLI: 11.1.4
Node: 10.16.3
OS: darwin x64

Angular: 11.2.0
... animations, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... router
Ivy Workspace: Yes

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1101.4
@angular-devkit/build-angular   0.1101.4
@angular-devkit/core            11.1.4
@angular-devkit/schematics      11.1.4
@angular/cdk                    7.3.7
@angular/cli                    11.1.4
@angular/upgrade                11.1.2
@schematics/angular             11.1.4
@schematics/update              0.1101.4
rxjs                            6.6.3
typescript                      4.0.5

Anything else relevant?
No

@JoostK
Copy link
Member

JoostK commented Mar 1, 2021

I can't tell what the root cause of this could be; there have been several changes to the Ivy Runtime between 9.1.13 and 11.2.0 but I couldn't tell if the regression you're seeing could be caused by any of them.

Could you investigate if you are perhaps affected by babel/babel#11356? Is @babel/plugin-transform-classses present as a dep and what version is used in v9 vs v11?

Btw, we are actively working on the rebuild performance in v11. A rebuild correctness fix has resulted in more expensive rebuilds in 11.1, see #40635 and status tracker in #40728 for more info.

@JoostK JoostK added area: performance needs: clarification This issue needs additional clarification from the reporter before the team can investigate. labels Mar 1, 2021
@ngbot ngbot bot modified the milestone: Backlog Mar 1, 2021
@galczo5
Copy link
Author

galczo5 commented Mar 1, 2021

Angular 9:

"@babel/plugin-transform-classes@^7.9.0":
  version "7.12.13"
  dependencies:
    "@babel/helper-annotate-as-pure" "^7.12.13"
    "@babel/helper-function-name" "^7.12.13"
    "@babel/helper-optimise-call-expression" "^7.12.13"
    "@babel/helper-plugin-utils" "^7.12.13"
    "@babel/helper-replace-supers" "^7.12.13"
    "@babel/helper-split-export-declaration" "^7.12.13"
    globals "^11.1.0"

Angular 11:

"@babel/plugin-transform-classes@^7.12.1":
  version "7.12.1"
  dependencies:
    "@babel/helper-annotate-as-pure" "^7.10.4"
    "@babel/helper-define-map" "^7.10.4"
    "@babel/helper-function-name" "^7.10.4"
    "@babel/helper-optimise-call-expression" "^7.10.4"
    "@babel/helper-plugin-utils" "^7.10.4"
    "@babel/helper-replace-supers" "^7.12.1"
    "@babel/helper-split-export-declaration" "^7.10.4"
    globals "^11.1.0"

@JoostK
Copy link
Member

JoostK commented Mar 1, 2021

Thanks; that's supposedly not the cause then.

Although performance profiles might hint at where the bottleneck occurs, finding the root cause would probably require bisecting to determine the version where this is introduced. Performance traces are very hard to interpret for production bundles, and even more so because of bundle (size) optimizations and JS engine optimizations such as function inlining, which can skew the results significantly. I suspect that you are in the best position to try various versions of Angular to determine which patch release introduced the regression, as we don't have the ability to build from scratch with different versions.

@galczo5
Copy link
Author

galczo5 commented Mar 1, 2021

We had problems with dev build before. After #40374 everything was ok. As I know this fix is not included in Angular 10 and some versions of Angular 11. Bisecting can be hard to do or impossible on dev mode without this patch.

I'll find out if prod build works without this fix.

@galczo5
Copy link
Author

galczo5 commented Mar 3, 2021

Prod build works with our config. I'll try to do bisection.

@galczo5
Copy link
Author

galczo5 commented Mar 3, 2021

Ok, I don't have good news. Problem occurs between major version, so it can be hard to say where the problem is.
The biggest performance drop is between 9.1.13 and 10.0.0.

image

image

image

image

Every build was started with AOT and prod mode enabled. I was testing and recording profiles with the same data, hardware and in short period of time. I think we can assume that hardware or environment are not the problem.

Main changes in package.json:

"@angular/animations": "10.0.0",
"@angular/cdk": "^7.3.7",
"@angular/common": "^10.0.0",
"@angular/compiler": "^10.0.0",
"@angular/core": "^10.0.0",
"@angular/forms": "^10.0.0",
"@angular/platform-browser": "^10.0.0",
"@angular/platform-browser-dynamic": "^10.0.0",
"@angular/platform-server": "^10.0.0",
"@angular/router": "^10.0.0",
"@angular/upgrade": "^10.0.0",
"@angular/animations": "^9.1.13",
"@angular/cdk": "^7.3.7",
"@angular/common": "^9.1.13",
"@angular/compiler": "^9.1.13",
"@angular/core": "^9.1.13",
"@angular/forms": "^9.1.13",
"@angular/platform-browser": "^9.1.13",
"@angular/platform-browser-dynamic": "^9.1.13",
"@angular/platform-server": "9.1.13",
"@angular/router": "^9.1.13",
"@angular/upgrade": "9.1.13",

Any ideas what I can check next to find the problem?

@JoostK
Copy link
Member

JoostK commented Mar 3, 2021

Thanks for the extensive research! Did you also test the prerelease/RC versions of v10? The screenshots show buildPrunedListOfNodes and selectIsDragging, I suppose that is project code? What does the bottom-up table in the performance profile look like when you select selectIsDragging? Hopefully there is something standing out as a difference there.

@galczo5
Copy link
Author

galczo5 commented Mar 3, 2021

Yes, this methods are our methods. selectIsDragging is method that subscribes to observable and triggers change detection via changeDetectorRef.detectChanges() on new value.

Here you will find more detailed profiles.

image

image

Second one shows one simple operation I found. Selected rectangles presents the same place of rendering code in both versions. As you can see Angular 10 performs more operations than 9. In both cases we have Ivy enabled.

@galczo5
Copy link
Author

galczo5 commented Mar 3, 2021

In both cases our project code was not changed, I forgot to mention it.

@galczo5
Copy link
Author

galczo5 commented Mar 4, 2021

@JoostK If you need I probably can send you profiles via email.

@galczo5
Copy link
Author

galczo5 commented Mar 5, 2021

Ok, I think I have something that might be problem here. I decided to dig some more today and I've compared dev build for 9 and 11 versions.

image

In both cases Angular call ɵɵproperty method and it takes 0.12ms in both cases. Difference is in ɵɵadvance (https://github.com/angular/angular/blob/a6971ba89adc253bfa4260036ee4a1e0bd76159f/packages/core/src/render3/instructions/advance.ts). There is no call for this method in Angular 9.

In sources I found:

/**
 * This is a synthetic lifecycle hook which gets inserted into `TView.preOrderHooks` to simulate
 * `ngOnChanges`.
 *
 * The hook reads the `NgSimpleChangesStore` data from the component instance and if changes are
 * found it invokes `ngOnChanges` on the component instance.
 *
 * @param this Component instance. Because this function gets inserted into `TView.preOrderHooks`,
 *     it is guaranteed to be called with component instance.
 */
function rememberChangeHistoryAndInvokeOnChangesHook(this: OnChanges) {
  const simpleChangesStore = getSimpleChangesStore(this);
  const current = simpleChangesStore?.current;

  if (current) {
    const previous = simpleChangesStore!.previous;
    if (previous === EMPTY_OBJ) {
      simpleChangesStore!.previous = current;
    } else {
      // New changes are copied to the previous store, so that we don't lose history for inputs
      // which were not changed this time
      for (let key in current) {
        previous[key] = current[key];
      }
    }
    simpleChangesStore!.current = null;
    this.ngOnChanges(current);
  }
}

It is part of NgOnChangesFeature. In the same file I found this doc:

/**
* The NgOnChangesFeature decorates a component with support for the ngOnChanges
* lifecycle hook, so it should be included in any component that implements
* that hook.
*
  !!! HERE
* If the component or directive uses inheritance, the NgOnChangesFeature MUST
* be included as a feature AFTER {@link InheritDefinitionFeature}, otherwise
* inherited properties will not be propagated to the ngOnChanges lifecycle
* hook.
*
* Example usage:
*
* ```
* static ɵcmp = defineComponent({
*   ...
*   inputs: {name: 'publicName'},
*   features: [NgOnChangesFeature]
* });
* ```
*
* @codeGenApi
*/
export function ɵɵNgOnChangesFeature<T>(): DirectiveDefFeature {
 return NgOnChangesFeatureImpl;
}

I understand that using inheritance may be the problem here. Am I right? Call of rememberChangeHistoryAndInvokeOnChangesHook exists in prod profiles too, so I assume that it is not used only in dev build. Execution times are almost the same in both prod and dev build profiles.

Almost all our components inherit from class like this:

@Directive()
export abstract class BaseComponent {
 ...
 private ngOnInit() { ... }
 private ngOnDestroy() { ... }
}

It's strange, that this method fires. Our base component does not implement ngOnChanges hook. I've found this hook only in children of this component.

Can someone describe how it works and how inheritance can affect angular performance? I didn't found this in release notes. Can I disable NgOnChangesFeature in our base components?

I'm working on minimal env reproduction, I'll post it later.

@galczo5
Copy link
Author

galczo5 commented Mar 5, 2021

Ok, you can check it here: https://github.com/galczo5/angular-11-rendering

image

This ɵɵadvance was called after adding directive that implements hooks and extends class.
You can change number of rendered items to 100k, should be easier to measure.
In my tests difference between 9 and 11 was from .5s up to 1s.

@galczo5
Copy link
Author

galczo5 commented Mar 9, 2021

Do you need more info from me? For us it's a blocker for feature upgrades of angular.

@AndrewKushnir
Copy link
Contributor

Hi @galczo5, thanks for sharing additional information.

I've tried to reproduce the problem using the test repository you shared. Here are some details:

  • for the v11-based app I've used the most up-to-date version of Angular packages (v11.2.5)
  • I've used Chrome 88 in Incognito mode
  • I did a few tests and picked the lowest number for both v9 and v11

Here are the results:
v9-vs-v11

Based on my tests, the most recent v11 version (v11.2.5) has slightly better performance than v9.

Could you please try to use the mentioned version for v11-based app, run apps in Incognito mode? Also please do a few runs and pick the lowest (i.e. best, not average) result for both v9 and v11 and compare the results.

@AndrewKushnir AndrewKushnir added the area: core Issues related to the framework runtime label Mar 11, 2021
@galczo5
Copy link
Author

galczo5 commented Mar 11, 2021

@AndrewKushnir Ok. You may expect results tomorrow to the end of the day. I want to test both given minimal reproduction and our app with 11.2.5

@joebnb
Copy link

joebnb commented Mar 11, 2021

could fix this performance issue,if not using AOT it's coast a lot of time to compile angular template,why not turn compile to on demand or lazyload #41044

@galczo5
Copy link
Author

galczo5 commented Mar 12, 2021

I'm back with the results.

image

Testing setup:

  • Incognito mode, Chrome version: 88.0.4324.192
  • Angular 9.1.13 vs Angular 11.2.5
  • 9 tests for each variant (Angular Version x Prod/Dev mode build)

App code between tests was not changed. I'm using buildPrunedListOfNodes and selectIsDragging because they call detectChanges and it's affecting a lot of child components.

Last test selected component was my try of checking refreshView method. I collected data for calls presented on images below. My reference point here was ngOnInit and setDropdownConfig methods.
image
image

As you can see there is huge drop between 9 and 11. I've tested it with Angular 12, results was similar to Angular 11.

Our project is huge and I'm afraid that it's can be impossible to reproduce it with minimal env.

@galczo5
Copy link
Author

galczo5 commented Mar 12, 2021

@joebnb We have AOT enabled in prod builds. Problem still occurs.

@joebnb
Copy link

joebnb commented Mar 12, 2021

@joebnb We have AOT enabled in prod builds. Problem still occurs.

could you disable aot and take a screenshot,in my cases enable aot will improve a lot. in your case it should be slower than 9/11 aot enabled.

@galczo5
Copy link
Author

galczo5 commented Mar 12, 2021

@joebnb disabling AOT is not acceptable solution for our problem.
Without app bootstrap takes a lot of time. I can try it but it shouldn't be used in production.

@AndrewKushnir
Copy link
Contributor

@galczo5 thanks for additional information. I don't see anything unusual on the flame charts provided.

Do you see any functional differences in the app between v9 and v11.2.5? For example, more calls to lifecycle hooks that now show up in the profiler or something similar?

I believe it would be also helpful to profile an app (take the same steps that you described) with Angular v10 and all minor versions for v10 and v11 (v10.1.0, v11.1.0, v11.2.0) to see if there was a performance drop at a particular version.

@galczo5
Copy link
Author

galczo5 commented Mar 15, 2021

@AndrewKushnir I've checked it before. Drop appears between 9.1.13 and 10.0.0.
I've just checked hooks calls.

hook Angular 9 Angular 11
ngOnInit 352 1235
ngOnChanges 929 549
ngDoCheck 257 0
ngAfterContentInit 2 45
ngAfterViewInit 11 15
ngOnDestroy 42 142

I think that this can be really our problem. I'll try to compare differences between ngOnInit hooks. There is the biggest difference.

Any helpful advices?

@galczo5
Copy link
Author

galczo5 commented Mar 15, 2021

After first analysis, problem may be connected to:

  • Components used with @ContentChild(TemplateRef, { static: true })
  • Directives on this components

Anything important changed in ContentChild in Angular 10?

@AndrewKushnir
Copy link
Contributor

Hi @galczo5, thanks for additional information.

I've looked at the 10.0.0 changes in the CHANGELOG.md and there is a couple PR that might potentially be related to lifecycle hooks (#35968 and #35840), but looking at these changes I don't see anything suspicious that may cause the problem that you described.

Drop appears between 9.1.13 and 10.0.0.
...
Any helpful advices?

Could you please also try different next/RC versions between 9.1.13 and 10.0.0? The versions are:

  • 10.0.0-next.0, 10.0.0-next.1, ... 10.0.0-next.9
  • 10.0.0-rc.0, 10.0.0-rc.1, ... 10.0.0-rc.6

If you can help identify a specific version where the drop happened, it'd be easier to find what caused that and what the possible solutions might be.

Thank you.

@galczo5
Copy link
Author

galczo5 commented Mar 17, 2021

@AndrewKushnir I'm back with the results. Prod mode enabled, AOT build.

Lowest results for buildPrunedListOfNodes:

Times:

9.1.13 10.0.0-next.0 10.0.0-rc.0
468.75 ms 1.19 s 1.18 s

Hooks:

hook 9.1.13 10.0.0-next.0 10.0.0-rc.0
ngOnInit 431 1297 1262
ngOnChanges 1078 595 586
ngOnDestroy 92 120 98

It's strange that with Angular 10 number of ngOnInit hooks is increasing but number of ngOnChanges is reduced to 50%.

@galczo5
Copy link
Author

galczo5 commented Apr 8, 2021

I know, we are warned about possible optimization bailouts. I'm quite sure that it should not affect performance in our case. I still have problems to find what is wrong and what changed so I decided to ask about this warning.

@galczo5
Copy link
Author

galczo5 commented Apr 13, 2021

We checked our app for memory leaks. In both cases we have the same memory leaks, so it should not be the problem.
By the time we will try to run part of app based on clean angular/cli app.

Can you tell me if anything in tsconfig.json may be the problem?

{
   "compileOnSave": false,
   "compilerOptions": {
      "baseUrl": "src",
      "outDir": "./dist/out-tsc",
      "sourceMap": true,
      "declaration": false,
      "module": "ESNext",
      "moduleResolution": "node",
      "emitDecoratorMetadata": true,
      "experimentalDecorators": true,
      "skipLibCheck": true,
      "skipDefaultLibCheck": true,
      "target": "es5",
      "types": [
         "node",
         "chrome",
         "cometd3",
         "cometd4",
         "lodash"
      ],
      "lib": [
         "es2017",
         "dom",
         "esnext.asynciterable"
      ]
   }
}

Both in angular 11 and 9 we use the same tsconfig.json.

Similar situation with polyfills, we use the same version of "core-js": "3.8.3" and "zone.js": "~0.10.3". Is it correct to use these versions in angular 11?

@BrkCoder
Copy link

@galczo5 Hey I have occurred this performance problem in my team, can we get any assistant or workaround for this problem?

@galczo5
Copy link
Author

galczo5 commented Apr 19, 2021

@BrkCoder we didn't found the root of the problem. I'm still trying to debug it, but for me, it looks like everything just works slower. We will stay with angular 9 for now.

If you want we can compare our cases, maybe it will reveal where the problem is.

@hakimio
Copy link

hakimio commented May 6, 2021

Similar situation with polyfills, we use the same version of "core-js": "3.8.3" and "zone.js": "~0.10.3". Is it correct to use these versions in angular 11?

"core-js" should be removed from dependencies. Starting with Angular 10, "core-js" is part of Angular CLI. Version of "zone.js" depends on your Angular version (v0.9.x -> Angular v9, v0.10.x -> Angular v10, v0.11.x -> Angular v11 and so on).

@sean-tj
Copy link

sean-tj commented May 7, 2021

@galczo5 Have you solve this performance issue? We got the same problem in our application after upgrade to Angular 11.

@galczo5
Copy link
Author

galczo5 commented May 7, 2021

@sean-tj unfortunately no :( Everything works just normal in the clean angular CLI app.
Last time I've tried to generate an app with 1000 different components, to check if it is a scale problem.
Still works a little bit faster on Angular 11. I'm going to try it with more components soon. Can you tell me how many components do you have?

Can you try to reproduce my previous tests in this thread to confirm the problem?

@fipciu1996

This comment has been minimized.

@donovancarthew
Copy link

We have also seen a huge performance drop after upgrading from Angular 9 to 11. For us the problem is most noticeable in rendering tables with angular/material.

I'm not a performance guru, so I'm not sure where to start when debugging this. But you can see from the attached screenshots that there is some "Task" that is taking 5 times as long with Angular 11.

Hope this helps with the investigation in some way. For now we have reverted back to Angular 9 and are still looking for a workaround before we bring angular 11 to production.

Screen Shot 2021-05-31 at 3 33 59 pm

Screen Shot 2021-05-31 at 3 36 22 pm

@galczo5
Copy link
Author

galczo5 commented Jun 14, 2021

@AndrewKushnir @JoostK Any idea what we can check next? We are still blocked with upgrade from 9 to 11/12.

@AndrewKushnir
Copy link
Contributor

Hi @galczo5, I've performed some tests using an internal benchmarking tool that we've put together some time ago to investigate performance bottlenecks and compared v9.0.0 and v12.0.0 (and a few major/minor versions in between) and didn't find any regressions: v9 and v12 results are very close. We'd like to help more, but it's hard to make further progress without a minimal repro that we can investigate.

Any idea what we can check next?

There is a couple things you can try next (sorry if it was already mentioned):

  • Try to find a particular version (or version range) where you see perf regression. You can try upgrading using minor versions and when you see a regression - try patch versions in that range. Having this information would allow us to narrow down the set of changes that might be involved.
  • Try to simplify templates of the components that you have in the app (on the pages that you use for perf benchmarking) and see if there is a pattern where removing some elements/components allows to avoid perf regression.
  • Check if removing some third-party libs from the app restores the performance (that'd also help to narrow down the issue).

Thank you.

@galczo5
Copy link
Author

galczo5 commented Jun 28, 2021

Drop appears between versions 9 and 10. I was mentioned some time ago.
Right now I have our app updated to Angular 12.1.0, the problem is still noticeable.

@AndrewKushnir @JoostK If you want I can record a video with both versions.
If you want I can organize a video meeting, probably it would be the best way to demonstrate the problem.

For us it's a blocker of any upgrade. 😢

@galczo5
Copy link
Author

galczo5 commented Jun 28, 2021

@donovancarthew @sean-tj did you found any workaround?

@AndrewKushnir
Copy link
Contributor

Drop appears between versions 9 and 10. I was mentioned some time ago.

Thanks for additional information. Could you please try to find a specific v9 patch version (e.g. 9.1.5) that is causing the problem (by changing Angular package versions in package.json and running the app)? You can find the list of versions here: https://github.com/angular/angular/releases?after=10.0.1.

Also as I described in my previous message, please try to simplify template(s) of the affected pages and see if there is a particular component/directive that is causing perf regressions.

@petebacondarwin
Copy link
Member

@AndrewKushnir - from reading through the previous comments it looks like the drop was between the last of the v9 patch versions (i.e. 9.1.13) and the first of the v10 pre-releases (i.e. 10.0.0-next.0).

This could be very painful and slow @galczo5, but you could consider locally building versions of Angular at different commits between the point when v10 was branched from v9. There are 149 such commits: 8968b20...10.0.0-next.0. A binary search could identify the problem commit in 7 attempts.

@petebacondarwin
Copy link
Member

These are the only commits (from looking through that list) that could have impacted core performance IMO:


build(zone.js): update zone.js version to 0.10.3 (#36214)

PR Close #36214


fix(zone.js): UNPATCHED_EVENTS and PASSIVE_EVENTS should be string[] not boolean (#36258)

__zone_symbol__UNPATCHED_EVENTS and __zone_symbol__PASSIVE_EVENTS should be string[] type not boolean.
For example:

const config = window as ZoneGlobalConfigurations;
config.__zone_symbol__UNPATCHED_EVENTS = ['scroll'];
config.__zone_symbol__PASSIVE_EVENTS = ['scroll'];

PR Close #36258


fix(zone.js): should not try to patch fetch if it is not writable (#36311)

Close #36142

In Firefox extensions, the window.fetch is not configurable, that means

const desc = Object.getOwnPropertyDescriptor(window, 'fetch');
desc.writable === false;

So in this case, we should not try to patch fetch, otherwise, it will
throw error ('fetch is ReadOnly`)

PR Close #36311


fix(core): ngOnDestroy on multi providers called with incorrect context (#35840)

Currently destroy hooks are stored in memory as [1, hook, 5, hook] where
the numbers represent the index at which to find the context and hook is
the function to be invoked. This breaks down for multi providers,
because the value at the index will be an array of providers, resulting in
the hook being invoked with an array of all the multi provider values,
rather than the provider that was destroyed. In ViewEngine ngOnDestroy
wasn't being called for multi providers at all.

These changes fix the issue by changing the structure of the destroy hooks to [1, hook, 5, [0, hook, 3, hook]] where the indexes inside the inner array point to the provider inside of the multi provider array. Note that this is slightly different from the original design which called for the structure to be [1, hook, 5, [hook, hook], because in the process of implementing it, I realized that we wouldn't get passing the correct context if only some of the multi providers have ngOnDestroy and others don't.

I've run the newly-added view_destroy_hooks benchmark against these changes and compared it to master. The difference seems to be insignificant (between 1% and 2% slower).

Fixes #35231.

PR Close #35840

@galczo5
Copy link
Author

galczo5 commented Jul 26, 2021

@petebacondarwin thx! I'll try to do it and I'll be back with results as soon as possible.

@galczo5
Copy link
Author

galczo5 commented Jul 30, 2021

@petebacondarwin @JoostK @AndrewKushnir

Ok, I'm back with the results. So, good and bad news.

Bad first.
8968b20...10.0.0-next.0 These commits are not the reason of the performance drop and that is strange. Previous tests with version 10.0.0-next.0 had performance issues.

Good news.
By mistake, I found that problem is not connected to angular/angular repo packages. During the binary search, I forgot to update @angular-devkit/build-angular and as I said, there was no performance drop.

So in the end.
10.0.0-next.0 with @angular-devkit/build-angular 0.901.13 works fine.
10.0.0-next.0 with @angular-devkit/build-angular 0.1000.0-next.0 has performance drop.

If I'm not wrong one of angular/angular-cli@v9.0.1...v10.0.0-next.0 commits should be the problem. Should I do the binary search once again?

@JoostK
Copy link
Member

JoostK commented Jul 30, 2021

If I'm not wrong one of angular/angular-cli@v9.0.1...v10.0.0-next.0 commits should be the problem. Should I do the binary search once again?

That sounds like a reasonable plan. Alternatively, you may want to compare the generated bundle across the different CLI versions. Yet another idea might be to lock all Babel packages to the same version as used in v9. I linked a Babel perf issue in my initial comment here and you confirmed that you weren't affected by that, but perhaps there is other things going on.

@galczo5
Copy link
Author

galczo5 commented Jul 31, 2021

I decided to do it. By this time I have a workaround. Angular 12 compiles and works without any problem with the 9.0.1 devkit. There is only one thing that I had to do, I had to change the target from ES5 to ES2015 and solve circular dependencies. There are no performance issues, so that's great!

@donovancarthew @sean-tj @BrkCoder FYI, using this method you'll have all features of Angular 12, but the whole build will be bundled with Webpack 4.

I'll be back with binary search results in the next few days 😄

@galczo5
Copy link
Author

galczo5 commented Aug 2, 2021

Ok, I'll need few more days, the results I got during manual tests with chrome performance profiles are very inconsistent.
In fact, I didn't reproduce the results from Friday.

I'm going to push the upgrade into our automatic performance tests. It'll take some time but should give us better results.
It's hard to believe that after few months of investigation it works as it should 😄

I'll let you know after the automatic tests end.

@donovancarthew
Copy link

For us we found that it actually had something to do with the @angular/flex-layout library that we were using. We were using it to position items within a table cell, for tables that had 50+ rows. Since we were only using flex-layout in a few places, we decided to replace the usage of @angular/flex-layout with just plain css flex-box. This resolved the problem for us. Not sure if this helps anyone else.

@galczo5
Copy link
Author

galczo5 commented Aug 11, 2021

@donovancarthew can you tell me which version of @angular/flex-layout you used?
We don't use this lib, but I can verify if there is a drop using this lib, it can tell us where and why it's not working at full speed. I need to know what happened :D

Our automatic performance tests are in progress so we need a few days more to confirm that the performance drop in our app disappeared.

@galczo5
Copy link
Author

galczo5 commented Sep 6, 2021

We upgraded the app to angular 12. Our automatic performance testing tools confirmed that there is no drop anymore.
Because it disappeared quite suddenly I'm quite sure that it has to be connected to very specific angular usage and it might be hard to reproduce.

Thanks for all your help! 😃

@donovancarthew
Copy link

donovancarthew commented Sep 6, 2021 via email

@AndrewKushnir
Copy link
Contributor

@galczo5 thanks for the update! Great to see that the perf issue is now resolved.

@AndrewKushnir
Copy link
Contributor

There were few other perf-related reports within this thread and if the problem still exists (and there is a chance that it's a framework-related perf regression), please create a new ticket with the necessary details (ideally including a repro), so that we can investigate. Thank you.

@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Oct 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: core Issues related to the framework runtime area: performance needs: clarification This issue needs additional clarification from the reporter before the team can investigate.
Projects
None yet
Development

No branches or pull requests