Improve the performances of ChangeNotifier (#71947)

packages/flutter/lib/src/foundation/change_notifier.dart

@@ -2,8 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-import 'dart:collection';
-
 import 'package:meta/meta.dart';
 
 import 'assertions.dart';
@@ -94,11 +92,6 @@ abstract class ValueListenable<T> extends Listenable {
   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
 /// API using [VoidCallback] for notifications.
 ///
@@ -109,11 +102,15 @@ class _ListenerEntry extends LinkedListEntry<_ListenerEntry> {
 ///
 ///  * [ValueNotifier], which is a [ChangeNotifier] that wraps a single value.
 class ChangeNotifier implements Listenable {
-  LinkedList<_ListenerEntry>? _listeners = LinkedList<_ListenerEntry>();
+  int _count = 0;
+  List<VoidCallback?> _listeners = List<VoidCallback?>.filled(0, null);
+  int _notificationCallStackDepth = 0;
+  int _reentrantlyRemovedListeners = 0;
+  bool _debugDisposed = false;
 
   bool _debugAssertNotDisposed() {
     assert(() {
-      if (_listeners == null) {
+      if (_debugDisposed) {
         throw FlutterError(
           'A $runtimeType was used after being disposed.\n'
           'Once you have called dispose() on a $runtimeType, it can no longer be used.',
@@ -142,7 +139,7 @@ class ChangeNotifier implements Listenable {
   @protected
   bool get hasListeners {
     assert(_debugAssertNotDisposed());
-    return _listeners!.isNotEmpty;
+    return _count > 0;
   }
 
   /// Register a closure to be called when the object changes.
@@ -174,7 +171,48 @@ class ChangeNotifier implements Listenable {
   @override
   void addListener(VoidCallback listener) {
     assert(_debugAssertNotDisposed());
-    _listeners!.add(_ListenerEntry(listener));
+    if (_count == _listeners.length) {
+      if (_count == 0) {
+        _listeners = List<VoidCallback?>.filled(1, null);
+      } else {
+        final List<VoidCallback?> newListeners =
+            List<VoidCallback?>.filled(_listeners.length * 2, null);
+        for (int i = 0; i < _count; i++) {
+          newListeners[i] = _listeners[i];
+        }
+        _listeners = newListeners;
+      }
+    }
+    _listeners[_count++] = listener;
+  }
+
+  void _removeAt(int index) {
+    // The list holding the listeners is not growable for performances reasons.
+    // We still want to shrink this list if a lot of listeners have been added
+    // and then removed outside a notifyListeners iteration.
+    // We do this only when the real number of listeners is half the length
+    // of our list.
+    _count -= 1;
+    if (_count * 2 <= _listeners.length) {
+      final List<VoidCallback?> newListeners = List<VoidCallback?>.filled(_count, null);
+
+      // Listeners before the index are at the same place.
+      for (int i = 0; i < index; i++)
+        newListeners[i] = _listeners[i];
+
+      // Listeners after the index move towards the start of the list.
+      for (int i = index; i < _count; i++)
+        newListeners[i] = _listeners[i + 1];
+
+      _listeners = newListeners;
+    } else {
+      // When there are more listeners than half the length of the list, we only
+      // shift our listeners, so that we avoid to reallocate memory for the
+      // whole list.
+      for (int i = index; i < _count; i++)
+        _listeners[i] = _listeners[i + 1];
+      _listeners[_count] = null;
+    }
   }
 
   /// Remove a previously registered closure from the list of closures that are
@@ -193,10 +231,22 @@ class ChangeNotifier implements Listenable {
   @override
   void removeListener(VoidCallback listener) {
     assert(_debugAssertNotDisposed());
-    for (final _ListenerEntry entry in _listeners!) {
-      if (entry.listener == listener) {
-        entry.unlink();
-        return;
+    for (int i = 0; i < _count; i++) {
+      final VoidCallback? _listener = _listeners[i];
+      if (_listener == listener) {
+        if (_notificationCallStackDepth > 0) {
+          // We don't resize the list during notifyListeners iterations
+          // but we set to null, the listeners we want to remove. We will
+          // effectively resize the list at the end of all notifyListeners
+          // iterations.
+          _listeners[i] = null;
+          _reentrantlyRemovedListeners++;
+        } else {
+          // When we are outside the notifyListeners iterations we can
+          // effectively shrink the list.
+          _removeAt(i);
+        }
+        break;
       }
     }
   }
@@ -210,7 +260,10 @@ class ChangeNotifier implements Listenable {
   @mustCallSuper
   void dispose() {
     assert(_debugAssertNotDisposed());
-    _listeners = null;
+    assert(() {
+      _debugDisposed = true;
+      return true;
+    }());
   }
 
   /// Call all the registered listeners.
@@ -232,15 +285,26 @@ class ChangeNotifier implements Listenable {
   @visibleForTesting
   void notifyListeners() {
     assert(_debugAssertNotDisposed());
-    if (_listeners!.isEmpty)
+    if (_count == 0)
       return;
 
-    final List<_ListenerEntry> localListeners = List<_ListenerEntry>.from(_listeners!);
+    // To make sure that listeners removed during this iteration are not called,
+    // we set them to null, but we don't shrink the list right away.
+    // By doing this, we can continue to iterate on our list until it reaches
+    // the last listener added before the call to this method.
+
+    // To allow potential listeners to recursively call notifyListener, we track
+    // the number of times this method is called in _notificationCallStackDepth.
+    // Once every recursive iteration is finished (i.e. when _notificationCallStackDepth == 0),
+    // we can safely shrink our list so that it will only contain not null
+    // listeners.
 
-    for (final _ListenerEntry entry in localListeners) {
+    _notificationCallStackDepth++;
+
+    final int end = _count;
+    for (int i = 0; i < end; i++) {
       try {
-        if (entry.list != null)
-          entry.listener();
+        _listeners[i]?.call();
       } catch (exception, stack) {
         FlutterError.reportError(FlutterErrorDetails(
           exception: exception,
@@ -257,6 +321,44 @@ class ChangeNotifier implements Listenable {
         ));
       }
     }
+
+    _notificationCallStackDepth--;
+
+    if (_notificationCallStackDepth == 0 && _reentrantlyRemovedListeners > 0) {
+      // We really remove the listeners when all notifications are done.
+      final int newLength = _count - _reentrantlyRemovedListeners;
+      if (newLength * 2 <= _listeners.length) {
+        // As in _removeAt, we only shrink the list when the real number of
+        // listeners is half the length of our list.
+        final List<VoidCallback?> newListeners = List<VoidCallback?>.filled(newLength, null);
+
+        int newIndex = 0;
+        for (int i = 0; i < _count; i++) {
+          final VoidCallback? listener = _listeners[i];
+          if (listener != null) {
+            newListeners[newIndex++] = listener;
+          }
+        }
+
+        _listeners = newListeners;
+      } else {
+        // Otherwise we put all the null references at the end.
+        for (int i = 0; i < newLength; i += 1) {
+          if (_listeners[i] == null) {
+            // We swap this item with the next not null item.
+            int swapIndex = i + 1;
+            while(_listeners[swapIndex] == null){
+              swapIndex += 1;
+            }
+            _listeners[i] = _listeners[swapIndex];
+            _listeners[swapIndex] = null;
+          }
+        }
+      }
+
+      _reentrantlyRemovedListeners = 0;
+      _count = newLength;
+    }
   }
 }
 

packages/flutter/test/foundation/change_notifier_test.dart

@@ -20,7 +20,9 @@ class HasListenersTester<T> extends ValueNotifier<T> {
 
 class A {
   bool result = false;
-  void test() { result = true; }
+  void test() {
+    result = true;
+  }
 }
 
 class B extends A with ChangeNotifier {
@@ -31,12 +33,36 @@ class B extends A with ChangeNotifier {
   }
 }
 
+class Counter with ChangeNotifier {
+  int get value => _value;
+  int _value = 0;
+  set value(int value) {
+    if (_value != value) {
+      _value = value;
+      notifyListeners();
+    }
+  }
+
+  void notify() {
+    notifyListeners();
+  }
+}
+
 void main() {
   testWidgets('ChangeNotifier', (WidgetTester tester) async {
     final List<String> log = <String>[];
-    void listener() { log.add('listener'); }
-    void listener1() { log.add('listener1'); }
-    void listener2() { log.add('listener2'); }
+    void listener() {
+      log.add('listener');
+    }
+
+    void listener1() {
+      log.add('listener1');
+    }
+
+    void listener2() {
+      log.add('listener2');
+    }
+
     void badListener() {
       log.add('badListener');
       throw ArgumentError();
@@ -111,9 +137,18 @@ void main() {
     final TestNotifier test = TestNotifier();
     final List<String> log = <String>[];
 
-    void listener1() { log.add('listener1'); }
-    void listener3() { log.add('listener3'); }
-    void listener4() { log.add('listener4'); }
+    void listener1() {
+      log.add('listener1');
+    }
+
+    void listener3() {
+      log.add('listener3');
+    }
+
+    void listener4() {
+      log.add('listener4');
+    }
+
     void listener2() {
       log.add('listener2');
       test.removeListener(listener1);
@@ -137,12 +172,19 @@ void main() {
     log.clear();
   });
 
-  test('During notifyListeners, a listener was added and removed immediately', () {
+  test('During notifyListeners, a listener was added and removed immediately',
+      () {
     final TestNotifier source = TestNotifier();
     final List<String> log = <String>[];
 
-    void listener3() { log.add('listener3'); }
-    void listener2() { log.add('listener2'); }
+    void listener3() {
+      log.add('listener3');
+    }
+
+    void listener2() {
+      log.add('listener2');
+    }
+
     void listener1() {
       log.add('listener1');
       source.addListener(listener2);
@@ -167,7 +209,10 @@ void main() {
       log.add('selfRemovingListener');
       source.removeListener(selfRemovingListener);
     }
-    void listener1() { log.add('listener1'); }
+
+    void listener1() {
+      log.add('listener1');
+    }
 
     source.addListener(listener1);
     source.addListener(selfRemovingListener);
@@ -178,7 +223,9 @@ void main() {
     expect(log, <String>['listener1', 'selfRemovingListener', 'listener1']);
   });
 
-  test('If the first listener removes itself, notifyListeners still notify all listeners', () {
+  test(
+      'If the first listener removes itself, notifyListeners still notify all listeners',
+      () {
     final TestNotifier source = TestNotifier();
     final List<String> log = <String>[];
 
@@ -186,6 +233,7 @@ void main() {
       log.add('selfRemovingListener');
       source.removeListener(selfRemovingListener);
     }
+
     void listener1() {
       log.add('listener1');
     }
@@ -205,8 +253,13 @@ void main() {
     final List<String> log = <String>[];
 
     final Listenable merged = Listenable.merge(<Listenable>[source1, source2]);
-    void listener1() { log.add('listener1'); }
-    void listener2() { log.add('listener2'); }
+    void listener1() {
+      log.add('listener1');
+    }
+
+    void listener2() {
+      log.add('listener2');
+    }
 
     merged.addListener(listener1);
     source1.notify();
@@ -236,8 +289,11 @@ void main() {
     final TestNotifier source2 = TestNotifier();
     final List<String> log = <String>[];
 
-    final Listenable merged = Listenable.merge(<Listenable?>[null, source1, null, source2, null]);
-    void listener() { log.add('listener'); }
+    final Listenable merged =
+        Listenable.merge(<Listenable?>[null, source1, null, source2, null]);
+    void listener() {
+      log.add('listener');
+    }
 
     merged.addListener(listener);
     source1.notify();
@@ -252,7 +308,9 @@ void main() {
     final List<String> log = <String>[];
 
     final Listenable merged = Listenable.merge(<Listenable>[source1, source2]);
-    void listener() { log.add('listener'); }
+    void listener() {
+      log.add('listener');
+    }
 
     merged.addListener(listener);
     source1.notify();
@@ -269,22 +327,32 @@ void main() {
   test('Cannot use a disposed ChangeNotifier', () {
     final TestNotifier source = TestNotifier();
     source.dispose();
-    expect(() { source.addListener(() { }); }, throwsFlutterError);
-    expect(() { source.removeListener(() { }); }, throwsFlutterError);
-    expect(() { source.dispose(); }, throwsFlutterError);
-    expect(() { source.notify(); }, throwsFlutterError);
+    expect(() {
+      source.addListener(() {});
+    }, throwsFlutterError);
+    expect(() {
+      source.removeListener(() {});
+    }, throwsFlutterError);
+    expect(() {
+      source.dispose();
+    }, throwsFlutterError);
+    expect(() {
+      source.notify();
+    }, throwsFlutterError);
   });
 
   test('Value notifier', () {
     final ValueNotifier<double> notifier = ValueNotifier<double>(2.0);
 
     final List<double> log = <double>[];
-    void listener() { log.add(notifier.value); }
+    void listener() {
+      log.add(notifier.value);
+    }
 
     notifier.addListener(listener);
     notifier.value = 3.0;
 
-    expect(log, equals(<double>[ 3.0 ]));
+    expect(log, equals(<double>[3.0]));
     log.clear();
 
     notifier.value = 3.0;
@@ -325,9 +393,10 @@ void main() {
 
     final TestNotifier source1 = TestNotifier();
     final TestNotifier source2 = TestNotifier();
-    void fakeListener() { }
+    void fakeListener() {}
 
-    final Listenable listenableUnderTest = Listenable.merge(<Listenable>[source1, source2]);
+    final Listenable listenableUnderTest =
+        Listenable.merge(<Listenable>[source1, source2]);
     expect(source1.isListenedTo, isFalse);
     expect(source2.isListenedTo, isFalse);
     listenableUnderTest.addListener(fakeListener);
@@ -339,12 +408,11 @@ void main() {
     expect(source2.isListenedTo, isFalse);
   });
 
-
   test('hasListeners', () {
     final HasListenersTester<bool> notifier = HasListenersTester<bool>(true);
     expect(notifier.testHasListeners, isFalse);
-    void test1() { }
-    void test2() { }
+    void test1() {}
+    void test2() {}
     notifier.addListener(test1);
     expect(notifier.testHasListeners, isTrue);
     notifier.addListener(test1);
@@ -388,12 +456,94 @@ void main() {
     }
     expect(error, isNotNull);
     expect(error!, isFlutterError);
-    expect(error.toStringDeep(), equalsIgnoringHashCodes(
-      'FlutterError\n'
-      '   A TestNotifier was used after being disposed.\n'
-      '   Once you have called dispose() on a TestNotifier, it can no\n'
-      '   longer be used.\n'
-    ));
+    expect(
+        error.toStringDeep(),
+        equalsIgnoringHashCodes('FlutterError\n'
+            '   A TestNotifier was used after being disposed.\n'
+            '   Once you have called dispose() on a TestNotifier, it can no\n'
+            '   longer be used.\n'));
+  });
+
+  test('notifyListener can be called recursively', () {
+    final Counter counter = Counter();
+    final List<String> log = <String>[];
+
+    void listener1() {
+      log.add('listener1');
+      if (counter.value < 0) {
+        counter.value = 0;
+      }
+    }
+
+    counter.addListener(listener1);
+    counter.notify();
+    expect(log, <String>['listener1']);
+    log.clear();
+
+    counter.value = 3;
+    expect(log, <String>['listener1']);
+    log.clear();
+
+    counter.value = -2;
+    expect(log, <String>['listener1', 'listener1']);
+    log.clear();
   });
 
+  test('Remove Listeners while notifying on a list which will not resize', () {
+    final TestNotifier test = TestNotifier();
+    final List<String> log = <String>[];
+    final List<VoidCallback> listeners = <VoidCallback>[];
+
+    void autoRemove() {
+      // We remove 4 listeners.
+      // We will end up with (13-4 = 9) listeners.
+      test.removeListener(listeners[1]);
+      test.removeListener(listeners[3]);
+      test.removeListener(listeners[4]);
+      test.removeListener(autoRemove);
+    }
+
+    test.addListener(autoRemove);
+
+    // We add 12 more listeners.
+    for (int i = 0; i < 12; i++) {
+      void listener() {
+        log.add('listener$i');
+      }
+
+      listeners.add(listener);
+      test.addListener(listener);
+    }
+
+    final List<int> remainingListenerIndexes = <int>[
+      0,
+      2,
+      5,
+      6,
+      7,
+      8,
+      9,
+      10,
+      11
+    ];
+    final List<String> expectedLog =
+        remainingListenerIndexes.map((int i) => 'listener$i').toList();
+
+    test.notify();
+    expect(log, expectedLog);
+
+    log.clear();
+    // We expect to have the same result after the removal of previous listeners.
+    test.notify();
+    expect(log, expectedLog);
+
+    // We remove all other listeners.
+    for (int i = 0; i < remainingListenerIndexes.length; i++) {
+      test.removeListener(listeners[remainingListenerIndexes[i]]);
+    }
+
+    log.clear();
+    test.notify();
+    expect(log, <String>[]);
+  });
 }