From 40fe4b5d9a98f81230369eec3f1bfcba737d2dd1 Mon Sep 17 00:00:00 2001 From: Tom Brow Date: Tue, 3 Nov 2026 10:32:56 -0800 Subject: [PATCH] perf(mobile): evict verified-profile interest in one pass (#9007) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Follow-up to [#8931](https://github.com/block/buzz/pull/8931), from Wes's [review note](https://github.com/block/buzz/pull/8931#discussion_r4456891410). [#8931](https://github.com/block/buzz/pull/8931) replaced the per-eviction scan in `VerifiedProfilesNotifier.track` with `keys.first`. That was much cheaper than main, but not constant time: removing from a Dart map leaves deleted slots, and each new `keys` iterator walks past them. One `track` call that overflows the 1,024-subject cap by E subjects still costs about E²/2 slot checks. Wes measured ~3 ms at 4,300 subjects, 62 ms at 8,600 and 205 ms at 17,200. Eviction now takes the overflow from a single iteration (`keys.take(overflow).toList()`), so it's linear. Which subjects are kept doesn't change: still the 1,024 most recently inserted. The [#8931](https://github.com/block/buzz/pull/8931) eviction test counted keys yielded by the map, which `keys.first` keeps at one per eviction, so it couldn't fail on the quadratic path. It now counts iterations started over the interest map and expects exactly one per `track` call. Main's scan and the merged `keys.first` loop both start one per eviction. ### Testing - Falsification: with the merged `keys.first` loop restored, the new test fails (`Expected: <1>, Actual: <3276>`). With this change it passes. - `flutter test test/shared/verified_profiles/`: 31 passed. The existing "keeps the 1,024 most recently tracked subjects" journey still passes, so retention is unchanged. - `just mobile-check`: clean. - `just mobile-test`: 4,227 Flutter tests passed, 40 skipped, plus 4 push and 27 crypto tests. - Automated app run: installed this build on an iOS 27 simulator (iPhone 17 Pro) signed in to a Builderlab community, opened the inbox, a channel timeline with images and the image viewer, and the channel details member list, which all track verified-profile subjects. No errors. That community is small (30 members), so the overflow path itself is covered by the unit tests, not the app run. - Agent review: an independent agent review found no blockers. - Human test: waived under the mobile testing exception; the automated app run above replaces it. Not covered: Android, and no new device profile (Wes's AOT numbers above are the cost model; this removes the E² term). buzz-review-completed 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Tom Brow Co-authored-by: Tom Brow Co-authored-by: studio Co-authored-by: Claude Opus 5.5 --- .../verified_profile_directory.dart | 18 ++++++---- .../verified_profile_directory_test.dart | 34 +++++++++---------- 2 files changed, 28 insertions(+), 24 deletions(-) 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); }); });