Skip to content

Performance bug in angular/packages/compiler/src/output/output_ast.ts #31627

Closed
@tebbi

Description

@tebbi

private _clone(obj: any): any {
const clone = Object.create(obj.constructor.prototype);
for (let prop in obj) {
clone[prop] = obj[prop];
}
return clone;
}

This for-in loop also iterates over the prototype chain, copying many functions from the prototypes onto the cloned object as own properties. This doesn't make sense, since the cloned object also has the right prototype, so in a way it has all these functions twice now: on the prototype chain and as own properties.
Putting a check in this loop and only copying own properties will improve performance and avoid wasting memory on unnecessary properties.

In V8 versions before 7.4 (Chrome 74, Node 12), we had an optimization that improved the situation. This is why Node.js observed a huge memory regression for Angular builds when updating V8 in Node.js 12: nodejs/node#28205
That is, we had an optimization that detected function properties that are always the same and shares them in the hidden class instead of actually putting them on the object. This is kind of V8 doing something like prototypes without the user using prototypes. When we enabled constant field tracking, which serves a similar purpose in other cases, we removed this old optimization. This results in us always putting own properties into the object, so they always consume memory, as one might expect actually.

Is this code only used for Angular builds or does it also affect webpages?

Activity

added this to the needsTriage milestone on Jul 18, 2019
modified the milestones: needsTriage, Backlog on Jul 18, 2019
AndrewKushnir

AndrewKushnir commented on Jul 18, 2019

@AndrewKushnir
Contributor

@tebbi thanks a lot for investigation and detailed information provided! 👍

We've created a PR to address this issue (PR #31638).

Is this code only used for Angular builds or does it also affect webpages?

The mentioned code is only used during compilation process and is not invoked on web pages.

Thank you.

filipesilva

filipesilva commented on Jul 19, 2019

@filipesilva
Contributor

@AndrewKushnir won't this compiler code also run at browser app runtime when it is using JIT compilation?

added a commit that references this issue on Jul 19, 2019
1b26797
added a commit that references this issue on Jul 23, 2019
1f3daa0

3 remaining items

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

      Development

      Participants

      @tebbi@filipesilva@alxhub@AndrewKushnir

      Issue actions

        Performance bug in angular/packages/compiler/src/output/output_ast.ts · Issue #31627 · angular/angular