Implement Glimmer Engine #10501

Merged
merged 321 commits into from May 5, 2015

Projects

None yet
@wycats
Member
wycats commented Feb 21, 2015

Glimmer is the next-generation rendering engine for Ember.js. Built on top of HTMLBars, streams, and an entirely rewritten view layer, Glimmer offers the best performance of any of the major JavaScript frameworks while remaining backwards compatible with Ember 1.x apps.


The Virtual DOM showed the world that render-time diffing delivers awesome performance by minimizing DOM updates and keeping unchanged elements stable. But like any abstraction, it comes with a cost. In this case, constructing the virtual DOM to compare requires at least some subset of components do a full re-render, and building virtual DOM nodes for static areas of your markup that will never change for every change requires many more allocations and increases GC pressure.

Glimmer analyzes templates at compile time, relying on the fact that Handlebars' declarative syntax clearly differentiates between static areas, which make up the majority of a template, and the dynamic areas that can change. Instead of invoking a render() method and diffing the result, Glimmer only walks the (much smaller) existing dynamic tree.


Handlebars' declarative syntax means that from the perspective of an app, the template is being "re-rendered every time", but we can take advantage of static knowledge to reduce the work that we need to do. User code, like helpers and computed properties, are executed during the walk, but only to get (primitive) values which can be === compared, not to build up a new tree. In other words, the programming model is equivalent to "render every time", but we take advantage of the declarative nature of Ember's APIs to reduce work.

Internally, instead of creating a virtual DOM, Glimmer builds a tree of streams, each stream pointing to a node in the DOM. On initial render, we track the last value (always a primitive) that we inserted into the DOM. The streams are not exposed to application code; Glimmer executes user code when necessary and pushes the new value into the stream.

When re-rendering, we walk the tree and flush the streams. If the primitive value produced by the stream has not changed, we do nothing. If it has changed, we write the change to the DOM. Like Virtual DOM approaches, this is a "write-only" algorithm.

One of the benefits of this approach is that we can naturally fuse the Ember/Polymer model of observing individual properties with the React model of explicitly re-rendering areas of the template. No matter what happens, the process of revalidation walks the tree, looking for dirty nodes, and revalidates any dirty nodes it finds.

The only difference is how much of dynamic tree is marked dirty before revalidation begins. Also, because observation can only dirty dynamic nodes (and schedule a top-down revalidation), multiple observers and re-renders that occur during a single run-loop still only produce a single batch of DOM updates.

In essence, the biggest difference between Glimmer and traditional virtual DOM approaches is that we diff values, not DOM. This approach not only lets us avoid the algorithmic complexity of tree diffing (instead, we just === check two values), it also lets us avoid doing many render-time allocations.

Slides from the talk at EmberConf

Work List

(this looks like more things than it actually is)

  • merge subscription change into our branch
  • linkRenderNode can mark stability
  • Ember should return true from linkRenderNode
  • make sure all child streams correctly disconnect from parents
  • add stream pruning logic that correctly prunes ancestors
  • add scope teardown hook to HTMLBars (recursive upon clear or remove)
    • this is also responsible for view layer teardown
  • Propagate correct visitor through descendent hooks (so dirty checks can be avoided for a subtree)
  • Do not dirty check render nodes that were just created (fixes regression)
  • Add "classify" hook to decide which hook content et al should delegate to
  • Make sure that #view is idempotent
  • Components and views should create a new scope frame
    • Implement custom scope in Ember (so that ambient view, controller, etc. can be stored)
    • Eliminate *component* hacks
  • Lifecycle hooks on the renderer
  • Move code duplicated between #view, #component and Ember.View into the Renderer
  • Support reflecting attributes onto angle-bracket components
  • Pass mutable bindings as "mutator" objects
  • Support foo={{action "bar"}}
  • Legacy support: Implement unknownProperty/setUnknownProperty to work with mutator objects
    • Deprecate setting non-mut attributes, and support (mut foo) sexprs in legacy Handlebars syntax
  • Investigate executing teardown callbacks at a "quiescent" state ("vent plasma")
    • Make sure to consider the rpflo stress test situation
  • Implement a zip and map abstraction for streams to clean up ad hoc stream creation
  • Add tests for memory leaks
  • Make sure that custom render functions that mutate the buffer continue to work
  • options.template should not exist if there is no template
  • feature flag all the things
  • Handlebars helper compat
  • Template function that returns a string (legacy)
  • Context shifting #each
  • collection helper
  • {{input}}
    • TextField
    • Checkbox
    • TextArea
  • Ember.Select
  • unbound
    • non-block form
    • block form (partially done, but not completely)
  • #view and view helper
  • #view and view helper with back-compat edge cases
  • #with helper
  • with helper with back-compat edge-cases
  • yield without a block
  • {{render}}
  • ContainerView
  • Top-level view APIs
    • createElement
    • appendTo
    • replaceIn
    • destroyElement
  • isVisible
@wycats wycats referenced this pull request in emberjs/rfcs Feb 21, 2015
Merged

The Road to Ember 2.0 RFC #15

@wycats
Member
wycats commented Feb 22, 2015
React Hook Ember Hook Server? Initial Render? Rerender? Purpose
componentWillMount init Yes Yes No Set initial component state without triggering re-render
componentDidMount didInsertElement No Yes No Provides opportunity for manual DOM manipulation
componentWillReceiveProps willReceiveAttrs No No Yes React to changes in component attributes, so that setState can be invoked before render
shouldComponentUpdate Maybe N/A No No Yes Gives a component an opportunity to reject downstream revalidation
componentWillUpdate willUpdate No No Yes Invoked before a template is re-rendered to give the component an opportunity to inspect the DOM before updates have been applied (example)
componentDidUpdate didUpdate No No Yes Invoked after a template is re-rendered to give the component an opportunity to update the DOM (example)
componentWillUnmount willDestroyElement No No Yes The inverse of componentDidMount/didInsertElement; clean up anything set up in that hook
N/A willRender No Yes Yes In Ember, executed both after init and after willUpdate*
N/A didRender No Yes Yes In Ember, executed both after didInsertElement and didUpdate*

* These hooks can be used in cases where the setup for initial render and subsequent re-renders is idempotent (e.g. $().addClass) instead of duplicating the logic in both places. In most cases, it is better to try to make these hooks idempotent, in keeping with the spirit of "re-render from scratch every time".

@wycats
Member
wycats commented Feb 22, 2015

The above comment is an attempt to classify the hooks in React and Ember so that we can make sure that the new Ember components implement at least the hooks that React folks have identified as being important in the programming model.

@rwjblue
Member
rwjblue commented Feb 22, 2015

All new API's will also need to be feature flagged.

@wycats
Member
wycats commented Feb 22, 2015

@rwjblue yessir

@Linicks
Linicks commented Feb 22, 2015

The init hook name is a little unclear compared to componentWillMount. Could it be named initElelement, or something similar ?

@stefanpenner
Member

The init hook name is a little unclear compared to componentWillMount. Could it be named initElelement, or something similar ?

I would prefer leaving it as init, as it clearly describes what it handles, initialization of the current entity.

That entity just happens to be a component. initFoo as a pattern feels like an exercise in over specification. Especially when you consider the rest of the collaborators all having a unique initFoo method, initModel initRoute etc.

Also worth noting, as we continue to align ourselves with >= ES2015 init will likely become constructor

@stefanpenner
Member

@wycats @tomdale good job guys :)

@alexspeller
Contributor

😍

@alexdiliberto
Contributor

This is amazing 👍 👍 👍

@Linicks
Linicks commented Feb 22, 2015

The problem with just using init is that init is used in so many places in the tech world. In an Ember only world it would be clear and concise, but in an ever increasing complex tech stack it's nice to have that extra bit of clarity. I think that's why FB chose an even more verbose naming scheme than my proposal. I have had the chance/misfortune of maintaining allot of legacy software, and appreciate when the code is self describeing as practical. This is really a preference in either direction, but after being in this game for so many years I'm getting a little opionated :) Either way this is exciting stuff, and look forward to seeing this come together in Ember.

@ebryn
Member
ebryn commented Feb 23, 2015

@Linicks if you look at their ES6 syntax, you'll see they're utilizing the constructor to initialize this.state

@fivetanley
Member

What is a "ShadowRoot"?

@gonvaled

This being a "DOM write-only" implementation, how does Ember deal with changes to the DOM done by third-party components, like jQuery based changes, which are not controlled by Ember?

@davidlormor

"the Dynamic Tree only includes nodes that may have changed (...) and never needs to compare static content that will never change"...sounds fast! 🚀 👍

@stefanpenner
Member

This being a "DOM write-only" implementation, how does Ember deal with changes to the DOM done by third-party components, like jQuery based changes, which are not controlled by Ember?

It doesn't and likely cannot support this. It would be impossible to know what those plugins are doing

@mgenev
mgenev commented Feb 23, 2015

@stefanpenner that means we can no longer use bootstrap (jQiery dependent) in an ember app unless a bootstrap-for-ember version is written, right? Same goes for foundation etc...

@stefanpenner
Member

@stefanpenner that means we can no longer use bootstrap (jQiery dependent) in an ember app unless a bootstrap-for-ember version is written, right? Same goes for foundation etc...

