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

[BUGFIX] Makes UpdatableTag updates lazy #1010

Merged
merged 2 commits into from
Jan 24, 2020
Merged

[BUGFIX] Makes UpdatableTag updates lazy #1010

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
295312b
[BUGFIX] Makes UpdatableTag updates lazy
Jan 22, 2020
1b06a4f
update tests, remove extraneous assertion logic
Jan 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 40 additions & 14 deletions packages/@glimmer/validator/lib/validators.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,11 @@ class MonomorphicTagImpl implements MonomorphicTag {
private lastValue = INITIAL;

private isUpdating = false;
private subtag: Tag | null = null;
private subtags: Tag[] | null = null;

private subtag: Tag | null = null;
private subtagCachedValue: Revision | null = null;

[TYPE]: MonomorphicTagType;

constructor(type: MonomorphicTagTypes) {
Expand Down Expand Up @@ -176,10 +178,24 @@ class MonomorphicTagImpl implements MonomorphicTag {
}

try {
let { subtags, subtag, revision } = this;
let {
subtags,
subtag,
subtagCachedValue,
lastValue,
revision
} = this;

if (subtag !== null) {
revision = Math.max(revision, subtag[COMPUTE]());
let subtagValue = subtag[COMPUTE]();

if (subtagValue === subtagCachedValue) {
revision = Math.max(revision, lastValue);
} else {
// Clear the temporary cache
this.subtagCachedValue = null;
revision = Math.max(revision, subtagValue);
}
}

if (subtags !== null) {
Expand All @@ -198,27 +214,37 @@ class MonomorphicTagImpl implements MonomorphicTag {
return this.lastValue;
}

static update(_tag: UpdatableTag, subtag: Tag) {
static update(_tag: UpdatableTag, _subtag: Tag) {
if (DEBUG && _tag[TYPE] !== MonomorphicTagTypes.Updatable) {
throw new Error('Attempted to update a tag that was not updatable');
}

// TODO: TS 3.7 should allow us to do this via assertion
let tag = _tag as MonomorphicTagImpl;
let subtag = _subtag as MonomorphicTagImpl;

if (subtag === CONSTANT_TAG) {
tag.subtag = null;
} else {
if (
DEBUG &&
tag.lastChecked === $REVISION &&
(subtag as MonomorphicTagImpl)[COMPUTE]() > tag.lastValue
) {
throw new Error(
'BUG: attempted to update a tag with a tag that has a more recent revision as its value'
);
}

// There are two different possibilities when updating a subtag:
//
// 1. subtag[COMPUTE]() <= tag[COMPUTE]();
// 2. subtag[COMPUTE]() > tag[COMPUTE]();
//
// The first possibility is completely fine within our caching model, but
// the second possibility presents a problem. If the parent tag has
// already been read, then it's value is cached and will not update to
// reflect the subtag's greater value. Next time the cache is busted, the
// subtag's value _will_ be read, and it's value will be _greater_ than
// the saved snapshot of the parent, causing the resulting calculation to
// be rerun erroneously.
//
// In order to prevent this, when we first update to a new subtag we store
// its computed value, and then check against that computed value on
// subsequent updates. If its value hasn't changed, then we return the
// parent's previous value. Once the subtag changes for the first time,
// we clear the cache and everything is finally in sync with the parent.
tag.subtagCachedValue = subtag[COMPUTE]();
tag.subtag = subtag;
}
}
Expand Down