-
-
Notifications
You must be signed in to change notification settings - Fork 408
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
@cached #566
@cached #566
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,197 @@ | ||
--- | ||
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 | ||
|
||
## Summary | ||
|
||
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. | ||
|
||
```js | ||
class Person { | ||
@tracked firstName = 'Jen'; | ||
@tracked lastName = 'Weber'; | ||
|
||
@cached | ||
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 `@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, cached } from '@glimmer/tracking'; | ||
|
||
class Person { | ||
@tracked firstName = 'Jen'; | ||
@tracked lastName = 'Weber'; | ||
|
||
@cached | ||
get fullName() { | ||
return `${this.firstName} ${this.lastName}`; | ||
} | ||
} | ||
``` | ||
|
||
In this example, the `fullName` getter will be memoized whenever it is called, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Although perhaps both say the same thing. |
||
and will only be recalculated the next time the `firstName` or `lastName` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @pzuraq is the opposite true-- if your cached getter relies on zero tracked properties, will the getter just calculate once and then not recalculate? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @roneesh, a But I believe this behavior is not opposite to the one you quoted. |
||
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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Perhaps it was only not clear to me but would changing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That’s exactly what that was meant to capture. Essentially, any autotrack-able change should bust the cache. If it would invalidate the template, it would invalidate |
||
|
||
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 | ||
|
||
`@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. | ||
|
||
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 `@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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is good information! A few other questions I had. It seems this is a per instance memoization technique? Will this only cache the last result (for memory safety)? Do you think these are relevant points or just implementation details that don't need to be flushed out at this time? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This definitely would be per-instance, and it would capture only the last result. I think we can specify it here 👍 |
||
|
||
## How we teach this | ||
|
||
`@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. 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 | ||
|
||
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 | ||
|
||
- 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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A few spots you discussed potential downside but I'm not sure I grasped them simply from reading. Is it cache space, performance, complexity? (and sorry if I didn't read carefully enough) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Every
These costs are tiny, but if every getter in an app was decorated, they would add up. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you add this information to the Maybe the advanced guides could also go into detail about how to performance profile this. Can we make this info easily accessible in the Ember Inspector maybe? |
||
|
||
- 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. | ||
|
||
- `@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 `@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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a bit confusing. If the purpose of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, and that was specified in the original tracked properties RFC: https://github.com/emberjs/rfcs/blob/master/text/0410-tracked-properties.md#manual-invalidation This clause basically is stating that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This would be very interesting to see! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Hmm, so the RFC for "tracked" may state how invalidation works, but to me, the whole purpose of a Maybe a code example would help: class Foo {
@tracked bar;
@memo
get modBar() {
return this.bar + 1;
}
}
const foo = new Foo();
foo.bar = 1;
console.log(foo.modBar);
foo.bar = 1;
console.log(foo.modBar); With this code, the |
||
|
||
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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I especially like this requirement, because equality gets fuzzy when you start working with non-primitives. In the documentation for this, I think it'd be good to include examples of complex equality checking through layer of tracked objects to demonstrate that it would not be possible to performantly check value diffs in any memorization implementation? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I also like that this is laid out in the RFC, but I think it should be part of the the How We Teach This section also. In my previous experience (with Ruby), a memoized value does not change unless the cache itself is reset. Whereas in this case, it's unexpectedly(?) resetting when an upstream value is set. I wonder of either I understand memoization incorrectly or that it really can mean different things? Here's an example of how I've memoized in Ruby: class Foo
def bar
@_bar || some_expensive_method
end
private
def some_expensive_method
# some expensive code
end
end the memoized return value of |
||
this.trackedProp = newValue; | ||
} | ||
``` | ||
|
||
## Alternatives | ||
|
||
- `@memo` or `@memoized` could be alternative names. Initially `@memo` was used, | ||
but it was decided to switch to `@cached`. | ||
|
||
- `@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. | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the title should be more descriptive.