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.

Sign up for GitHub

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

Jump to bottom

Backport: Remove unused scope from Walker #1357

Merged
merged 5 commits into from
Oct 22, 2021
Merged

Backport: Remove unused scope from Walker #1357

merged 5 commits into from
Oct 22, 2021

Commits on Oct 22, 2021

  1. Use _private instead of #private properties 

    The updates to the template compiler made in support of strict mode
introduced a number of uses of private class fields, some of them in hot
paths. Although all supported versions of Node support private class
fields, `tsc` transpiles private class fields to `WeakMap`s for all
values of `target` lower than `ESNEXT`. As a result, the use of private
class fields results in dramatically higher memory usage and garbage
collection than in the previous version of the template compiler. This
in turn causes at least a large portion of the regression noted in
[emberjs/ember.js#19750][1].

[1]: emberjs/ember.js#19750

Here, we replace *all* private class fields with `_` private fields,
which substantially (thought not 100%) closed the gap with the original.

Co-authored-by: Brenden Palmer <[email protected]>
    @chriskrycho @brendenpalmer
    chriskrycho and brendenpalmer committed Oct 22, 2021
    Configuration menu
    Copy the full SHA
    8563072 View commit details
    Browse the repository at this point in the history

  2. Use Object.assign instead of custom copy function

    @chriskrycho
    chriskrycho committed Oct 22, 2021
    Configuration menu
    Copy the full SHA
    689c402 View commit details
    Browse the repository at this point in the history

  3. Fold another manual copy into Object.assign 

    Previously, this field was being assigned twice: once via direct
assignment in the constructor, and once via the `copy` (previously) or
`Object.assign` (now in this stream of changes) call.
    @chriskrycho
    chriskrycho committed Oct 22, 2021
    Configuration menu
    Copy the full SHA
    b36ef11 View commit details
    Browse the repository at this point in the history

  4. Use internal assign from @glimmer/utils 

    The project already has handling for using `Object.assign` if available
and a polyfill if not; use it!
    @chriskrycho
    chriskrycho committed Oct 22, 2021
    Configuration menu
    Copy the full SHA
    e57c5e0 View commit details
    Browse the repository at this point in the history

  5. Remove unused scope from Walker 

    The `scope` info here is entirely unused in the Glimmer codebase. It may
be used in some AST transforms, but this is fairly unlikely, since it
has only been available since Ember 3.16. Profiling template compilation
highlighted the recursive walking of the whole tree for locals in scope
during syntax parsing as a major cost in CPU usage, so the hope is that
removing this will help.
    @chriskrycho
    chriskrycho committed Oct 22, 2021
    Configuration menu
    Copy the full SHA
    8efb85e View commit details
    Browse the repository at this point in the history