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

perf(compiler): avoid copying from prototype while cloning an object #31638

Closed

Conversation

@AndrewKushnir
Copy link
Contributor

commented Jul 18, 2019

This commit updates the _clone function of the _ApplySourceSpanTransformer class, where the for-in loop was used, resulting in copying from prototype to own properties, thus consuming more memory. Prior to NodeJS 12 (V8 versions before 7.4) there was an optimization that was improving the situation and since that logic was removed in favor of other optimizations, the situation with memory consumption caused by the for-in loop got worse. This commit adds a check to make sure we copy only own properties over to cloned object.

Closes #31627.

PR Type

What kind of change does this PR introduce?

  • Other... Please describe: performance improvement

Does this PR introduce a breaking change?

  • Yes
  • No

AndrewKushnir added some commits Jul 18, 2019

perf(compiler): avoid copying from prototype while cloning an object
This commit updates the `_clone` function of the `_ApplySourceSpanTransformer` class, where the for-in loop was used, resulting in copying from prototype to own properties, thus consuming more memory. Prior to NodeJS 12 (V8 versions before 7.4) there was an optimization that was improving the situation and since that logic was removed in favor of other optimizations, the situation with memory consumption caused by the for-in loop got worse. This commit adds a check to make sure we copy only own properties over to cloned object.

Closes #31627.

@AndrewKushnir AndrewKushnir force-pushed the AndrewKushnir:FW-1462_output_ast_clone_fix branch from 1c1e55f to 7f3d1e0 Jul 19, 2019

@AndrewKushnir

This comment has been minimized.

Copy link
Contributor Author

commented Jul 23, 2019

@alxhub

alxhub approved these changes Jul 23, 2019

@kara

kara approved these changes Jul 23, 2019

Copy link
Contributor

left a comment

LGTM

@kara kara unassigned alxhub Jul 23, 2019

@kara kara closed this in 24ca582 Jul 23, 2019

kara added a commit that referenced this pull request Jul 23, 2019

perf(compiler): avoid copying from prototype while cloning an object (#…
…31638)

This commit updates the `_clone` function of the `_ApplySourceSpanTransformer` class, where the for-in loop was used, resulting in copying from prototype to own properties, thus consuming more memory. Prior to NodeJS 12 (V8 versions before 7.4) there was an optimization that was improving the situation and since that logic was removed in favor of other optimizations, the situation with memory consumption caused by the for-in loop got worse. This commit adds a check to make sure we copy only own properties over to cloned object.

Closes #31627.

PR Close #31638
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.