Strengthen animation listener iteration patterns#7566
Merged
abarth merged 1 commit intoflutter:masterfrom Jan 20, 2017
Merged
Conversation
This patch aligns the iteration patterns used by animations and ChangeNotifier. They now both respect re-entrant removal of listeners and coalesce duplication registrations. (Also, ChangeNotifier notification is no longer N^2). This patch introduces ObserverList to avoid the performance regression that the previous version of this patch caused. Fixes flutter#7533
Contributor
Author
|
Attempt 2 |
Hixie
reviewed
Jan 20, 2017
| assert(_debugAssertNotDisposed); | ||
| if (_listeners != null) { | ||
| List<VoidCallback> localListeners = new List<VoidCallback>.from(_listeners); | ||
| final List<VoidCallback> localListeners = new List<VoidCallback>.from(_listeners); |
Contributor
There was a problem hiding this comment.
it's tempting to suggest we should do a copy-on-write during iteration here, since we now control this more closely.
but that's probably a premature optimisation.
Contributor
Author
There was a problem hiding this comment.
Yeah, I'd prefer to wait to see this show up on a profile.
Contributor
|
Please file a bug (or comment on an existing bug) or add a TODO about adding documentation about the fact that you'll get called a surprising number of times if you remove yourself while iterating but you added yourself to multiple listenables that all ended up deferring to the same change notifier. |
Contributor
Author
|
I filed #7570 so we don't lose track of it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This patch aligns the iteration patterns used by animations and
ChangeNotifier. They now both respect re-entrant removal of listeners
and coalesce duplication registrations. (Also, ChangeNotifier
notification is no longer N^2).
This patch introduces ObserverList to avoid the performance regression that the
previous version of this patch caused.
Fixes #7533