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

[BUGFIX release] Simplify get and computeds #19077

Merged
merged 1 commit into from
Aug 7, 2020
Merged

Conversation

pzuraq
Copy link
Contributor

@pzuraq pzuraq commented Aug 5, 2020

This refactor simplifies get and computeds in the following ways:

  • Removes several brand checks that were unnecessary from get:
    • isEmberArray: Ember Arrays that are not native arrays will
      automatically be tracked through proper usage.
    • isProxy: This was only necessary for conditionals, and updates in
      the VM allow us to autotrack that instead.
  • Uses tagFor instead of tagForProperty in many places including
    get. This allows us to avoid the brand check for
    CUSTOM_PROPERTY_TAG, which is really only necessary for chain tags.
  • Updates everywhere that looks up tags multiple times on the same
    object to use a shared tagMeta so we don't lookup that map multiple
    times.
  • Moves computed revision and value caches onto Meta, so we're looking
    up fewer maps.
  • Creates a new AutoComputed descriptor for computed properties that
    use autotracking, so we can simplify the get logic for standard CPs.

@pzuraq
Copy link
Contributor Author

pzuraq commented Aug 5, 2020

Benchmark results against current master of Ember and Glimmer VM:
artifact-53.pdf

@pzuraq pzuraq changed the title [REFACTOR] Simplify get and computeds [BUGFIX release] Simplify get and computeds Aug 5, 2020
@pzuraq pzuraq force-pushed the refactor/better-get branch 2 times, most recently from 2d7373d to 4543f51 Compare August 5, 2020 22:14
This refactor simplifies get and computeds in the following ways:

- Removes several brand checks that were unnecessary from `get`:
  - `isEmberArray`: Ember Arrays that are not native arrays will
    automatically be tracked through proper usage.
  - `isProxy`: This was only necessary for conditionals, and updates in
    the VM allow us to autotrack that instead.
- Uses `tagFor` instead of `tagForProperty` in many places including
  `get`. This allows us to avoid the brand check for
  `CUSTOM_PROPERTY_TAG`, which is really only necessary for chain tags.
- Updates everywhere that looks up tags multiple times on the same
  object to use a shared `tagMeta` so we don't lookup that map multiple
  times.
- Moves computed revision and value caches onto Meta, so we're looking
  up fewer maps.
- Creates a new AutoComputed descriptor for computed properties that
  use autotracking, so we can simplify the get logic for standard CPs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants