Pathological view teardown performance

[ef4]

I started investigating this last night but haven't had a chance to finish, so I'm posting the issue in case anybody else sees this or wants to dig in.

@chadhietala created a repo that reproduces this issue. When you toggle between the routes, you'll see a massive delay.

The problem is somewhere in the legacy childViews support. The "controller:before" and "controller:after" actions are firing over 1.1 million times each during the render.

[runspired]

1.1million times o.O that's on a scale I'm not used to hearing.

[stefanpenner]

worst case, we can special case teardown. But this legacy stuff may be causing grief in other areas as well.

[asakusuma]

Did some digging. I don't think these two functions can co-exist, at least in their current state:

ember.js/packages/ember-views/lib/mixins/view_context_support.js

Lines 98 to 104 in 4630cfa

	_legacyControllerDidChange: observer('controller', function() {
	this.walkChildViews(view => view.notifyPropertyChange('controller'));
	}),
	_notifyControllerChange: on('parentViewDidChange', function() {
	this.notifyPropertyChange('controller');
	})

For the repro repo, if you get rid of _notifyControllerChange, you don't have million function calls. Trying to figure out if we can just axe one of them.

[runspired]

_legacyControllerDidChange: observer('controller', function() {
    this.walkChildViews(view => view.notifyPropertyChange('controller'));
  }),

I think if you can't axe this, the answer is wrapping this method in an if legacy clause if possible. Seems like _notifyControllerChange is dependent on an event that wouldn't be present in the legacy case.

[asakusuma]

@runspired yea I've separately tried having each function check to make sure the other doesn't exist, haven't found anything that still passes this test:

ember.js/packages/ember-views/tests/views/view/controller_test.js

Line 7 in 680f997

QUnit.test('controller property should be inherited from nearest ancestor with controller', function() {

The first 4 assertions are fine, it's the last 4 that don't pass. Yo @tomdale, git blame implicates you on both the code and the tests, got any ideas?

[ryanlitalien]

@runspired @asakusuma For my app, I was having an issue with the view coming in as undefined. Error: Uncaught TypeError: Cannot read property 'notifyPropertyChange' of undefined.

I removed the walkChildViews function as @runspired hinted at, and the route transitions were successful. I'm using Ember 2.0.1. I'm not sure what else I could provide to assist in this issue, please let me know.

_legacyControllerDidChange: _emberMetalMixin.observer('controller', function () { }),

[asakusuma]

@ryanlitalien if you can figure out a way to fix the issue and have the tests still pass, that would be awesome. I tried the change you mentioned 3 weeks ago and it broke tests.

[rwjblue]

#12414 fixed the issue reported here