Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Optimize away broadcasts (and maybe emits) with no listeners #5341

Closed
kseamon opened this issue Dec 9, 2013 · 3 comments
Closed

Optimize away broadcasts (and maybe emits) with no listeners #5341

kseamon opened this issue Dec 9, 2013 · 3 comments

Comments

@kseamon
Copy link
Contributor

kseamon commented Dec 9, 2013

On my app's more complex pages with lots of nested scopes, $broadcast can become expensive, eating up 30ms for a $location change and 10-20 more if it results in a route change.

Most if not all of the events broadcasted in the process are not used by my app.

I propose the following two optimizations:

  1. Have scope.$on register that an event key has a listener. Short circuit $broadcast if there are no listeners for a given key.
    1a) Extra credit: store a "reference count" along with the registration and decrement it on scope.$destroy in order to keep the registry garbage collected.
  2. Note if a registration is on $rootScope. For events that have only been registered on $rootScope, there's no need to walk down the tree.
@IgorMinar
Copy link
Contributor

instead of having a global registry, create a counter on each scope that will tell us that the current scope or some of its children have a listener for event X.

This means that on each registration or deregistration and scope destcruction we would have to traverse the scope tree from the current scope up to the root scope and decrement the counters.

But given that the registration of event listeners is a relatively rare event it should be super cheap.

@cburgdorf
Copy link
Contributor

  1. Note if a registration is on $rootScope. For events that have only been registered on $rootScope, there's no need to walk down the tree.

I totally agree with your conclusion here. Furthermore I see it as anti pattern when library authors write $rootScope.$broadcast() because what they really want is $rootScope.$emit() (no need to walk down the tree when you want to notify everyone!).

This is why I came up with the proposal of $scope.$onRootscope() which binds to events $emitted (or $broadcasted but that's to avoid!) on the $rootScope but also handles the automatic cleanup when your local $scope gets destroyed.

Given the amount of upvotes this technique received on StackOverflow you could actually call it a best practice I guess:

http://stackoverflow.com/questions/11252780/whats-the-correct-way-to-communicate-between-controllers-in-angularjs/19498009#19498009

I would love to see it addressed in core but so far the technique received a push back from the core team (See #4574)

@ghost ghost assigned tbosch Dec 10, 2013
@kseamon
Copy link
Contributor Author

kseamon commented Dec 10, 2013

PR is in the works. Stay tuned.

kseamon added a commit to kseamon/angular.js that referenced this issue Dec 11, 2013
…stered the event

Update $on and $destroy to maintain a count of event keys registered for each scope and its children.
$broadcast will not descend past a node that has a count of 0/undefined for the $broadcasted event key.

Resolves angular#5341
kseamon added a commit to kseamon/angular.js that referenced this issue Dec 27, 2013
…stered the event

Update $on and $destroy to maintain a count of event keys registered for each scope and its children.
$broadcast will not descend past a node that has a count of 0/undefined for the $broadcasted event key.

Resolves angular#5341
kseamon added a commit to kseamon/angular.js that referenced this issue Dec 27, 2013
…stered the event

Update $on and $destroy to maintain a count of event keys registered for each scope and its children.
$broadcast will not descend past a node that has a count of 0/undefined for the $broadcasted event key.

Resolves angular#5341
kseamon added a commit to kseamon/angular.js that referenced this issue Dec 27, 2013
…stered the event

Update $on and $destroy to maintain a count of event keys registered for each scope and its children.
$broadcast will not descend past a node that has a count of 0/undefined for the $broadcasted event key.

Resolves angular#5341
IgorMinar pushed a commit to IgorMinar/angular.js that referenced this issue Dec 28, 2013
…eners for the event

Update $on and $destroy to maintain a count of event keys registered for each scope and its children.
$broadcast will not descend past a node that has a count of 0/undefined for the $broadcasted event key.

Closes angular#5341
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this issue Jan 27, 2014
…eners for the event

Update $on and $destroy to maintain a count of event keys registered for each scope and its children.
$broadcast will not descend past a node that has a count of 0/undefined for the $broadcasted event key.

Closes angular#5341
Closes angular#5371
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this issue Jan 27, 2014
…eners for the event

Update $on and $destroy to maintain a count of event keys registered for each scope and its children.
$broadcast will not descend past a node that has a count of 0/undefined for the $broadcasted event key.

Closes angular#5341
Closes angular#5371
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants