diff --git a/mobile/lib/shared/verified_profiles/verified_profile_directory.dart b/mobile/lib/shared/verified_profiles/verified_profile_directory.dart index d57a8301b0..2a64dc6679 100644 --- a/mobile/lib/shared/verified_profiles/verified_profile_directory.dart +++ b/mobile/lib/shared/verified_profiles/verified_profile_directory.dart @@ -61,7 +61,7 @@ class _Batch { /// Records survive disconnects as `checked: false` (cached) and are only /// reconciled by a completed subscription on the current connection. class VerifiedProfilesNotifier extends Notifier> { - final Map _wanted = {}; + late final Map _wanted = newInterestMap(); final Map _heads = {}; final Set _checked = {}; final Map _batches = {}; @@ -76,6 +76,11 @@ class VerifiedProfilesNotifier extends Notifier> { (String, String?)? _scope; Timer? _refreshTimer; + /// The interest map, insertion-ordered by interest. Tests override this to + /// count how many entries eviction visits. + @visibleForTesting + Map newInterestMap() => {}; + @override Map build() { final scope = ref.watch( @@ -128,13 +133,14 @@ class VerifiedProfilesNotifier extends Notifier> { added |= !_wanted.containsKey(key); _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.entries.reduce( - (a, b) => a.value < b.value ? a : b, - ); - _wanted.remove(oldest.key); - _heads.remove(oldest.key); - _checked.remove(oldest.key); + final oldest = _wanted.keys.first; + _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 01f574d509..e6f6bba295 100644 --- a/mobile/test/shared/verified_profiles/verified_profile_directory_test.dart +++ b/mobile/test/shared/verified_profiles/verified_profile_directory_test.dart @@ -1,4 +1,5 @@ import 'dart:async'; +import 'dart:collection'; import 'package:buzz/shared/profile/user_cache_provider.dart'; import 'package:buzz/shared/relay/relay.dart'; @@ -13,10 +14,52 @@ 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. +class _CountingMap extends MapBase { + _CountingMap(this.visits); + + final void Function() visits; + final Map _inner = {}; + + @override + int? operator [](Object? key) => _inner[key]; + @override + void operator []=(String key, int value) => _inner[key] = value; + @override + void clear() => _inner.clear(); + @override + int? remove(Object? key) => _inner.remove(key); + @override + int get length => _inner.length; + @override + bool get isEmpty => _inner.isEmpty; + @override + bool get isNotEmpty => _inner.isNotEmpty; + @override + bool containsKey(Object? key) => _inner.containsKey(key); + @override + Iterable get keys sync* { + for (final key in _inner.keys) { + visits(); + yield key; + } + } +} + +class _CountingNotifier extends VerifiedProfilesNotifier { + _CountingNotifier(this.visits); + + final void Function() visits; + + @override + Map newInterestMap() => _CountingMap(visits); +} + class Harness { - Harness(this.async) { + Harness(this.async, {VerifiedProfilesNotifier Function()? notifier}) { container = ProviderContainer( overrides: [ + if (notifier != null) verifiedProfilesProvider.overrideWith(notifier), ...relay.overrides, verifiedProfileDiscoveryFetchProvider.overrideWith( (ref) => () async { @@ -66,6 +109,26 @@ void journey(String name, void Function(Harness h) body) { } void main() { + test('evicting past the interest limit does not scan the interest map', () { + fakeAsync((async) { + var visits = 0; + final h = Harness( + async, + notifier: () => _CountingNotifier(() => visits++), + ); + 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. + h.container.read(verifiedProfilesProvider.notifier).track([ + for (var i = 1024; i < 4300; i++) key(i), + ]); + expect(visits, lessThanOrEqualTo(4300 - 1024)); + }); + }); + journey('no marks until discovery succeeds and advertises public_profile', ( h, ) { @@ -102,6 +165,23 @@ void main() { expect(sizes..sort(), [2, 128]); }); + journey('a large roster keeps the 1,024 most recently tracked subjects', (h) { + String key(int i) => i.toRadixString(16).padLeft(64, '0'); + h.start([for (var i = 0; i < 10; i++) key(i)]); + final before = h.dFilters.length; + // Tracking a subject again does not refresh its age. + h.container.read(verifiedProfilesProvider.notifier).track([ + key(0), + for (var i = 10; i < 4300; i++) key(i), + ]); + h.settle(); + final subscribed = { + for (final f in h.dFilters.skip(before)) + ...(f['#d'] as List).cast(), + }; + expect(subscribed, {for (var i = 4300 - 1024; i < 4300; i++) key(i)}); + }); + journey('live updates, retractions and ordering apply on the subscription', ( h, ) {