diff --git a/mobile/lib/shared/verified_profiles/verified_profile_directory.dart b/mobile/lib/shared/verified_profiles/verified_profile_directory.dart index 2a64dc6679..c3a258bb56 100644 --- a/mobile/lib/shared/verified_profiles/verified_profile_directory.dart +++ b/mobile/lib/shared/verified_profiles/verified_profile_directory.dart @@ -134,13 +134,17 @@ class VerifiedProfilesNotifier extends Notifier> { _wanted[key] ??= ++_interest; } // A key's interest is set once, when it is inserted, so the map's - // insertion order is interest order and the oldest key is the first. A - // scan per eviction made one 4,000-member roster take ~300 ms on device. - while (_wanted.length > _wantedLimit) { - final oldest = _wanted.keys.first; - _wanted.remove(oldest); - _heads.remove(oldest); - _checked.remove(oldest); + // insertion order is interest order and the oldest keys come first. Take + // them in one pass: a scan per eviction made one 4,000-member roster take + // ~300 ms on device, and even `keys.first` per eviction is quadratic, + // because each new iterator skips the slots earlier removals left behind. + final overflow = _wanted.length - _wantedLimit; + if (overflow > 0) { + for (final oldest in _wanted.keys.take(overflow).toList()) { + _wanted.remove(oldest); + _heads.remove(oldest); + _checked.remove(oldest); + } added = true; } if (!added) return; diff --git a/mobile/test/shared/verified_profiles/verified_profile_directory_test.dart b/mobile/test/shared/verified_profiles/verified_profile_directory_test.dart index e6f6bba295..cd8d5bd45f 100644 --- a/mobile/test/shared/verified_profiles/verified_profile_directory_test.dart +++ b/mobile/test/shared/verified_profiles/verified_profile_directory_test.dart @@ -14,11 +14,13 @@ import 'package:hooks_riverpod/hooks_riverpod.dart'; import '../../helpers/fake_relay.dart'; import 'verified_profile_fixture.dart'; -/// Counts the keys read through [keys], which [entries] and [values] also use. +/// Counts the iterations started over [keys], which [entries] and [values] +/// also use. Each one costs a walk past every slot that earlier removals left +/// behind, so one per eviction is quadratic even when it stops at the first key. class _CountingMap extends MapBase { - _CountingMap(this.visits); + _CountingMap(this.iterations); - final void Function() visits; + final void Function() iterations; final Map _inner = {}; @override @@ -39,20 +41,18 @@ class _CountingMap extends MapBase { bool containsKey(Object? key) => _inner.containsKey(key); @override Iterable get keys sync* { - for (final key in _inner.keys) { - visits(); - yield key; - } + iterations(); + yield* _inner.keys; } } class _CountingNotifier extends VerifiedProfilesNotifier { - _CountingNotifier(this.visits); + _CountingNotifier(this.iterations); - final void Function() visits; + final void Function() iterations; @override - Map newInterestMap() => _CountingMap(visits); + Map newInterestMap() => _CountingMap(iterations); } class Harness { @@ -109,23 +109,23 @@ void journey(String name, void Function(Harness h) body) { } void main() { - test('evicting past the interest limit does not scan the interest map', () { + test('evicting past the interest limit iterates the interest map once', () { fakeAsync((async) { - var visits = 0; + var iterations = 0; final h = Harness( async, - notifier: () => _CountingNotifier(() => visits++), + notifier: () => _CountingNotifier(() => iterations++), ); addTearDown(h.container.dispose); String key(int i) => i.toRadixString(16).padLeft(64, '0'); h.start([for (var i = 0; i < 1024; i++) key(i)]); - visits = 0; - // Each new subject evicts one: 3,276 evictions. Scanning the map for the - // oldest entry each time would visit about a thousand keys apiece. + iterations = 0; + // 3,276 evictions. Starting an iteration per eviction, whether to scan + // for the oldest entry or to read the first key, is quadratic. h.container.read(verifiedProfilesProvider.notifier).track([ for (var i = 1024; i < 4300; i++) key(i), ]); - expect(visits, lessThanOrEqualTo(4300 - 1024)); + expect(iterations, 1); }); });