From 0f3c48ef430605f4d185b77bd043529dac7f9dff Mon Sep 17 00:00:00 2001 From: Chris Garrett Date: Sun, 22 Dec 2019 19:19:37 -0800 Subject: [PATCH 1/5] Adds @memo RFC --- text/0000-memo-decorator.md | 141 ++++++++++++++++++++++++++++++++++++ 1 file changed, 141 insertions(+) create mode 100644 text/0000-memo-decorator.md diff --git a/text/0000-memo-decorator.md b/text/0000-memo-decorator.md new file mode 100644 index 0000000000..9fa0840898 --- /dev/null +++ b/text/0000-memo-decorator.md @@ -0,0 +1,141 @@ +- Start Date: 2019-12-22 +- Relevant Team(s): Ember.js +- RFC PR: (after opening the RFC PR, update this with a link to it and update the file name) +- Tracking: (leave this empty) + +# @memo + +## Summary + +Add a `@memo` decorator for memoizing the result of a getter based on +autotracking. In the following example, `fullName` would only recalculate if +`firstName` or `lastName` is updated. + +```js +class Person { + @tracked firstName = 'Jen'; + @tracked lastName = 'Weber'; + + @memo + get fullName() { + return `${this.firstName} ${this.lastName}`; + } +} +``` + +## Motivation + +One of the major differences between computed properties and tracked properties +with autotracking in Octane is that native, autotracked getters do not +automatically cache their values, where computed properties were cached by +default. This was an intentional design choice, as the memoization logic for +computed properties was actually more costly, on average, than rerunning the +getter in the first place. This was especially true given that computed +properties would usually only ever be calculated and used once or twice per +render before being updated. + +However, there are absolutely cases where getters _are_ expensive, and their +values are used repeatedly, so memoization would be very helpful. Strategic, +opt-in memoization is a useful tool that would help Ember developers optimize +their apps when relevant, without adding extra overhead unless necessary. + +## Detailed design + +The `@memo` decorator will be exported from `@glimmer/tracking`, alongside +`@tracked`. It can be used on native getters to memoize their return values +based on the tracked state they consume while being calculated. + +```js +import { tracked, memo } from '@glimmer/tracking'; + +class Person { + @tracked firstName = 'Jen'; + @tracked lastName = 'Weber'; + + @memo + get fullName() { + return `${this.firstName} ${this.lastName}`; + } +} +``` + +In this example, the `fullName` getter will be memoized whenever it is called, +and will only be recalculated the next time the `firstName` or `lastName` +properties are set. This would apply to any autotracking tags consumed while +calculating the getter, so changes to `EmberArray`s and other tracked +primitives, for instance, would also cause invalidations. + +If used on a non-getter, `@memo` will throw an error in `DEBUG` modes. +Properties can also include a setter, but it won't affect the memoization of the +getter (except by potentially setting the state that was tracked in the first +place). + +### Invalidation + +`@memo` will propagate invalidations whenever any of the properties it is +entangled with are invalidated, causing any downstream state that has consumed +to be invalidated at the same time. + +This is necessary in order to have the memoized value be pulled on again in +general. There is no way to, for instance, only propagate changes if the +memoized value has changed, since we must calculate the memoized value first to +know what it's new value is, and we must propagate the change in order to ensure +that it will be recalculated. + +### Cycles + +Cycles will not be allowed with `@memo`. The cache will only be activated +_after_ the getter has fully calculated, so any cycles will cause infinite +recursion (and eventually, stack overflow), just like un-memoized getters. If a +cycle is detected in `DEBUG` mode, it will throw an error. + +## How we teach this + +`@memo` is not an essential part of the reactivity model, so it shouldn't be +covered during the main component/reactivity guide flow. Instead, it should be +covered in the intermediate/in-depth guides, and in any performance related +guides. + +### API Docs + +TODO + +## Drawbacks + +- Adds extra complexity when programming (whether or not a value should be + memoized is now a decision that has to be made). In general, we should make + sure this is not an issue by recommending that memoization idiomatically _not_ + be used _unless_ it is absolutely necessary. + +- Adds extra overhead for each memoized getter. This again should be addressed + by teaching that it should be avoided when possible. The cost tradeoff should + be noted in documentation in particular to emphasize this, and discourage + overuse. + +- `@memo` may rerun even if the values themselves have not changed, since + tracked properties will always invalidate even if their underlying value did + not change. Unfortunately, this is not really something that `@memo` can + circumvent, since there's no way to tell if a value has actually changed, or + to know which values are being accessed when the memoized value is accessed + until the getter is run. + + Instead, we should be sure that the rules of property invalidation are clear, + and in performance sensitive situations we recommend diff checking when + assigning the property: + + ```js + if (newValue !== this.trackedProp) { + this.trackedProp = newValue; + } + ``` + +## Alternatives + +- `@cached` or `@memoized` could be alternative names. + +- `@memo` could receive arguments of the keys to memoize based on. This would + bring us back to the ergonomics of computed properties, however, and would not + be ideal. It also would bring no actual benefits, except being able to exclude + certain values from recalculation. + + From 35a16d13bb8005580603c42b44ce5c6ff57d6713 Mon Sep 17 00:00:00 2001 From: Chris Garrett Date: Sun, 22 Dec 2019 19:21:10 -0800 Subject: [PATCH 2/5] rename to match PR --- text/{0000-memo-decorator.md => 0566-memo-decorator.md} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename text/{0000-memo-decorator.md => 0566-memo-decorator.md} (98%) diff --git a/text/0000-memo-decorator.md b/text/0566-memo-decorator.md similarity index 98% rename from text/0000-memo-decorator.md rename to text/0566-memo-decorator.md index 9fa0840898..278c0ad1c3 100644 --- a/text/0000-memo-decorator.md +++ b/text/0566-memo-decorator.md @@ -1,6 +1,6 @@ - Start Date: 2019-12-22 - Relevant Team(s): Ember.js -- RFC PR: (after opening the RFC PR, update this with a link to it and update the file name) +- RFC PR: https://github.com/emberjs/rfcs/pull/566 - Tracking: (leave this empty) # @memo From f301756c8ac063df821dc376cb611b15d7c8cf24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Buscht=C3=B6ns?= Date: Tue, 11 Aug 2020 11:09:29 +0200 Subject: [PATCH 3/5] rename `@memo` -> `@cached` https://discord.com/channels/480462759797063690/489814042031161344/742517989156454414 https://github.com/ember-polyfills/ember-cached-decorator-polyfill/issues/2 --- text/0566-memo-decorator.md | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/text/0566-memo-decorator.md b/text/0566-memo-decorator.md index 278c0ad1c3..1a01c99a71 100644 --- a/text/0566-memo-decorator.md +++ b/text/0566-memo-decorator.md @@ -3,11 +3,11 @@ - RFC PR: https://github.com/emberjs/rfcs/pull/566 - Tracking: (leave this empty) -# @memo +# @cached ## Summary -Add a `@memo` decorator for memoizing the result of a getter based on +Add a `@cached` decorator for memoizing the result of a getter based on autotracking. In the following example, `fullName` would only recalculate if `firstName` or `lastName` is updated. @@ -16,7 +16,7 @@ class Person { @tracked firstName = 'Jen'; @tracked lastName = 'Weber'; - @memo + @cached get fullName() { return `${this.firstName} ${this.lastName}`; } @@ -41,18 +41,18 @@ their apps when relevant, without adding extra overhead unless necessary. ## Detailed design -The `@memo` decorator will be exported from `@glimmer/tracking`, alongside +The `@cached` decorator will be exported from `@glimmer/tracking`, alongside `@tracked`. It can be used on native getters to memoize their return values based on the tracked state they consume while being calculated. ```js -import { tracked, memo } from '@glimmer/tracking'; +import { tracked, cached } from '@glimmer/tracking'; class Person { @tracked firstName = 'Jen'; @tracked lastName = 'Weber'; - @memo + @cached get fullName() { return `${this.firstName} ${this.lastName}`; } @@ -65,14 +65,14 @@ properties are set. This would apply to any autotracking tags consumed while calculating the getter, so changes to `EmberArray`s and other tracked primitives, for instance, would also cause invalidations. -If used on a non-getter, `@memo` will throw an error in `DEBUG` modes. +If used on a non-getter, `@cached` will throw an error in `DEBUG` modes. Properties can also include a setter, but it won't affect the memoization of the getter (except by potentially setting the state that was tracked in the first place). ### Invalidation -`@memo` will propagate invalidations whenever any of the properties it is +`@cached` will propagate invalidations whenever any of the properties it is entangled with are invalidated, causing any downstream state that has consumed to be invalidated at the same time. @@ -84,14 +84,14 @@ that it will be recalculated. ### Cycles -Cycles will not be allowed with `@memo`. The cache will only be activated +Cycles will not be allowed with `@cached`. The cache will only be activated _after_ the getter has fully calculated, so any cycles will cause infinite recursion (and eventually, stack overflow), just like un-memoized getters. If a cycle is detected in `DEBUG` mode, it will throw an error. ## How we teach this -`@memo` is not an essential part of the reactivity model, so it shouldn't be +`@cached` is not an essential part of the reactivity model, so it shouldn't be covered during the main component/reactivity guide flow. Instead, it should be covered in the intermediate/in-depth guides, and in any performance related guides. @@ -112,9 +112,9 @@ TODO be noted in documentation in particular to emphasize this, and discourage overuse. -- `@memo` may rerun even if the values themselves have not changed, since +- `@cached` may rerun even if the values themselves have not changed, since tracked properties will always invalidate even if their underlying value did - not change. Unfortunately, this is not really something that `@memo` can + not change. Unfortunately, this is not really something that `@cached` can circumvent, since there's no way to tell if a value has actually changed, or to know which values are being accessed when the memoized value is accessed until the getter is run. @@ -131,9 +131,10 @@ TODO ## Alternatives -- `@cached` or `@memoized` could be alternative names. +- `@memo` or `@memoized` could be alternative names. Initially `@memo` was used, + but it was decided to switch to `@cached`. -- `@memo` could receive arguments of the keys to memoize based on. This would +- `@cached` could receive arguments of the keys to memoize based on. This would bring us back to the ergonomics of computed properties, however, and would not be ideal. It also would bring no actual benefits, except being able to exclude certain values from recalculation. From 959c9b1d2f42f35b5430c7242e8a139764ec851a Mon Sep 17 00:00:00 2001 From: Chris Garrett Date: Fri, 12 Feb 2021 12:35:19 -0800 Subject: [PATCH 4/5] Add API docs --- text/0566-memo-decorator.md | 53 +++++++++++++++++++++++++++++++++++-- 1 file changed, 51 insertions(+), 2 deletions(-) diff --git a/text/0566-memo-decorator.md b/text/0566-memo-decorator.md index 1a01c99a71..9c1c051243 100644 --- a/text/0566-memo-decorator.md +++ b/text/0566-memo-decorator.md @@ -94,11 +94,60 @@ cycle is detected in `DEBUG` mode, it will throw an error. `@cached` is not an essential part of the reactivity model, so it shouldn't be covered during the main component/reactivity guide flow. Instead, it should be covered in the intermediate/in-depth guides, and in any performance related -guides. +guides. We should also note that the decorator should not be sprinkled around +carelessly or defensively - instead, it should only be used when the computation +being cached is confirmed to be computationally expensive. ### API Docs -TODO +The `@cached` decorator can be used on getters in order to cache the return +value of the getter. This is useful when a getter is expensive and used very +often. For instance, in this guest list class, we have the `sortedGuests` +getter that sorts the guests alphabetically: + +```js +import { tracked } from '@glimmer/tracking'; + +class GuestList { + @tracked guests = ['Zoey', 'Tomster']; + + get sortedGuests() { + return this.guests.slice().sort() + } +} +``` + +Every time `sortedGuests` is accessed, a new array will be created and sorted, +because JavaScript getters do not cache by default. When the guest list is +small, like the one in the example, this is not a problem. However, if the guest +list were to grow very large, it would mean that we would be doing a large +amount of work each time we accessed `sortedGetters`. With `@cached`, we can +cache the value instead: + +```js +import { tracked, cached } from '@glimmer/tracking'; + +class GuestList { + @tracked guests = ['Zoey', 'Tomster']; + + @cached + get sortedGuests() { + return this.guests.slice().sort() + } +} +``` + +Now the `sortedGuests` getter will be cached based on _autotracking_. It will +only rerun and create a new sorted array when the `guests` tracked property is +updated. + +In general, you should avoid using `@cached` unless you have confirmed that the +getter you are decorating is computationally expensive. `@cached` adds a small +amount of overhead to the getter, making it more expensive. While this overhead +is small, if `@cached` is overused it can add up to a large impact overall in +your app. Many getters and tracked properties are only accessed once, rendered, +and then never rerendered, so adding `@cached` when it is unnecessary can +negatively impact performance. ## Drawbacks From 7404b446087374c38c342af9ae08d8ed12065ce6 Mon Sep 17 00:00:00 2001 From: Chris Garrett Date: Fri, 12 Feb 2021 12:39:00 -0800 Subject: [PATCH 5/5] Update frontmatter --- text/0566-memo-decorator.md | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/text/0566-memo-decorator.md b/text/0566-memo-decorator.md index 9c1c051243..a1955533ba 100644 --- a/text/0566-memo-decorator.md +++ b/text/0566-memo-decorator.md @@ -1,7 +1,13 @@ -- Start Date: 2019-12-22 -- Relevant Team(s): Ember.js -- RFC PR: https://github.com/emberjs/rfcs/pull/566 -- Tracking: (leave this empty) +--- +Stage: Accepted +Start Date: 2019-12-22 +Release Date: Unreleased +Release Versions: + ember-source: vX.Y.Z + ember-data: vX.Y.Z +Relevant Team(s): Ember.js +RFC PR: https://github.com/emberjs/rfcs/pull/566 +--- # @cached