No, this is no different then today. If you component is re-rendered your bootstrap and jquery code needs to re-run.

@wycats
Member
wycats commented Feb 23, 2015

@stefanpenner @mgenev I think there's a little bit of confusion here.

From new-world Ember's perspective, as long as the element for a component still exists in the DOM, it doesn't matter where it has been moved, whether it has been decorated, etc.

There are a few restrictions on moving elements around, but they're very minor and would be easy to abstract. Stay tuned as we continue to make more progress.

@ef4 ef4 commented on an outdated diff Feb 24, 2015
.../ember-htmlbars/tests/helpers/new_each_helper_test.js
+import { default as EmberController } from "ember-runtime/controllers/controller";
+import { Registry } from "ember-runtime/system/container";
+
+import { get } from "ember-metal/property_get";
+import { set } from "ember-metal/property_set";
+import { runAppend, runDestroy } from "ember-runtime/tests/utils";
+
+import compile from "ember-template-compiler/system/compile";
+
+var people, view, registry, container;
+
+// This function lets us write {{#EACH|people|p}} {{p}} {{/each}}
+// and generate:
+//
+// - {{#each p in people}} (legacy)
+// - {{#each people as |p|}} (legacy)
@ef4
ef4 Feb 24, 2015 Contributor

why is {{#each people as |p|}} marked as legacy? Are we not keeping that syntax?

@ef4 ef4 commented on the diff Feb 24, 2015
packages/ember-metal/lib/utils.js
@@ -105,6 +105,14 @@ function intern(str) {
return str;
}
+export function symbol(debugName) {
+ // TODO: Investigate using platform symbols, but we do not
+ // want to require non-enumerability for this API, which
+ // would introduce a large cost.
+
+ return intern(debugName + ' [id=' + GUID_KEY + Math.floor(Math.random() * new Date()) + ']');
@ef4
ef4 Feb 24, 2015 Contributor

Are there really platforms where Math.random() is so broken that we need to multiply by new Date?

@tomdale
tomdale Feb 24, 2015 Member

Dunno, let's have our IE friends run http://jsbin.com/lexonidara/2/edit?html,js,output overnight.

@jdalton
jdalton Mar 4, 2015

Ran this in IE11 overnight and no collisions.

@loopmode
loopmode Mar 6, 2015

I came from using millis to multiplying by random in that particular order, so the idea was to allow for more possible values during a single CPU cycle. Maybe the author had a similar purpose in mind.

@rwjblue rwjblue changed the title from [WIP] <my-component> syntax and stable full re-renders to [WIP] Implement Glimmer Engine Mar 3, 2015
@rwjblue
Member
rwjblue commented Mar 3, 2015

Updated title + description based on EmberConf presentation.

@toblender

Minor correction: teack === track?

@morenoh149

@toblender yeah. I thought he meant teach

@darkbaby123
Contributor

@wycats You mentioned a lot about diffing values to check if they're dirty. I'm curious what's the difference between Glimmer's diffing logic and Angular's dirty-checking.

As I know Angular dirty-checking also checks values (expressions) one by one and execute corresponding callbacks (mostly change doms). But:

  1. Angular doesn't have run loop so it's can not batch update doms.
  2. watchers need to be triggered by $apply() manually sometimes.
@drogus
Contributor
drogus commented Mar 4, 2015

I'm curious what's the difference between Glimmer's diffing logic and Angular's dirty-checking.

These two diffing situations are not happening at the same level. Angular's diffing happens outside of a view layer in order to check if a template change is needed. So let's say that you have a user = { login: 'drogus' } object. And you use it in a template as <span class="login">{{user.login}}</span>. Now, in order to update the template automatically you need to know if a value of the user object was changed. In Angular it's done by dirty checking all of the properties used in templates. In Ember properties are observed, so it knows when a property is changed.

This PR and diffing described here happen in a view layer when a framework already knows that it needs to rerender something, but it needs to figure out what to rerender exactly. DOM is slow, so regenerating the entire template may be costly and here comes the diffing algorithm that React started with.

@bitinn
bitinn commented Mar 5, 2015

One interesting side-effects of React (or other similar dom-diffing) approach is you can worry less about what's being updated in DOM (as oppose to update them manually on value change).

What sort of limit will value-diffing approach impose on DOM manipulation? Would you say this approach is closer to something like Riot than React?

Update: to me, it seems the key differences here are:

  1. dom-diff approach allow user to define almost arbitrary dom update logics, then perform it through a full render.
  2. value-diff approach capture those logic via template/helpers, and can minimize the need to go through static dom tree.
  3. value-diff is conceptually closer to traditional template renderer, though Glimmer use batch dom update (what dom-diff commonly do) to improve performance.

Apologize if I am totally mistaken :)

@wagenet wagenet referenced this pull request Mar 6, 2015
Closed

Reduce dependency on ArrayComputed and ReduceComputed #10582

0 of 2 tasks complete
@mixonic
Member
mixonic commented Mar 8, 2015

@bitinn your understanding is pretty good. I think the comparison to Riot is a good one.

Your points one and two are correct IMO. Point three is a bit off, but I'm not sure what you are thinking when you say "traditional template renderer". A full rerender in glimmer will re-use the static DOM anywhere the template being used has not changed, and will only update dynamic content if it has changed. Where the template has changed, Glimmer will cloneNode a cached fragment of pre-hydrated DOM.

I'm sure the architecture will be discussed in more details as this gets closer to landing.

@jonathanKingston
Contributor

Is the intention to swap out ShadowRoot with document.ShadowRoot eventually?

Tom Dale and... and others added some commits Mar 19, 2015
@tilde-engineering Tom Dale and Yehuda Katz Made more {{#view}} tests pass 22c7c6a
@tilde-engineering Tom Dale and Yehuda Katz Attrs, scope and view destruction cleanup
This commit fixes several edge cases in how scope was managed in
templates, how attrs are looked up, and how views are cleaned up,
particularly the willDestroyElement user hook.
b504a00
@tilde-engineering Tom Dale and Yehuda Katz Take machete to inBuffer state 907910f
@tilde-engineering Tom Dale and Yehuda Katz Implement {{collection}} helper d65c7cb
@tilde-engineering Tom Dale and Yehuda Katz Unpend passing collection tests f39428f
@tilde-engineering Tom Dale and Yehuda Katz Helper for finding pending tests that now pass f9bb21a
@wycats @tilde-engineering wycats Got 75 more tests passing in ember-htmlbars ef9d51e
@wycats @tilde-engineering wycats Left off a bunch of new files 4d2e775
@wycats @tilde-engineering wycats Get more tests passing c7ab85c
@mmun @tilde-engineering mmun Fix style errors 3e61a6a
@wycats @tilde-engineering wycats Get more tests passing 72bbc6f
@mmun @tilde-engineering mmun Mark 3 regressions as skipped 0346b9b
@mmun @tilde-engineering mmun Get action keyword mostly working 9ba35a6
@mmun @tilde-engineering mmun Bump htmlbars version f23f4cd
@rwjblue @tilde-engineering rwjblue Update HTMLBars to v0.12.0.
No longer requires `npm link` for HTMLBars.
f6640d7
@tilde-engineering Tom Dale and Yehuda Katz Fix rebase conflicts and get tests passing 2dd0093
@mixonic @tilde-engineering mixonic Port getViewClientRects, getViewboundingClientRect to Glimmer 6d171bb
@mixonic @tilde-engineering mixonic Drop tests for removed isVirtual behaviors a1bff06
@mixonic @tilde-engineering mixonic Reenable tests related to undefined/null attr values df2e6a9
@tilde-engineering Tom Dale and Yehuda Katz Rerender {{with}} when param changes 992f4af
@tilde-engineering Tom Dale and Yehuda Katz Always set shadowScope’s `view` property 19cd545
@tilde-engineering Tom Dale and Yehuda Katz Fix yield in nested views bfa40e2
@rwjblue rwjblue Use `isStream` helper to check for streamness. 34e28a2
@rwjblue rwjblue Prevent errors in updateScope hook when view is null. 7cc0894
@rwjblue rwjblue Ensure Ember.Handlebars.makeBoundHelper helpers get `options.data.pro…
…perties`.
a3b9668
@mmun mmun Get render keyword mostly working e1fb347
@rwjblue rwjblue Throw an assertion if `makeBoundHelper` is provided a block. 8da40d9
@rwjblue rwjblue Allow Handlebars compat helpers to specify additional dep keys.
In Ember.Handlebars helpers it is possible to add additional dependent
keys:

```javascript
Ember.Handlebars.helper('capitalize-name', function(value) {
  return get(value, 'name').toUpperCase();
}, 'name');
```

---

Unfortunately, the area that we know about the render node and can add
more dependent keys, had no knowledge of the individual helper being rendered.

Thsi commit adds a `findHelper` call to `linkRenderNode` so that we can
determine if additional dependent keys are needed. This is sub-optimal
because the additional `findHelper` may have a negative impact on render
speed, but a better solution is not currently possible.
edcb457
@rwjblue rwjblue Process `fooBinding` in general.
Previously, the `TransformOldBindingSyntax` AST walker was only
processing `{{view}}` invocations. This change makes it process all
helper invocation, and updates a number of related tests.

In order to do that, we had to change the asserts and deprecations in
the transform so that when the `loc` information is not present no
errors are thrown.
878f2cf
@mmun mmun Fix debugger keyword 5e19bfa
@rwjblue rwjblue Fix expected deprecations for recently fixed tests. 7e338d0
@rwjblue rwjblue Update to use new _itemViewTemplate for CollectionView test.
The `{{collection}}` helper sets this automatically, so we need to
ensure the tests follow suit.
6401464
@rwjblue rwjblue Fix Ember.LinkView export test. b120eb1
@rwjblue rwjblue Add `QUnit.skip` to feature flagged tests. 29e6eae
@rwjblue rwjblue Use `expectAssertion` for testing Ember.assert. c868b10
@rwjblue rwjblue Get node tests running.
Some are still skipped, but will require further thought/details on
passing the renderer through to all children in
`ComponentNode.prototype.createOrUpdateComponent`.
3648872
@rwjblue rwjblue Remove unused import.
Fixes JSHint error.
5669af5
@rwjblue rwjblue Enable LOG_VIEW_LOOKUPS for {{outlet}} keyword.
Note: that {{outlet}} templates without a view instance do not get a
default view any longer (the template is just rendered).
89cf0a9
@rwjblue rwjblue Honor `viewName` property with `{{view}}` helper. 5e1787a
@rwjblue rwjblue Restore Component.prototype.targetObject.
This is needed to target the parentView.controller with the action (when
it is provided in the template).

It is possible that in 2.x this conceptually needs to change from
`parentView.controller` to something else, but the general idea will
still remain.
d22789e
mmun and others added some commits Mar 12, 2015
@mmun mmun Split out Dependency and Subscriber classes 386d8e7
@mmun mmun Refactor stream dependencies
Stream dependencies now mock the Stream interface which accomodates some
leaner code.

Stream now has a revalidate hook which is used for manual dependency
management. This hook is invoked in two cases:

  1. The stream just activated.
  2. The stream is already activate and is about to re-compute.
f12ae8b
@mmun mmun Add deactivate hook in Stream, fix KeyStream observers lifecycle
The commit ensures observers for a KeyStream are removed correctly.
A new revalidate hook was introduced as a place to clean up any manual
dependencies. This hook is invoked when the last subscriber is removed
(including when the stream is being destroyed).
218eab7
@rwjblue rwjblue Make Handlebars compat helpers properly stamp themselves as HTMLBars. c8e340b
@rwjblue rwjblue Fix expected deprecation for `attributeBindings`. fa9ccb5
@ef4 ef4 Fixing outlets inside {{render}} helper
Four new passing tests.
dbb7dae
mixonic added some commits May 3, 2015
@mixonic mixonic Bring back the render hook with a pushable buffer de821af
@mixonic mixonic Merge pull request #11012 from mixonic/idempotent-rerender-buffer
Bring back the render hook with a pushable buffer
86ecba9
tilde-engineering and others added some commits May 3, 2015
@tilde-engineering @tomdale tilde-engineering Propagate controller keyword for outlets
5a33e9c
@mixonic mixonic Drop old render_buffer file in favor of compat/ file
658f260
@mixonic mixonic Skipped test for values propagating upstream via templates
42b5df0
@tilde-engineering @tomdale tilde-engineering Fix {{#each}} with itemController in outlet 4f46a23
@tilde-engineering @tomdale tilde-engineering Fix controller local w/ `{{#each itemController}}`
4cfb2bd
@rwjblue rwjblue Make JSCS happy.
6795e0b
@mixonic mixonic Add a (passing?) test for settable upstreaming template bindings
5d38b79
@rwjblue rwjblue Fix propagation test.
a4f9da4
@rwjblue rwjblue Remove deprecation on accessing `attrs` in component root.
We have not fully fleshed out the transition plan for this. Remove the
deprecation until we have...
b010313
@bcardarella
Contributor

ZOMG

@raytiley
Contributor
raytiley commented May 4, 2015

Getting an error pretty regularly about morph.clear() is not a function. Not sure if its related the todo in the code. I'm not exactly sure how you get a logical child that's not in the dom.

windows 7 x64 2015-05-04 10-00-26

rwjblue and others added some commits May 4, 2015
@rwjblue rwjblue Add skipped test showing issue with mutable binding system.
0672868
@wycats wycats Don't warn when for-compat stream is not a path
The `(mut foo)` construction is designed to throw an error if you try to
do `(mut "foo")`, for obvious reasons.

When I wrote that error, I missed an alternative scenario:

```hbs
{{!-- component-one: --}}

{{!-- bar is not a path here --}}
<component-two foo="bat" />

{{!-- component-two: --}}

{{!-- attrs.foo is a path here --}}
<component-three baz={{mut attrs.foo}} />
```

In this case, `attrs.foo` is a path, but it directly refers to a string,
which is itself not mutable.

With angle-bracket components, when `{{mut}}` is explicit, the error
still makes sense: `<component-three>` should not try to mutate
`attrs.foo` and we should warn as soon as we can.

Unfortunately, that logic does not apply to the compatibility work we
are doing:

```hbs
{{!-- component-one: --}}

{{component-two foo="bat"}}

{{!-- component-two: --}}

{{component-three baz=attrs.foo}}
```

In this case, we can't know ahead of time that `attrs.foo` refers to a
non-mutator, so we have to allow for the possibility that this is a
two-way binding. For what it's worth, scenarios like this, which are
clearly bugs (`component-three` settting `baz`), help to explain the
motivation for the change in the first place.

This commit adds a private `@mut` that can support the looser semantics
of Ember 1.x curly components, while retaining the stricter errors for
explicit calls to `mut` used with angle bracket components.
807a0cd
@mmun @rwjblue mmun Intercept property changes instead of sets
(cherry picked from commit b4d53bfd2d2769c4133f5f55e5551c6cc2dc87f8)
27a6c57
@rwjblue rwjblue Unskip mutable binding test. d9e5c80
@rwjblue rwjblue Publish Glimmer builds to S3.
Make testing apps easier...
a233059
@rwjblue rwjblue Allow publishing of idempotent-rerender PR to S3.
12b2496
@rwjblue rwjblue Bring Ember._Metamorph and Ember._MetamorphView back and deprecate.
ember-test-helpers still attempts to register `Ember._Metamorph` as
`view:default` which causes an error. We will need to update
ember-test-helpers to check for its existence first and remove the
globals at a later date.
dddca9a
@rwjblue rwjblue Revert "Allow publishing of idempotent-rerender PR to S3."
This reverts commit 12b2496.

This was accidentally pushing the glimmer builds to `canary` URL's.

Removed for now.  Lets just merge this PR....
ff82f2e
@rwjblue
Member
rwjblue commented May 4, 2015

We have 3 tests still skipped at the moment (the fixes are in various stages of in-flight). I recommend that we merge, and iterate on the remaining issues in canary. To make this a nice stable beta (in roughly a week) we need more users to be able to test these changes in their applications and report any issues they come across.

@emberjs/owners - Any objections?

@wycats
Member
wycats commented May 4, 2015

@rwjblue works for me. I plan to have a blog post explaining the status and remaining open issues in the morning (Greek time).

@chadhietala
Member

:shipit: Will hook it up to an app this week.

@rwjblue rwjblue changed the title from [WIP] Implement Glimmer Engine to Implement Glimmer Engine May 4, 2015
@blesh
Contributor
blesh commented May 4, 2015

So I ran 1.12.0-beta.1+canary.27ed45f8 against https://github.com/blesh/is-ember-really-fast-yet

Unfortunately, I didn't see much improvement for my use case of graphs in an each. Most of the time spent is in scheduleRender, which is a little hard to see in the production build profile.

Here's what the production build profile looks like:

screen shot 2015-05-04 at 4 25 14 pm

It takes about 8-9 seconds to render, as opposed to a baseline of ~1sec. (You can see the baseline if you clone the repo and run it, navigating to http://localhost:4200/basline, thanks to @jeff3dx)

Graphs in an each weren't the only place I've seen this particular issue. Basically any large number of views or components in an each will cause this slowness.

I have the .cpuprofile file if you're all interested.

cc/ @stefanpenner

mixonic and others added some commits May 5, 2015
@mixonic mixonic outlet keywords must pass the template to render as template
Previously, they were passing it as layout which smashed a layout that
may have existed on the view being rendered.
f81b17d
@rwjblue rwjblue Merge pull request #11022 from mixonic/idempotent-rerender-layout
outlet keywords must pass the template to render as template
5bbfe68
@rwjblue
Member
rwjblue commented May 5, 2015

@blesh:

This refactor addresses re-render performance by avoiding creating, destroying, and re-recreating DOM elements and views/components. It does improve the cost of view/component instantiation, but not significantly (which I would suspect is a fairly significant factor in the case you reference). There are a number of contributing factors to the initial view render (as a few others have mentioned), and this PR is definitely a step in the right direction.

This refactor along with some Ember 2.0 spring cleaning and straight up performance work, will address the initial render time speed issues your examples are referencing.

mmun added some commits May 5, 2015
@mmun mmun Fix a memory leak when KeyStream is observering a proxy
535e44b
@mmun mmun Remove dead code
4d177ca
@mmun mmun Privatize bind-attr class compat helper
12cd907
@mmun mmun Remove more dead code, relocate docs
820d559
@sandstrom
Contributor

Awesome work! ⛵️ 🎆 🎈

I'll hook it up to our app and see if I can find some bugs.


@blesh I feel this pain too, but I think it's mostly orthogonal to glimmer. This dedicated issue is better to track the case you mention.

I've created a test-repo without the graph component (easier to investigate performance without third-party code), running on Glimmer (even though I don't think that has much of an effect, for the reasons @rwjblue mentions).

rwjblue added some commits May 5, 2015
@rwjblue rwjblue Add test controller propagation from route driven views.
(cherry picked from commit 035545ed2c47b84f1c104d3e22bded6f72db1a37)
38a9d86
@rwjblue rwjblue Ensure that `controller` is provided for route backed views.
Fixes #11013.
691a5b0
@rwjblue rwjblue merged commit 8962e36 into master May 5, 2015

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@rwjblue rwjblue deleted the idempotent-rerender branch May 5, 2015
@bcardarella
Contributor

@Emerson
Emerson commented May 5, 2015

Congrats on the merge - been following along since Ember Conf - super exciting stuff!

@rondale-sc
Contributor

@rtablada
rtablada commented May 5, 2015

There was much rejoicing

@bcardarella
Contributor

@danmcclain
Contributor

tumblr_m3vi749jwv1r3zat8

@jdjkelly
Contributor
jdjkelly commented May 5, 2015

005385766_2cool

@JPBetley
JPBetley commented May 5, 2015

@chadhietala
Member

@thejameskyle
Contributor

googles "glimmer gif"

@rtablada
rtablada commented May 5, 2015

@thejameskyle results may vary

@jdalton
jdalton commented May 5, 2015

woooooooo!

@jeanleonino

Amen!

@bartocc
bartocc commented May 5, 2015

Congrats !

@Adriaaaaan

Anyone got it working yet? I just get

Uncaught TypeError: Cannot read property 'forEach' of undefined

childviews doesn't exist

    appendChildren: function(view, children, retained) {
        var self = this;
        var childViews = view.get('_childViews');

        childViews.forEach(function(childView) {
          if (!(childView instanceof Ember.Object)) { return; }

          if (self.shouldShowView(childView)) {
            var grandChildren = [];
            children.push({ value: self.inspectView(childView, retained), children: grandChildren });
            self.appendChildren(childView, grandChildren, retained);
          } else {
            self.appendChildren(childView, children, retained);
          }
        });
      },
@mixonic
Member
mixonic commented May 5, 2015

@Adriaaaaan _childViews is a private API that has been removed. Please use view.get('childViews').

@binhums
Contributor
binhums commented May 5, 2015

Woohoo!

@oskarrough

Just updated from 1.11.1 and it took five minutes to get everything working through all your console.logs. So good!

@Fed03
Fed03 commented May 5, 2015

image

@tubbo
tubbo commented May 6, 2015

yay

@btecu
Contributor
btecu commented May 6, 2015

Great work!

@Adriaaaaan

Great work! although our app is currently very broken still I'm already seeing a huge improvement in performance in my list views. Now i just have a million deprecation warnings to fix

@wycats
Member
wycats commented May 6, 2015

@Adriaaaaan That's great news! Please do report any notable breakage. We want things to be pretty clean by the time we ship 1.13 final :)

You might also want to run in prod mode to check out perf. Those deprecation warnings aren't free to generate ;)

@chrism
chrism commented May 6, 2015

Great Work!

Anyone using Liquid Fire should check that ember-animation/liquid-fire#276 has been resolved before updating as it is not yet glimmer compatible.

@jmurphyau
Contributor

@wycats any help you can provide on unbound helpers? A helper that produces a stream like value that can be invalidated causing the value in the template to update? Trying to get ember-get-helper to work with glimmer.. Simply returning a new child stream (e..g objectStream.get(key)) doesn't cause a new value to render in the template when that value changes.. Do I need to look into some of the hooks or somehow subscribe to the new stream?

@jmurphyau
Contributor

I can't seem to find that really detailed description of all the HTMLBars lifecycle hooks etc.. Do you have a link to that that you could provide?

@wycats
Member
wycats commented May 6, 2015

@jmurphyau right now there is no public way for a helper to express dynamic dependencies, but that's a good idea.

Glimmer intentionally doesn't expose the internal stream machinery; it's changing quite a bit still, but it makes sense for a helper to be able to tell Ember when to invalidate it.

It also kind of makes sense for us to include this get helper in Ember itself honestly. It's a primitive, like {{component}}.

@wycats
Member
wycats commented May 6, 2015

@jmurphyau as I said in the blog post, more info on the life cycle hooks is forthcoming :)

@Adriaaaaan

@wycats Hmmm have looked into it more, I'm now seeing a rather significant performance regression. related to interacting with sub resources templates from a list. It seems that interacting with it causes the the whole tree to redraw making it unusable if there are a lot of rows per page. 1.11 is faster, its most notable when selecting the rows (the detail page is very laggy at appearing, the more rows in the list the longer it takes). Although it is clearly much faster at rendering (the page appears much faster) interacting with it is much worse as it seems to be doing more work everytime something changes. I'll see If I can make a simple jsbin

@jiyinyiyong

Hi, you mentioned there's slide in EmberConf 2015. But is there a video?
I can't find it in the list "EmberConf 2015 by Confreaks"
https://www.youtube.com/playlist?list=PLE7tQUdRKcyacwiUPs0CjPYt6tJub4xXU

@igorT
Member
igorT commented May 7, 2015

@jiyinyiyong it's the keynote, towards the end

@jmurphyau jmurphyau referenced this pull request May 7, 2015
Closed

[Proposal] get helper #10878

@jiyinyiyong

@igorT I don't get it. Could you point it out in the video?

found it, thx https://youtu.be/o12-90Dm-Qs?t=3077

@blessenm
Contributor
blessenm commented May 8, 2015

Stupid question. So there is no virtual DOM in ember right? Just value diffing to find which part of the DOM needs to be manipulated.

@axemclion

Glimmer is pretty fast - almost 25% faster !! Ran some tests and results here - http://blog.nparashuram.com/2015/05/performance-boost-in-emberjs-from.html

@jdurand
jdurand commented May 15, 2015

@blessenm instead of diffing DOM it diffs a tree of possibly mutable states that are bound to the real DOM. It doesn't need to maintain a virtual DOM because it has a deeper knowledge of your apps state.

@blessenm
Contributor

@jdurand thanks for the clarification.

@juggy
Contributor
juggy commented on 5a1c5e1 Jun 2, 2015

Why not expose componentNameMap as a global config to enable custom inputs using the same convenient {{input prop type="custom-comp"}} ?

Member

@juggy Not sure exactly what you are asking. Using another type should "just work". E.g. {{input type="color"}} should be fine.

Contributor
juggy replied Jun 3, 2015

type=date-picker to load the component date-picker for example with the value and properties set. But now I realise this is just sugar that might not be good in the long run. Just call the component directly.

I guess what bugs me here is that input either create a checkbox or a textfield, you also have textarea to create a text area. At the end we should get rid of all that and just call the components. Another discussion altogether. So nevermind for now.

Member

@juggy - {{textarea}} creates a <textarea>

Member

@juggy The type="checkbox" isn't actually intended to only be sugar to simplify creation of check box inputs. It's to reflect the fact that both text inputs and check box inputs are the same html tag (input) but have somewhat different semantics. The meaning of the value property is somewhat different between the two, and a checkbox input has a boolean checked property that is meaningless for text inputs. For custom inputs making your own component would definitely be the best way to go (i.e. {{calendar-date-picker-input}}).

@jmurphyau
Contributor

@wycats someone has asked a relatively simple question on stackoverflow and in slack - 'what is a stream'.. I answered as best I could but wanted to bring it up here just so I could get a better understanding - what is a stream? Did the name 'stream' come from anywhere/anything in particular?

I mean - I do know what it is (I think) - just not sure how to best describe it..

@antigremlin

Hi Robert,
Do you know of any future plans to support viewName? Is anything going to change with the upcoming deprecation of views in Ember 2.0? I have posted a question on Ember forums here but no one seems to know the answer.

@matthewp

Is there somewhere that explains how Glimmer works in more detail, with links to code maybe? Given how many commits are included in this PR it's hard to figure out how the details are implemented in practice. For example you say:

On initial render, we track the last value (always a primitive) that we inserted into the DOM.

But I can't find in the code where this happens. And I don't know what you mean by a primitive value. I assume you mean that you track attribute string values and text node's string nodeValues but am having a difficult time finding where this happens in the code.

Is there maybe one or two commits that contain most of the work that I can look at?

@ef4
Contributor
ef4 commented Jun 19, 2015

@matthewp Much of the infrastructure is actually in https://github.com/tildeio/htmlbars

There's not really a single commit to point to -- the work was extensive and many people put in hundreds of commits.

@OrKoN OrKoN referenced this pull request in ember-vcl/meta-list Sep 8, 2015
@piotrpalek piotrpalek remove selected attribute (not needed) 8cd6bb3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment