Skip to content

Commit

Permalink
[REFACTOR] Simplify get and computeds
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Chris Garrett committed Aug 5, 2020
1 parent 3ef8495 commit 72d0216
Show file tree
Hide file tree
Showing 21 changed files with 410 additions and 350 deletions.
3 changes: 2 additions & 1 deletion packages/@ember/-internals/glimmer/lib/environment.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { ENV } from '@ember/-internals/environment';
import { get, set } from '@ember/-internals/metal';
import { _getProp, get, set } from '@ember/-internals/metal';
import { Owner } from '@ember/-internals/owner';
import { getDebugName } from '@ember/-internals/utils';
import { constructStyleDeprecationMessage } from '@ember/-internals/views';
Expand Down Expand Up @@ -117,6 +117,7 @@ export class EmberEnvironmentDelegate implements EnvironmentDelegate<EmberEnviro
public toBool = toBool;
public toIterator = toIterator;

public getProp = _getProp;
public getPath = get;
public setPath = set;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@
@module ember
*/
import { tagForProperty } from '@ember/-internals/metal';
import { isObject } from '@ember/-internals/utils';
import { VMArguments } from '@glimmer/interfaces';
import { VersionedPathReference } from '@glimmer/reference';
import { combine, createUpdatableTag, Tag, updateTag } from '@glimmer/validator';
import { combine, CONSTANT_TAG, createUpdatableTag, Tag, updateTag } from '@glimmer/validator';

/**
This reference is used to get the `[]` tag of iterables, so we can trigger
Expand All @@ -22,7 +23,7 @@ class TrackArrayReference implements VersionedPathReference {
value(): unknown {
let iterable = this.inner.value();

let tag = tagForProperty(iterable, '[]');
let tag = isObject(iterable) ? tagForProperty(iterable, '[]') : CONSTANT_TAG;

updateTag(this.valueTag, tag);

Expand Down
10 changes: 5 additions & 5 deletions packages/@ember/-internals/glimmer/lib/utils/iterator.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { objectAt, tagForProperty } from '@ember/-internals/metal';
import { objectAt } from '@ember/-internals/metal';
import { _contentFor } from '@ember/-internals/runtime';
import { EmberArray, HAS_NATIVE_SYMBOL, isEmberArray, isObject } from '@ember/-internals/utils';
import { Option } from '@glimmer/interfaces';
import { IteratorDelegate } from '@glimmer/reference';
import { consumeTag, isTracking } from '@glimmer/validator';
import { consumeTag, isTracking, tagFor } from '@glimmer/validator';
import { EachInWrapper } from '../helpers/each-in';

export default function toIterator(iterable: unknown): Option<IteratorDelegate> {
Expand Down Expand Up @@ -131,10 +131,10 @@ class ObjectIterator extends BoundedIterator {
// Add the tag of the returned value if it is an array, since arrays
// should always cause updates if they are consumed and then changed
if (isTracking()) {
consumeTag(tagForProperty(obj, key));
consumeTag(tagFor(obj, key));

if (Array.isArray(value) || isEmberArray(value)) {
consumeTag(tagForProperty(value, '[]'));
if (Array.isArray(value)) {
consumeTag(tagFor(value, '[]'));
}
}

Expand Down
7 changes: 6 additions & 1 deletion packages/@ember/-internals/glimmer/lib/utils/to-bool.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
import { get } from '@ember/-internals/metal';
import { get, tagForProperty } from '@ember/-internals/metal';
import { isArray } from '@ember/-internals/runtime';
import { isProxy } from '@ember/-internals/utils';
import { consumeTag } from '@glimmer/validator';

export default function toBool(predicate: unknown): boolean {
if (isProxy(predicate)) {
consumeTag(tagForProperty(predicate as object, 'content'));

return Boolean(get(predicate, 'isTruthy'));
} else if (isArray(predicate)) {
consumeTag(tagForProperty(predicate as object, '[]'));

return (predicate as { length: number }).length !== 0;
} else {
return Boolean(predicate);
Expand Down
54 changes: 42 additions & 12 deletions packages/@ember/-internals/meta/lib/meta.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { symbol, toString } from '@ember/-internals/utils';
import { assert, deprecate } from '@ember/debug';
import { DEBUG } from '@glimmer/env';
import { isDestroyed, isDestroying } from '@glimmer/runtime';
import { UpdatableTag } from '@glimmer/validator';
import { Revision, UpdatableTag } from '@glimmer/validator';

type ObjMap<T> = { [key: string]: T };

Expand Down Expand Up @@ -89,7 +89,10 @@ export class Meta {
_descriptors: Map<string, any> | undefined;
_mixins: any | undefined;
_isInit: boolean;
_lazyChains: ObjMap<ObjMap<UpdatableTag>> | undefined;
_lazyChains: ObjMap<[UpdatableTag, unknown][]> | undefined;
_values: ObjMap<unknown> | undefined;
_tags: ObjMap<UpdatableTag> | undefined;
_revisions: ObjMap<Revision> | undefined;
source: object;
proto: object | undefined;
_parent: Meta | undefined | null;
Expand All @@ -100,16 +103,17 @@ export class Meta {
_flattenedVersion = 0;

// DEBUG
_values: any | undefined;

constructor(obj: object) {
if (DEBUG) {
counters!.metaInstantiated++;
this._values = undefined;
}
this._parent = undefined;
this._descriptors = undefined;
this._mixins = undefined;
this._lazyChains = undefined;
this._values = undefined;
this._tags = undefined;
this._revisions = undefined;

// initial value for all flags right now is false
// see FLAGS const for detailed list of flags used
Expand Down Expand Up @@ -231,21 +235,47 @@ export class Meta {
return false;
}

writableLazyChainsFor(key: string) {
valueFor(key: string): unknown {
let values = this._values;

return values !== undefined ? values[key] : undefined;
}

setValueFor(key: string, value: unknown) {
let values = this._getOrCreateOwnMap('_values');

values[key] = value;
}

revisionFor(key: string): Revision | undefined {
let revisions = this._revisions;

return revisions !== undefined ? revisions[key] : undefined;
}

setRevisionFor(key: string, revision: Revision | undefined) {
let revisions = this._getOrCreateOwnMap('_revisions');

revisions[key] = revision;
}

writableLazyChainsFor(key: string): [UpdatableTag, unknown][] {
if (DEBUG) {
counters!.writableLazyChainsCalls++;
}

let lazyChains = this._getOrCreateOwnMap('_lazyChains');

if (!(key in lazyChains)) {
lazyChains[key] = Object.create(null);
let chains = lazyChains[key];

if (chains === undefined) {
chains = lazyChains[key] = [];
}

return lazyChains[key];
return chains;
}

readableLazyChainsFor(key: string) {
readableLazyChainsFor(key: string): [UpdatableTag, unknown][] | undefined {
if (DEBUG) {
counters!.readableLazyChainsCalls++;
}
Expand Down Expand Up @@ -687,10 +717,10 @@ export const meta: {
counters!.metaCalls++;
}

let maybeMeta = peekMeta(obj);
let maybeMeta = metaStore.get(obj);

// remove this code, in-favor of explicit parent
if (maybeMeta !== null && maybeMeta.source === obj) {
if (maybeMeta !== undefined) {
return maybeMeta;
}

Expand Down
6 changes: 3 additions & 3 deletions packages/@ember/-internals/metal/index.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
export {
default as computed,
autoComputed,
isComputed,
_globalsComputed,
ComputedProperty,
} from './lib/computed';
export { getCacheFor, getCachedValueFor, peekCacheFor } from './lib/computed_cache';
export { default as alias } from './lib/alias';
export { deprecateProperty } from './lib/deprecate_property';
export { PROXY_CONTENT, _getPath, get, getWithDefault } from './lib/property_get';
export { PROXY_CONTENT, _getPath, get, getWithDefault, _getProp } from './lib/property_get';
export { set, trySet } from './lib/property_set';
export {
objectAt,
Expand Down Expand Up @@ -44,7 +44,7 @@ export {
isClassicDecorator,
setClassicDecorator,
} from './lib/descriptor_map';
export { getChainTagsForKey } from './lib/chain-tags';
export { getChainsTag } from './lib/chain-tags';
export { default as libraries, Libraries } from './lib/libraries';
export { default as getProperties } from './lib/get_properties';
export { default as setProperties } from './lib/set_properties';
Expand Down
27 changes: 12 additions & 15 deletions packages/@ember/-internals/metal/lib/alias.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
import { Meta } from '@ember/-internals/meta';
import { Meta, meta as metaFor } from '@ember/-internals/meta';
import { inspect } from '@ember/-internals/utils';
import { assert } from '@ember/debug';
import EmberError from '@ember/error';
import {
combine,
consumeTag,
tagFor,
tagMetaFor,
untrack,
UpdatableTag,
updateTag,
validateTag,
valueForTag,
} from '@glimmer/validator';
import { finishLazyChains, getChainTagsForKey } from './chain-tags';
import { getLastRevisionFor, setLastRevisionFor } from './computed_cache';
import { finishLazyChains, getChainsTag } from './chain-tags';
import {
ComputedDescriptor,
Decorator,
Expand All @@ -23,7 +23,6 @@ import { descriptorForDecorator } from './descriptor_map';
import { defineProperty } from './properties';
import { get } from './property_get';
import { set } from './property_set';
import { tagForProperty } from './tags';

export type AliasDecorator = Decorator & PropertyDecorator & AliasDecoratorImpl;

Expand Down Expand Up @@ -73,27 +72,25 @@ export class AliasedProperty extends ComputedDescriptor {
super.setup(obj, keyName, propertyDesc, meta);
}

teardown(obj: object, keyName: string, meta: Meta): void {
super.teardown(obj, keyName, meta);
}

get(obj: object, keyName: string): any {
let ret: any;

let propertyTag = tagForProperty(obj, keyName) as UpdatableTag;
let meta = metaFor(obj);
let tagMeta = tagMetaFor(obj);
let propertyTag = tagFor(obj, keyName, tagMeta) as UpdatableTag;

// We don't use the tag since CPs are not automatic, we just want to avoid
// anything tracking while we get the altKey
untrack(() => {
ret = get(obj, this.altKey);
});

let lastRevision = getLastRevisionFor(obj, keyName);
let lastRevision = meta.revisionFor(keyName);

if (!validateTag(propertyTag, lastRevision)) {
updateTag(propertyTag, combine(getChainTagsForKey(obj, this.altKey, true)));
setLastRevisionFor(obj, keyName, valueForTag(propertyTag));
finishLazyChains(obj, keyName, ret);
if (lastRevision === undefined || !validateTag(propertyTag, lastRevision)) {
updateTag(propertyTag, getChainsTag(obj, this.altKey, tagMeta, meta));
meta.setRevisionFor(keyName, valueForTag(propertyTag));
finishLazyChains(meta, keyName, ret);
}

consumeTag(propertyTag);
Expand Down
8 changes: 3 additions & 5 deletions packages/@ember/-internals/metal/lib/array_events.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { peekMeta } from '@ember/-internals/meta';
import { peekCacheFor } from './computed_cache';
import { sendEvent } from './events';
import { notifyPropertyChange } from './property_events';

Expand Down Expand Up @@ -61,20 +60,19 @@ export function arrayContentDidChange<T extends { length: number }>(

sendEvent(array, '@array:change', [array, startIdx, removeAmt, addAmt]);

let cache = peekCacheFor(array);
if (cache !== undefined) {
if (meta !== null) {
let length = array.length;
let addedAmount = addAmt === -1 ? 0 : addAmt;
let removedAmount = removeAmt === -1 ? 0 : removeAmt;
let delta = addedAmount - removedAmount;
let previousLength = length - delta;

let normalStartIdx = startIdx < 0 ? previousLength + startIdx : startIdx;
if (cache.has('firstObject') && normalStartIdx === 0) {
if (meta.revisionFor('firstObject') !== undefined && normalStartIdx === 0) {
notifyPropertyChange(array, 'firstObject', meta);
}

if (cache.has('lastObject')) {
if (meta.revisionFor('lastObject') !== undefined) {
let previousLastIndex = previousLength - 1;
let lastAffectedIndex = normalStartIdx + removedAmount;
if (previousLastIndex < lastAffectedIndex) {
Expand Down
Loading

0 comments on commit 72d0216

Please sign in to comment.