Skip to content

Commit

Permalink
Give Glimmer observer-less efficient updates
Browse files Browse the repository at this point in the history
Previously, the Glimmer 2 integration with Ember would recompute values
in curlies on each top-down revalidation.

This commit uses new features of the Glimmer engine to allow it to more
cheaply revalidate a reference, and often skip the computation if the
reference is sure the value cannot have changed.
  • Loading branch information
Godhuda authored and krisselden committed Apr 8, 2016
1 parent 47af7a2 commit e6c4ba0
Show file tree
Hide file tree
Showing 16 changed files with 277 additions and 167 deletions.
1 change: 1 addition & 0 deletions ember-cli-build.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ module.exports = function() {
addGlimmerPackage(vendorPackages, 'glimmer');
addGlimmerPackage(vendorPackages, 'glimmer-compiler');
addGlimmerPackage(vendorPackages, 'glimmer-object');
addGlimmerPackage(vendorPackages, 'glimmer-object-reference');
addGlimmerPackage(vendorPackages, 'glimmer-reference');
addGlimmerPackage(vendorPackages, 'glimmer-runtime');
addGlimmerPackage(vendorPackages, 'glimmer-syntax');
Expand Down
1 change: 1 addition & 0 deletions lib/packages.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ if (glimmerStatus === null || glimmerStatus === true) {
],
testingVendorRequirements: [
'glimmer-object',
'glimmer-object-reference',
'glimmer-engine-tests'
],
hasTemplates: true
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
"express": "^4.5.0",
"finalhandler": "^0.4.0",
"github": "^0.2.3",
"glimmer-engine": "tildeio/glimmer#33e6c9a",
"glimmer-engine": "tildeio/glimmer#b184257",
"glob": "^5.0.13",
"htmlbars": "0.14.16",
"mocha": "^2.4.5",
Expand Down
59 changes: 33 additions & 26 deletions packages/ember-glimmer/lib/components/curly-component.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { StatementSyntax, ValueReference } from 'glimmer-runtime';
import { AttributeBindingReference, applyClassNameBinding } from '../utils/references';
import { AttributeBindingReference, RootReference, applyClassNameBinding } from '../utils/references';
import EmptyObject from 'ember-metal/empty_object';

export class CurlyComponentSyntax extends StatementSyntax {
constructor({ args, definition, templates }) {
Expand All @@ -15,13 +16,13 @@ export class CurlyComponentSyntax extends StatementSyntax {
}
}

function argsToProps(args) {
let attrs = args.named.value();
let attrKeys = Object.keys(attrs);
let merged = { attrs: {} };
function attrsToProps(keys, attrs) {
let merged = new EmptyObject();

for (let i = 0, l = attrKeys.length; i < l; i++) {
let name = attrKeys[i];
merged.attrs = merged;

for (let i = 0, l = keys.length; i < l; i++) {
let name = keys[i];
let value = attrs[name];

// Do we have to support passing both class /and/ classNames...?
Expand All @@ -30,31 +31,34 @@ function argsToProps(args) {
}

merged[name] = value;
merged.attrs[name] = value;
}

return merged;
}

class CurlyComponentManager {
create(definition, args, dynamicScope) {
let klass = definition.ComponentClass;
let component = klass.create(argsToProps(args));
let parentView = dynamicScope.view;

let klass = definition.ComponentClass;
let attrs = args.named.value();
let props = attrsToProps(args.named.keys, attrs);

let component = klass.create(props);

dynamicScope.view = component;
parentView.appendChild(component);

// component.didInitAttrs({ attrs });
// component.didReceiveAttrs({ oldAttrs: null, newAttrs: attrs });
// component.willInsertElement();
// component.willRender();
// component.trigger('didInitAttrs', { attrs });
// component.trigger('didReceiveAttrs', { newAttrs: attrs });
// component.trigger('willInsertElement');
// component.trigger('willRender');

return component;
}

getSelf(component) {
return component;
return new RootReference(component);
}

didCreateElement(component, element, operations) {
Expand Down Expand Up @@ -84,26 +88,29 @@ class CurlyComponentManager {
}

didCreate(component) {
// component.didInsertElement();
// component.didRender();
// component.trigger('didInsertElement');
// component.trigger('didRender');
component._transitionTo('inDOM');
}

update(component, args, dynamicScope) {
component.setProperties(argsToProps(args));
let attrs = args.named.value();
let props = attrsToProps(args.named.keys, attrs);

// let oldAttrs = component.attrs;
// let newAttrs = args.named.value();
//
// component.didUpdateAttrs({ oldAttrs, newAttrs });
// component.didReceiveAttrs({ oldAttrs, newAttrs });
// component.willUpdate();
// component.willRender();
// let newAttrs = attrs;

component.setProperties(props);

// component.trigger('didUpdateAttrs', { oldAttrs, newAttrs });
// component.trigger('didReceiveAttrs', { oldAttrs, newAttrs });
// component.trigger('willUpdate');
// component.trigger('willRender');
}

didUpdate(component) {
// component.didUpdate();
// component.didRender();
// component.trigger('didUpdate');
// component.trigger('didRender');
}

getDestructor(component) {
Expand Down
5 changes: 1 addition & 4 deletions packages/ember-glimmer/lib/components/dynamic-component.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,9 @@ class DynamicComponentReference {
constructor({ nameRef, env }) {
this.nameRef = nameRef;
this.env = env;
this.tag = nameRef.tag;
}

isDirty() { return true; }

value() {
let { env, nameRef } = this;

Expand All @@ -39,6 +38,4 @@ class DynamicComponentReference {
throw new Error(`Cannot render ${name} as a component`);
}
}

destroy() {}
}
28 changes: 9 additions & 19 deletions packages/ember-glimmer/lib/components/outlet.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { StatementSyntax } from 'glimmer-runtime';
import { ConstReference } from 'glimmer-reference';
import { generateGuid, guidFor } from 'ember-metal/utils';
import { RootReference, NULL_REFERENCE } from '../utils/references';

export class OutletSyntax extends StatementSyntax {
constructor({ args }) {
Expand All @@ -26,24 +28,13 @@ function outletComponentFor(args, vm) {
}
}

class TopLevelOutletComponentReference {
class TopLevelOutletComponentReference extends ConstReference {
constructor(reference) {
this.reference = reference;
this.definition = null;
}
let outletState = reference.value();
let definition = new TopLevelOutletComponentDefinition(outletState.render.template);

isDirty() { return true; }

value() {
if (!this.definition) {
let outletState = this.reference.value();
this.definition = new TopLevelOutletComponentDefinition(outletState.render.template);
}

return this.definition;
super(definition);
}

destroy() {}
}

const INVALIDATE = null;
Expand All @@ -54,10 +45,9 @@ class OutletComponentReference {
this.reference = reference;
this.definition = null;
this.lastState = null;
this.tag = reference.tag;
}

isDirty() { return true; }

value() {
let { outletName, reference, definition, lastState } = this;
let newState = reference.value();
Expand Down Expand Up @@ -100,7 +90,7 @@ class AbstractOutletComponentManager {
}

getSelf(state) {
return state.render.controller;
return new RootReference(state.render.controller);
}

getDestructor(state) {
Expand Down Expand Up @@ -138,7 +128,7 @@ class EmptyOutletComponentManager extends AbstractOutletComponentManager {
}

getSelf(state) {
return null;
return NULL_REFERENCE;
}
}

Expand Down
22 changes: 5 additions & 17 deletions packages/ember-glimmer/lib/ember-routing-view/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import assign from 'ember-metal/assign';
import EmptyObject from 'ember-metal/empty_object';
import { DirtyableTag } from 'glimmer-reference';

/**
@module ember
Expand All @@ -9,7 +9,7 @@ import EmptyObject from 'ember-metal/empty_object';
class OutletStateReference {
constructor(outletView) {
this.outletView = outletView;
this.children = new EmptyObject();
this.tag = outletView._tag;
}

get(key) {
Expand All @@ -23,20 +23,13 @@ class OutletStateReference {
get isTopLevel() {
return true;
}

isDirty() {
return true;
}

destroy() {
}
}

class ChildOutletStateReference {
constructor(parent, key) {
this.parent = parent;
this.key = key;
this.children = new EmptyObject();
this.tag = parent.tag;
}

get(key) {
Expand All @@ -50,13 +43,6 @@ class ChildOutletStateReference {
get isTopLevel() {
return false;
}

isDirty() {
return true;
}

destroy() {
}
}

export class OutletView {
Expand Down Expand Up @@ -86,6 +72,7 @@ export class OutletView {
this.template = template;
this.outletState = null;
this._renderResult = null;
this._tag = new DirtyableTag();
}

appendTo(selector) {
Expand All @@ -106,6 +93,7 @@ export class OutletView {

setOutletState(state) {
this.outletState = state;
this._tag.dirty();
this.rerender(); // FIXME
}

Expand Down
15 changes: 8 additions & 7 deletions packages/ember-glimmer/lib/environment.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { Environment as GlimmerEnvironment } from 'glimmer-runtime';
import { isConst } from 'glimmer-reference';
import Dict from 'ember-metal/empty_object';
import { CurlyComponentSyntax, CurlyComponentDefinition } from './components/curly-component';
import { DynamicComponentSyntax } from './components/dynamic-component';
Expand All @@ -9,7 +8,6 @@ import createIterable from './utils/iterable';
import {
RootReference,
ConditionalReference,
ConstConditionalReference,
SimpleHelperReference,
ClassBasedHelperReference
} from './utils/references';
Expand Down Expand Up @@ -119,11 +117,14 @@ export default class Environment extends GlimmerEnvironment {
}

toConditionalReference(reference) {
if (isConst(reference)) {
return new ConstConditionalReference(reference);
} else {
return new ConditionalReference(reference);
}
// if (isConst(reference)) {
// return new ConstConditionalReference(reference);
// } else {
// return new ConditionalReference(reference);
// }

// FIXME: fix failing proxy tests
return new ConditionalReference(reference);
}

iterableFor(ref, args) {
Expand Down
13 changes: 12 additions & 1 deletion packages/ember-glimmer/lib/helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,12 @@
@submodule ember-templates
*/

import symbol from 'ember-metal/symbol';
import Object from 'ember-runtime/system/object';
import { POST_INIT } from 'ember-runtime/system/core_object';
import { DirtyableTag } from 'glimmer-reference';

export const RECOMPUTE_TAG = symbol('RECOMPUTE_TAG');

/**
Ember Helpers are functions that can compute values, and are used in templates.
Expand Down Expand Up @@ -48,6 +53,10 @@ import Object from 'ember-runtime/system/object';
var Helper = Object.extend({
isHelperInstance: true,

[POST_INIT]() {
this[RECOMPUTE_TAG] = new DirtyableTag();
},

/**
On a class-based helper, it may be useful to force a recomputation of that
helpers value. This is akin to `rerender` on a component.
Expand All @@ -72,7 +81,9 @@ var Helper = Object.extend({
@public
@since 1.13.0
*/
recompute() {}
recompute() {
this[RECOMPUTE_TAG].dirty();
}

/**
Override this function when writing a class-based helper.
Expand Down
12 changes: 10 additions & 2 deletions packages/ember-glimmer/lib/helpers/if-unless.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,25 @@
@submodule ember-templates
*/

import { VOLATILE_TAG } from 'glimmer-reference';
import { assert } from 'ember-metal/debug';
import emberToBool from '../utils/to-bool';
import { InternalHelperReference } from '../utils/references';

class ConditionalHelperReference extends InternalHelperReference {
constructor(helper, args) {
super(helper, args);
this.tag = VOLATILE_TAG;
}
}

function makeConditionalHelper(toBool) {
return {
isInternalHelper: true,
toReference(args) {
switch (args.positional.length) {
case 2: return new InternalHelperReference(conditionalWithoutAlternative, args);
case 3: return new InternalHelperReference(conditionalWithAlternative, args);
case 2: return new ConditionalHelperReference(conditionalWithoutAlternative, args);
case 3: return new ConditionalHelperReference(conditionalWithAlternative, args);
default:
assert(
'The inline form of the `if` and `unless` helpers expect two or ' +
Expand Down
Loading

0 comments on commit e6c4ba0

Please sign in to comment.