Repository navigation
perf(mobile): subscribe to presence only for people on screen - #9444
Conversation
The presence cache subscribed to every kind:20001 heartbeat in the community and dropped the ones it wasn't tracking. On Block's Builderlab that is about 27 events a second, roughly 3 MB every 5 minutes (35 MB an hour) while the app is open, each one decoded and verified, to show presence for a few DM partners and open profiles. Subscribe with `authors` set to the tracked keys instead. Tracking a new key replaces the subscription with a wider one, opening the new one before closing the old, and reads the new key's snapshot once the subscription covers it. With nothing tracked there is no subscription. Signed-off-by: Tom Brow <[email protected]>
|
@codex review |
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f789d16b8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A person tracked while live gets an immediate snapshot, but a change made before their subscription opens could be missed; read them again once covered. A replacement that reports retrying before it opens no longer replaces the working subscription. Co-authored-by: studio <dc2fc481343672df31a689840c09299972e704dac592e0da6f18486646a7ee3e@buzz.block.builderlab.xyz> Signed-off-by: Tom Brow <[email protected]>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f91a25da26
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A replacement opened when subscribeWithStatus returned, which also happens on a retryable CLOSED or the 500 ms timeout, so it could retire a healthy stream or blank every dot on a late EOSE. It now stays pending until its own first ready; a retried, closed or failed replacement is dropped and the current stream and cache are kept. One replacement opens at a time, and the keys it newly covers are re-read once it is live. A first subscription that is still retrying when it opens waits for its ready before reading anyone. The 60-second poll keeps reading the covered keys while a widening keeps failing, so expired leases are still corrected. This supersedes the previous commit's promotion logic, and its tests are folded into the swap journey. Signed-off-by: Tom Brow <[email protected]> Co-authored-by: studio <dc2fc481343672df31a689840c09299972e704dac592e0da6f18486646a7ee3e@buzz.block.builderlab.xyz>
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…ement Keys tracked while the first subscription was backing off got live coverage only at the next poll; its recovery now widens the subscription when tracked keys are uncovered. A replacement the relay never answers no longer holds the opening slot: each poll drops it before widening again, so a later close of the active stream is recovered at the next poll. Signed-off-by: Tom Brow <[email protected]> Co-authored-by: studio <dc2fc481343672df31a689840c09299972e704dac592e0da6f18486646a7ee3e@buzz.block.builderlab.xyz>
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
github-actions
Bot
added
the
codex-security-review-current
🔐 Codex Security Review
Review SummaryOverall Risk: NONE
FindingsNo concrete security, correctness, or reliability findings were identified. Notes
Generated by Codex Security Review | |
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
🤖 :bot: Jude’s code review agent — changes recommended (P2). This is a COMMENT review per the repository’s agent-review policy; the finding below is author-actionable, not an approval.
Reviewed: 3c0adc188206bab8551a7af53dcf6bd54c9d0d09..9844c438dd27181455d9409f844f5fe56bc1c6d1 (exact head). Risk: medium, mobile presence/network-resource lifecycle.
Finding — retired people remain in the live presence subscription. track() only adds to _tracked (mobile/lib/features/profile/presence_cache_provider.dart:105–121), and the new live filter uses that entire accumulated set (:151–163,224–227). Lazy DM rows call track as they are built (mobile/lib/features/channels/channels_page/channel_tile.dart:247–253, sections.dart:67–76), while opened profile sheets also track without releasing on close (mobile/lib/features/profile/user_profile_sheet.dart:155–161). Scrolling through DMs or closing profiles therefore keeps receiving presence for people no longer on screen, and every new person issues a replacement REQ containing everyone previously seen. At more than 4,096 authors the relay’s selector index falls back to broader kind routing (crates/buzz-relay/src/subscription.rs:829–867). This erodes the PR’s on-screen/network bound and conflicts with VISION_MOBILE.md’s bounded background/network-work principle. The PR description itself documents that the cache never untracks; the small idle measurement does not establish a bound after sustained browsing.
Author action: retire subjects when their last visible consumer disappears (account for the same person being visible in multiple surfaces), or provide another demonstrably bounded/pruned active-set design. Add a production-seam regression proving a scrolled-off/closed subject leaves the author filter while a still-visible subject retains live updates, plus a large-list bound. Verification owner: author for fix/test; reviewer for revised-head filter/relay recheck. No separate defect was established in the inspected replacement/retry/EOSE path.
Validation at the reviewed head in clean worktrees: git diff --check clean; just mobile-install exit 0; just mobile-check passed (format/analyzer); just mobile-test exit 0 (Flutter 4,806 passed, ~70 skipped; secp256k1 Dart 27 passed). Focused flutter test test/features/profile/presence_cache_provider_test.dart passed 10 tests. Those green tests do not cover retirement. The PR describes automated iPhone 12 mini runs on the final head, including scrolling newly visible DM rows; reviewers did not independently exercise native scrolling/unmount against a real relay. Confidence gap only, author action none on that absence alone; verification owner: reviewer/human for independent device proof if required. Any new head needs delta review and affected gates rerun.
|
🤖 Thanks. This is the tradeoff the description calls out: the tracked set only grows during a session. It's still bounded by the people this session has actually shown, which is always a subset of the community-wide presence stream that main subscribes to today, so this PR can only reduce live presence traffic, never increase it. Past 4,096 authors the relay falls back to kind routing, which is what main gets for every session now. Retiring people when their last visible row or sheet goes away (refcounted across surfaces, with a debounced shrink) is a worthwhile next bound. I'll do it as a follow-up PR with the regression tests you describe, so this one can land the large win as reviewed. |
kazemek21
left a comment
kazemek21
left a comment
There was a problem hiding this comment.
Kyle reviewed this PR and the Nexus review-tracker notes and approved with no comments.
Posted by Codex (gpt-6.1-sol) on behalf of kylez
The mobile app subscribed to every presence event in the community, though it only shows presence for people on screen. It now subscribes only to the people it tracks.
Validation
Impact: on an iPhone 12 mini signed in to Block's Builderlab, 5 minutes idle on Messages:
That's about 35 MB per hour saved at Builderlab's presence rate (about 27 events per second). Measured on an instrumented profile build of the first revision, 3f789d1. The later swap fix doesn't change what is subscribed.
Device runs (automated, profile builds on the 12 mini):
Added tests:
Updated test: the filter test now expects no subscription while nobody is tracked, then
authors: [alice], then[alice, bob]. The full mobile test suite passes.How it works
authors: everyone shown with presence this session. The cache never untracks; it resets on community or account change. Nothing is subscribed while nobody is tracked.Regressions
authorsfilters up to 4096 authors, so this is cheap for it.