Unverified Commit 68c31d3d authored by Remi Rousselet's avatar Remi Rousselet Committed by GitHub

Use a LinkedList to improve the performance of ChangeNotifier (#62330)

* Use a LinkedList in ChangeNotifier implementation to take O(N^2) notification time to O(N)
parent 56298610
...@@ -2,12 +2,13 @@ ...@@ -2,12 +2,13 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
import 'dart:collection';
import 'package:meta/meta.dart'; import 'package:meta/meta.dart';
import 'assertions.dart'; import 'assertions.dart';
import 'basic_types.dart'; import 'basic_types.dart';
import 'diagnostics.dart'; import 'diagnostics.dart';
import 'observer_list.dart';
/// An object that maintains a list of listeners. /// An object that maintains a list of listeners.
/// ///
...@@ -93,18 +94,22 @@ abstract class ValueListenable<T> extends Listenable { ...@@ -93,18 +94,22 @@ abstract class ValueListenable<T> extends Listenable {
T get value; T get value;
} }
class _ListenerEntry extends LinkedListEntry<_ListenerEntry> {
_ListenerEntry(this.listener);
final VoidCallback listener;
}
/// A class that can be extended or mixed in that provides a change notification /// A class that can be extended or mixed in that provides a change notification
/// API using [VoidCallback] for notifications. /// API using [VoidCallback] for notifications.
/// ///
/// [ChangeNotifier] is optimized for small numbers (one or two) of listeners. /// It is O(1) for adding listeners and O(N) for removing listeners and dispatching
/// It is O(N) for adding and removing listeners and O(N²) for dispatching
/// notifications (where N is the number of listeners). /// notifications (where N is the number of listeners).
/// ///
/// See also: /// See also:
/// ///
/// * [ValueNotifier], which is a [ChangeNotifier] that wraps a single value. /// * [ValueNotifier], which is a [ChangeNotifier] that wraps a single value.
class ChangeNotifier implements Listenable { class ChangeNotifier implements Listenable {
ObserverList<VoidCallback>? _listeners = ObserverList<VoidCallback>(); LinkedList<_ListenerEntry>? _listeners = LinkedList<_ListenerEntry>();
bool _debugAssertNotDisposed() { bool _debugAssertNotDisposed() {
assert(() { assert(() {
...@@ -146,7 +151,7 @@ class ChangeNotifier implements Listenable { ...@@ -146,7 +151,7 @@ class ChangeNotifier implements Listenable {
@override @override
void addListener(VoidCallback listener) { void addListener(VoidCallback listener) {
assert(_debugAssertNotDisposed()); assert(_debugAssertNotDisposed());
_listeners!.add(listener); _listeners!.add(_ListenerEntry(listener));
} }
/// Remove a previously registered closure from the list of closures that are /// Remove a previously registered closure from the list of closures that are
...@@ -171,7 +176,12 @@ class ChangeNotifier implements Listenable { ...@@ -171,7 +176,12 @@ class ChangeNotifier implements Listenable {
@override @override
void removeListener(VoidCallback listener) { void removeListener(VoidCallback listener) {
assert(_debugAssertNotDisposed()); assert(_debugAssertNotDisposed());
_listeners!.remove(listener); for (final _ListenerEntry entry in _listeners!) {
if (entry.listener == listener) {
entry.unlink();
return;
}
}
} }
/// Discards any resources used by the object. After this is called, the /// Discards any resources used by the object. After this is called, the
...@@ -205,27 +215,29 @@ class ChangeNotifier implements Listenable { ...@@ -205,27 +215,29 @@ class ChangeNotifier implements Listenable {
@visibleForTesting @visibleForTesting
void notifyListeners() { void notifyListeners() {
assert(_debugAssertNotDisposed()); assert(_debugAssertNotDisposed());
if (_listeners != null) { if (_listeners!.isEmpty)
final List<VoidCallback> localListeners = List<VoidCallback>.from(_listeners!); return;
for (final VoidCallback listener in localListeners) {
try { final List<_ListenerEntry> localListeners = List<_ListenerEntry>.from(_listeners!);
if (_listeners!.contains(listener))
listener(); for (final _ListenerEntry entry in localListeners) {
} catch (exception, stack) { try {
FlutterError.reportError(FlutterErrorDetails( if (entry.list != null)
exception: exception, entry.listener();
stack: stack, } catch (exception, stack) {
library: 'foundation library', FlutterError.reportError(FlutterErrorDetails(
context: ErrorDescription('while dispatching notifications for $runtimeType'), exception: exception,
informationCollector: () sync* { stack: stack,
yield DiagnosticsProperty<ChangeNotifier>( library: 'foundation library',
'The $runtimeType sending notification was', context: ErrorDescription('while dispatching notifications for $runtimeType'),
this, informationCollector: () sync* {
style: DiagnosticsTreeStyle.errorProperty, yield DiagnosticsProperty<ChangeNotifier>(
); 'The $runtimeType sending notification was',
}, this,
)); style: DiagnosticsTreeStyle.errorProperty,
} );
},
));
} }
} }
} }
......
...@@ -139,6 +139,67 @@ void main() { ...@@ -139,6 +139,67 @@ void main() {
log.clear(); log.clear();
}); });
test('During notifyListeners, a listener was added and removed immediately', () {
final TestNotifier source = TestNotifier();
final List<String> log = <String>[];
final VoidCallback listener3 = () { log.add('listener3'); };
final VoidCallback listener2 = () { log.add('listener2'); };
void listener1() {
log.add('listener1');
source.addListener(listener2);
source.removeListener(listener2);
source.addListener(listener3);
}
source.addListener(listener1);
source.notify();
expect(log, <String>['listener1']);
});
test(
'If a listener in the middle of the list of listeners removes itself, '
'notifyListeners still notifies all listeners', () {
final TestNotifier source = TestNotifier();
final List<String> log = <String>[];
void selfRemovingListener() {
log.add('selfRemovingListener');
source.removeListener(selfRemovingListener);
}
final VoidCallback listener1 = () { log.add('listener1'); };
source.addListener(listener1);
source.addListener(selfRemovingListener);
source.addListener(listener1);
source.notify();
expect(log, <String>['listener1', 'selfRemovingListener', 'listener1']);
});
test('If the first listener removes itself, notifyListeners still notify all listeners', () {
final TestNotifier source = TestNotifier();
final List<String> log = <String>[];
void selfRemovingListener() {
log.add('selfRemovingListener');
source.removeListener(selfRemovingListener);
}
void listener1() {
log.add('listener1');
}
source.addListener(selfRemovingListener);
source.addListener(listener1);
source.notifyListeners();
expect(log, <String>['selfRemovingListener', 'listener1']);
});
test('Merging change notifiers', () { test('Merging change notifiers', () {
final TestNotifier source1 = TestNotifier(); final TestNotifier source1 = TestNotifier();
final TestNotifier source2 = TestNotifier(); final TestNotifier source2 = TestNotifier();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment