mirror of
https://github.com/block/buzz.git
synced 2026-11-14 13:21:02 +00:00
perf(mobile): don't refetch profiles that are already on the wire (#9447)
`UserCacheNotifier.preload` skipped profiles already queued but not ones already on the wire. Opening a channel preloads the same people from several places (compose bar, DM header, member list); a preload that arrives while the first batch is in flight queues those people again, and the next batch downloads their profiles a second time. On Block's Builderlab, opening one channel sent the same 77-author kind 0 request twice, 521 KB each. A preload now leaves out profiles whose batch is in flight and waits for that batch instead. Only profiles that are neither cached, queued nor in flight start a new batch. ### Validation Added test: while a batch for `a` and `b` is on the wire, preloads for `b` and for `b, c` send one more request, for `c` only, and all three preloads complete once their profiles arrive. It fails without this change (the second request asks for `b` again). Existing user cache tests pass; full mobile test suite and `flutter analyze` pass. On an iPhone 12 mini signed in to Block's Builderlab (instrumented profile build), opening two channels in a row: before this change the second channel downloaded its 77 members' profiles twice (1,019 KB of kind 0 events), and the first downloaded its 43 twice (50 KB). With it, each downloaded them once: 509 KB and 25 KB. ### Regressions None expected. A caller whose profiles are in flight now waits for that batch instead of a new one, so it resolves no later than before; a failed in-flight batch reports failure to it, as its own batch would have. --------- Signed-off-by: Tom Brow <tomb@squareup.com> Co-authored-by: Tom Brow <tomb@squareup.com> Co-authored-by: studio <dc2fc481343672df31a689840c09299972e704dac592e0da6f18486646a7ee3e@localhost>
This commit is contained in:
@@ -36,6 +36,9 @@ class UserCacheNotifier extends Notifier<Map<String, UserProfile>> {
|
||||
|
||||
/// Lookups whose batch query is on the wire.
|
||||
final Set<String> _inFlight = {};
|
||||
|
||||
/// Completes when the batch query on the wire has finished.
|
||||
Completer<bool>? _flightCompleter;
|
||||
void Function()? _snapshotWriter;
|
||||
Timer? _snapshotTimer;
|
||||
Map<String, UserProfile> _latest = const {};
|
||||
@@ -175,17 +178,36 @@ class UserCacheNotifier extends Notifier<Map<String, UserProfile>> {
|
||||
final alreadyPending = normalized.any(
|
||||
(pk) => _pending.contains(pk) && !state.containsKey(pk),
|
||||
);
|
||||
// A batch already on the wire answers these; asking again would download
|
||||
// the same profiles twice.
|
||||
final flight = _flightCompleter;
|
||||
final awaitsFlight =
|
||||
flight != null &&
|
||||
normalized.any(
|
||||
(pk) => _inFlight.contains(pk) && !state.containsKey(pk),
|
||||
);
|
||||
final uncached = normalized
|
||||
.where((pk) => !state.containsKey(pk) && !_pending.contains(pk))
|
||||
.where(
|
||||
(pk) =>
|
||||
!state.containsKey(pk) &&
|
||||
!_pending.contains(pk) &&
|
||||
!_inFlight.contains(pk),
|
||||
)
|
||||
.toList();
|
||||
// Restored profiles already render; revalidate them without making the
|
||||
// caller wait on the network.
|
||||
normalized.where(_unrevalidated.contains).forEach(_scheduleFetch);
|
||||
if (uncached.isEmpty && !alreadyPending) return Future.value(true);
|
||||
if (uncached.isEmpty && !alreadyPending) {
|
||||
return awaitsFlight ? flight.future : Future.value(true);
|
||||
}
|
||||
_pending.addAll(uncached);
|
||||
final completer = _batchCompleter ??= Completer<bool>();
|
||||
_scheduleBatch();
|
||||
return completer.future;
|
||||
if (!awaitsFlight) return completer.future;
|
||||
return Future.wait([
|
||||
flight.future,
|
||||
completer.future,
|
||||
]).then((results) => results.every((succeeded) => succeeded));
|
||||
}
|
||||
|
||||
/// Force-refresh profiles for identity-sensitive gates.
|
||||
@@ -249,8 +271,9 @@ class UserCacheNotifier extends Notifier<Map<String, UserProfile>> {
|
||||
|
||||
final pubkeys = _pending.toList();
|
||||
_pending.clear();
|
||||
final completer = _batchCompleter;
|
||||
final completer = _batchCompleter ?? Completer<bool>();
|
||||
_batchCompleter = null;
|
||||
_flightCompleter = completer;
|
||||
|
||||
_flushInFlight = true;
|
||||
_inFlight.addAll(pubkeys);
|
||||
@@ -272,7 +295,7 @@ class UserCacheNotifier extends Notifier<Map<String, UserProfile>> {
|
||||
if (remaining == 0) {
|
||||
// All requested pages are ready.
|
||||
succeeded = true;
|
||||
completer?.complete(true);
|
||||
completer.complete(true);
|
||||
}
|
||||
}
|
||||
} catch (_) {
|
||||
@@ -280,9 +303,8 @@ class UserCacheNotifier extends Notifier<Map<String, UserProfile>> {
|
||||
// restored profile, which the next lookup revalidates again.
|
||||
} finally {
|
||||
if (_isCurrent(generation)) _inFlight.removeAll(pubkeys);
|
||||
if (completer != null && !completer.isCompleted) {
|
||||
completer.complete(succeeded);
|
||||
}
|
||||
if (!completer.isCompleted) completer.complete(succeeded);
|
||||
if (identical(_flightCompleter, completer)) _flightCompleter = null;
|
||||
_flushInFlight = false;
|
||||
if (ref.mounted && _pending.isNotEmpty) _scheduleBatch();
|
||||
}
|
||||
|
||||
@@ -393,6 +393,38 @@ void main() {
|
||||
},
|
||||
);
|
||||
|
||||
test('a preload never asks again for profiles already on the wire', () async {
|
||||
final firstBatch = Completer<List<NostrEvent>>();
|
||||
final session = _RecordingProfileSession(
|
||||
results: [
|
||||
firstBatch.future,
|
||||
Future.value([
|
||||
_profileEvent(id: 'c', pubkey: 'c', createdAt: 1, name: 'C'),
|
||||
]),
|
||||
],
|
||||
);
|
||||
final container = ProviderContainer(
|
||||
overrides: [relaySessionProvider.overrideWith(() => session)],
|
||||
);
|
||||
addTearDown(container.dispose);
|
||||
final cache = container.read(userCacheProvider.notifier);
|
||||
final first = cache.preload(['a', 'b']);
|
||||
await session.firstRequest.future;
|
||||
expect(session.requestCount, 1);
|
||||
|
||||
// b is on the wire; only c is new.
|
||||
final again = cache.preload(['b']);
|
||||
final more = cache.preload(['b', 'c']);
|
||||
firstBatch.complete([
|
||||
_profileEvent(id: 'a', pubkey: 'a', createdAt: 1, name: 'A'),
|
||||
_profileEvent(id: 'b', pubkey: 'b', createdAt: 1, name: 'B'),
|
||||
]);
|
||||
expect(await Future.wait([first, again, more]), everyElement(isTrue));
|
||||
expect(session.requestCount, 2);
|
||||
expect(session.requestedFilter?.authors, ['c']);
|
||||
expect(cache.state.keys, containsAll(['a', 'b', 'c']));
|
||||
});
|
||||
|
||||
test(
|
||||
'refresh verifies real OA batch while allowing main-isolate timers',
|
||||
() async {
|
||||
@@ -495,6 +527,7 @@ class _RecordingProfileSession extends RelaySessionNotifier {
|
||||
final List<Future<List<NostrEvent>>> _results;
|
||||
NostrFilter? requestedFilter;
|
||||
int requestCount = 0;
|
||||
final firstRequest = Completer<void>();
|
||||
|
||||
@override
|
||||
SessionState build() => const SessionState(status: SessionStatus.connected);
|
||||
@@ -506,6 +539,7 @@ class _RecordingProfileSession extends RelaySessionNotifier {
|
||||
}) async {
|
||||
requestCount++;
|
||||
requestedFilter = filter;
|
||||
if (!firstRequest.isCompleted) firstRequest.complete();
|
||||
return _results.isEmpty ? const [] : _results.removeAt(0);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user