mirror of
https://github.com/block/buzz.git
synced 2026-11-04 08:21:48 +00:00
perf(mobile): evict verified-profile interest in one pass (#9007)
## 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 <tomb@squareup.com> Co-authored-by: Tom Brow <tomb@squareup.com> Co-authored-by: studio <dc2fc481343672df31a689840c09299972e704dac592e0da6f18486646a7ee3e@buzz.block.builderlab.xyz> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -134,13 +134,17 @@ class VerifiedProfilesNotifier extends Notifier<Map<String, VerifiedProfile>> {
|
||||
_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;
|
||||
|
||||
@@ -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<String, int> {
|
||||
_CountingMap(this.visits);
|
||||
_CountingMap(this.iterations);
|
||||
|
||||
final void Function() visits;
|
||||
final void Function() iterations;
|
||||
final Map<String, int> _inner = {};
|
||||
|
||||
@override
|
||||
@@ -39,20 +41,18 @@ class _CountingMap extends MapBase<String, int> {
|
||||
bool containsKey(Object? key) => _inner.containsKey(key);
|
||||
@override
|
||||
Iterable<String> 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<String, int> newInterestMap() => _CountingMap(visits);
|
||||
Map<String, int> 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);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user