angular / zone.js

Implements Zones for JavaScript
https://github.com/angular/angular/tree/master/packages/zone.js/
MIT License
3.25k stars 408 forks source link

WTF_ISSUE_555 loop garbage collection / unfunctional #1244

Closed sod closed 5 years ago

sod commented 5 years ago

Profiling our page and improving perf I stumbled over zone.js that creates a hefty amount of GC pressure and CPU time.

In particular this method eventTargetLegacyPatch: image

That red bar inside that function is 50% of the load time of zone.js.

Looking in a debugger ... what is that even supposed to do? :D

image

So it takes the length of the string WTF_ISSUE_555, that is a loop of 425 iterations (because the string is 425 characters long) but then uses the index on the array WTF_ISSUE_555_ARRAY that only has 64 elements. And it has an inner loop with 218 elements, so that doubleloop creates 92625 strings and writes/overwrites them into the target object. And that then hold items like abort: "undefined.addEventListener:abort.

This can't be right, is it? :)

Seems like that loop was introduced in https://github.com/angular/zone.js/commit/b3a76d3f8d374be8edf009f7a077e51eefec6a65

zone.js version 0.9.1, inside zone.js/dist/zone.js line 2766 to 2773

JiaLiPassion commented 5 years ago

@sod, thanks for posting the issue, this has been fixed by https://github.com/angular/zone.js/pull/1102, but never get chance to be merged, I will ask the author to rebase or I can fix this again.

sod commented 5 years ago

well, that typo creates twice the amount of memory garbage (12 MB) then an entire vue.js app I wrote.

JiaLiPassion commented 5 years ago

We will fix this one ASAP, thanks for the waiting.

JiaLiPassion commented 5 years ago

And if you are targeting only the evergreen browsers, you can use zone-evergreen.js.

sod commented 5 years ago

The documentation is false regarding loading zone-evergreen and zone-legacy seperatly on https://github.com/angular/zone.js/blob/master/README.md. The documentation states

zone-legacy.js [...] This bundle must be loaded after zone-evergreen.js, zone.js=zone-evergreen.js + zone-legacy.js

It's exactly the other way around. zone-legacy writes a function into _global['__zone_symbol__legacyPatch'] that zone-everygreen calls. That is very nice, because one can just shove it into the legacy polyfill bundle ... just saying :D

JiaLiPassion commented 5 years ago

This should be fixed by https://github.com/angular/angular/pull/31208