PERF: bound Mail delta and expunge work per sync job (#663)

kayg commented

2026-10-02 13:09:27 +00:00

Owner

Context: #663 rules 8–9; Mail IMAP sync. Source evidence at c4a61e8cf and job/merge-round-7a 2f4482ded. No runtime latency claim.

Evidence at round-7a crates/plugins/mail/src/sync.rs:

:50 UID_WINDOW is 80; :51 MAX_BACKFILL_WINDOWS_PER_RUN is 100.
:549–582 history backfill consumes windows_left.
:597 new-UID loop drains from generation.delta_cursor to latest_uid without checking or consuming windows_left. Each window commits its cursor atomically, but there is no per-run page or elapsed-time limit.
:661 reconcile_expunged_uids collects and sorts SEARCH ALL into one complete live UID set. round-7a crates/plugins/mail/src/cache/store.rs:852 prune_absent_uids loads that set in 500-row chunks into a temp table and prunes memberships in one writer transaction (:863). The chunks do not release the writer connection. This is whole-folder work rather than a restartable bounded job.

Reasoned impact: a large catch-up occupies an ingest slot and one account guard for all its windows. 80,000 new UIDs require 1,000 windows, ten times the advertised history budget. Sparse UID space costs windows even when few messages remain. SQL commits yield to the runtime, but they do not release the job slot to other accounts. Module docs say each run processes a fixed number of windows; this does not hold for delta or expunge reconciliation.

Concrete fix: one shared per-job work budget for history, delta and expunge scans. Persist restartable progress and queue a continuation with the existing dedup/slot model. Do not publish a final folder reconciliation before a complete scan. Prefer provider delta capabilities when supported; keep a bounded fallback. Preserve page+cursor atomicity and UIDVALIDITY checks.

Tests: feed more than 100 delta windows and assert the first job stops at its budget; continuations converge with no skipped or duplicate UID records. Test sparse high UIDs, expunges concurrent with new arrivals, provider failure and restart. Demonstrate another account progresses between batches. Extend bench/mail-sync.py with a large post-offline catch-up and a concurrent interactive cache read.

Duplicate search: all issue titles for Mail/sync, #626 (UID identity), #613 (sync state), #668 (app stream). Those do not own per-run provider delta budgeting. No measured regression or merge-blocking defect is claimed.

Context: #663 rules 8–9; Mail IMAP sync. Source evidence at c4a61e8cf and job/merge-round-7a 2f4482ded. No runtime latency claim. Evidence at round-7a crates/plugins/mail/src/sync.rs: - :50 UID_WINDOW is 80; :51 MAX_BACKFILL_WINDOWS_PER_RUN is 100. - :549–582 history backfill consumes windows_left. - :597 new-UID loop drains from generation.delta_cursor to latest_uid without checking or consuming windows_left. Each window commits its cursor atomically, but there is no per-run page or elapsed-time limit. - :661 reconcile_expunged_uids collects and sorts SEARCH ALL into one complete live UID set. round-7a crates/plugins/mail/src/cache/store.rs:852 prune_absent_uids loads that set in 500-row chunks into a temp table and prunes memberships in one writer transaction (:863). The chunks do not release the writer connection. This is whole-folder work rather than a restartable bounded job. Reasoned impact: a large catch-up occupies an ingest slot and one account guard for all its windows. 80,000 new UIDs require 1,000 windows, ten times the advertised history budget. Sparse UID space costs windows even when few messages remain. SQL commits yield to the runtime, but they do not release the job slot to other accounts. Module docs say each run processes a fixed number of windows; this does not hold for delta or expunge reconciliation. Concrete fix: one shared per-job work budget for history, delta and expunge scans. Persist restartable progress and queue a continuation with the existing dedup/slot model. Do not publish a final folder reconciliation before a complete scan. Prefer provider delta capabilities when supported; keep a bounded fallback. Preserve page+cursor atomicity and UIDVALIDITY checks. Tests: feed more than 100 delta windows and assert the first job stops at its budget; continuations converge with no skipped or duplicate UID records. Test sparse high UIDs, expunges concurrent with new arrivals, provider failure and restart. Demonstrate another account progresses between batches. Extend bench/mail-sync.py with a large post-offline catch-up and a concurrent interactive cache read. Duplicate search: all issue titles for Mail/sync, #626 (UID identity), #613 (sync state), #668 (app stream). Those do not own per-run provider delta budgeting. No measured regression or merge-blocking defect is claimed.

kayg referenced this issue

2026-10-02 13:16:50 +00:00

PERF: instant everywhere — app-wide speed architecture rules, audit and shared primitives #663

kayg referenced this issue

2026-10-02 13:19:08 +00:00

PERF: make Mail folder page SQL work proportional to its window (#663) #825

kayg referenced this issue

2026-10-02 13:19:27 +00:00

PERF: instant everywhere — app-wide speed architecture rules, audit and shared primitives #663

kayg commented

2026-10-02 13:24:12 +00:00

Author

Owner

SQLite audit #663 on job/perf-arch-db. Base c4a61e8cf0; queued merge-round-7a 2f4482ded0 checked separately.

Additional SQLite index evidence for the expunge fix: mail/src/cache/store.rs:819 and :1186 delete orphan mail_messages using account_id alone plus a NOT EXISTS membership probe. mail_messages_account begins (owner_id,account_id,...), so account_id alone has no guaranteed leading-prefix seek; it may scan all accounts or use a statistics-dependent skip scan. Include owner_id in the scoped cleanup predicate or add an appropriate account-leading index, and capture plans with many Users/accounts, before and after statistics. The expunge temp-table loads and deletes all remain in one writer transaction despite 500-row statement chunks. Include unrelated interactive writer progress between resumable reconciliation batches; keep UIDVALIDITY/live-generation correctness.

Local query-work figures use Python SQLite 3.53.3 and synthetic data; VM callbacks count 1,000-instruction units. They are not production latency, CPU/RSS or HDD budget samples. The locked Rust bundled SQLite is 3.51.3. Repeat with audit-sqlite.py --plans and confirm the production-version plan before implementing. No existing assertion or product behavior was changed. Reuse this issue instead of filing a duplicate.

SQLite audit #663 on job/perf-arch-db. Base c4a61e8cf090170f35b1bed3350d9de20c83ecd5; queued merge-round-7a 2f4482ded066d9c5d9c59130377907f7fd2916c9 checked separately. Additional SQLite index evidence for the expunge fix: mail/src/cache/store.rs:819 and :1186 delete orphan mail_messages using account_id alone plus a NOT EXISTS membership probe. mail_messages_account begins (owner_id,account_id,...), so account_id alone has no guaranteed leading-prefix seek; it may scan all accounts or use a statistics-dependent skip scan. Include owner_id in the scoped cleanup predicate or add an appropriate account-leading index, and capture plans with many Users/accounts, before and after statistics. The expunge temp-table loads and deletes all remain in one writer transaction despite 500-row statement chunks. Include unrelated interactive writer progress between resumable reconciliation batches; keep UIDVALIDITY/live-generation correctness. Local query-work figures use Python SQLite 3.53.3 and synthetic data; VM callbacks count 1,000-instruction units. They are not production latency, CPU/RSS or HDD budget samples. The locked Rust bundled SQLite is 3.51.3. Repeat with audit-sqlite.py --plans and confirm the production-version plan before implementing. No existing assertion or product behavior was changed. Reuse this issue instead of filing a duplicate.

kayg commented

2026-10-02 13:27:15 +00:00

Author

Owner

Starting implementation on job/mailperf, based on job/merge-round-7a at 2f4482ded066d9c5d9c59130377907f7fd2916c9. The target areas are IDLE scheduling, bounded delta/expunge work, and unread-count invalidation. I will merge origin/dev and job/merge-round-7a before the final gates.

Starting implementation on `job/mailperf`, based on `job/merge-round-7a` at `2f4482ded066d9c5d9c59130377907f7fd2916c9`. The target areas are IDLE scheduling, bounded delta/expunge work, and unread-count invalidation. I will merge `origin/dev` and `job/merge-round-7a` before the final gates.

kayg referenced this issue

2026-10-02 13:39:12 +00:00

PERF: instant everywhere — app-wide speed architecture rules, audit and shared primitives #663

kayg commented

2026-10-02 17:43:21 +00:00

Author

Owner

The review of bounded expunge staging exposed an unsafe equal-count case: searching only cached candidate UIDs and comparing the result with EXISTS cannot detect one provider UID missing from cache when a different cached UID is stale. I changed reconciliation to query bounded numeric UID ranges, stage the complete provider UID set, compare the full staged count with a fresh mailbox snapshot, then prune cached UIDs absent from that set. The reconnect regression uses 160 sparse cached UIDs, removes one provider UID, and checks that the cursor resumes between ranges and pruning waits for validation. Targeted test passed: test sync::tests::expunge_reconciliation_is_bounded_and_resumes_for_sparse_uids ... ok.

The review of bounded expunge staging exposed an unsafe equal-count case: searching only cached candidate UIDs and comparing the result with `EXISTS` cannot detect one provider UID missing from cache when a different cached UID is stale. I changed reconciliation to query bounded numeric UID ranges, stage the complete provider UID set, compare the full staged count with a fresh mailbox snapshot, then prune cached UIDs absent from that set. The reconnect regression uses 160 sparse cached UIDs, removes one provider UID, and checks that the cursor resumes between ranges and pruning waits for validation. Targeted test passed: `test sync::tests::expunge_reconciliation_is_bounded_and_resumes_for_sparse_uids ... ok`.

kayg commented

2026-10-02 18:07:21 +00:00

Author

Owner

Implemented bounded delta catch-up and durable expunge reconciliation in de814554f. The sweep now scans numeric UID ranges, validates the complete provider UID set against a fresh mailbox snapshot, then prunes cached UIDs in bounded pages. The 160 sparse-UID reconnect regression and the full Mail crate suite passed before the required merges. Post-merge clippy was stopped at the four-hour cap; no perf VM before/after run was completed. Migration 0010_bounded_expunge_cursor.sql remains next after origin/dev migration 0009.

Implemented bounded delta catch-up and durable expunge reconciliation in `de814554f`. The sweep now scans numeric UID ranges, validates the complete provider UID set against a fresh mailbox snapshot, then prunes cached UIDs in bounded pages. The 160 sparse-UID reconnect regression and the full Mail crate suite passed before the required merges. Post-merge clippy was stopped at the four-hour cap; no perf VM before/after run was completed. Migration `0010_bounded_expunge_cursor.sql` remains next after `origin/dev` migration 0009.

kayg commented

2026-10-02 20:04:12 +00:00

Author

Owner

Independent read-only review: job/mailperf at 2e724c0529ef38623fd86c239d638371be5d9702, against origin/dev at c4faf184df726a9375ae0c13bdfb6018ac2cf57e. No build, test or measurement was run. All-state expunge search identifies #757 as the existing owner.

Two P2 findings remain in the staged expunge sweep:

Full-set work still holds the writer. crates/plugins/mail/src/cache/store.rs:1048–1071 counts all staged UIDs inside the writer transaction. :1078–1085 deletes the full staged set on failed validation; :1245–1267 does the same after success. restart_backfill (:875–885) also deletes it. A one-million-message folder has one million staged live UID rows. The page budget does not bound these operations. This contradicts #757's short bounded transactions. Maintain counts with page commits and use durable bounded cleanup pages. Test the maximum rows per transaction, another User's write between pages, and restart in both success and abort cleanup. The work estimate comes from source; there is no measured latency claim.
Arrivals discard all multi-run scan progress. cache/store.rs:1072–1095 requires latest UID and EXISTS to match scan start. Any change deletes all staged rows and resets the cursor. crates/plugins/mail/src/sync.rs:930–936 then returns incomplete. A large sweep needs several 100-window jobs, each SEARCH range covering 1,024 UID values. If one new high UID arrives during each sweep, no sweep validates, so stale memberships and unread counts remain indefinitely. #757 explicitly requires convergence and concurrent-arrival tests. New arrivals above the fixed scan bound must not discard the sweep. Test more than 102,400 UID values, one old expunge and one arrival per sweep; check that continuations remove the stale member and retain new messages, including after restart.

The page transactions themselves commit scan/prune data with their cursors. No crash-induced lost or duplicate message defect was confirmed from those transactions. Crash recovery needs execution evidence. Group the two findings in #757 because both concern the staged sweep lifecycle. See audit-findings.md and review-mailperf.md on job/rev2-mailperf for the complete review.

Independent read-only review: `job/mailperf` at `2e724c0529ef38623fd86c239d638371be5d9702`, against `origin/dev` at `c4faf184df726a9375ae0c13bdfb6018ac2cf57e`. No build, test or measurement was run. All-state expunge search identifies #757 as the existing owner. Two P2 findings remain in the staged expunge sweep: 1. **Full-set work still holds the writer.** `crates/plugins/mail/src/cache/store.rs:1048–1071` counts all staged UIDs inside the writer transaction. `:1078–1085` deletes the full staged set on failed validation; `:1245–1267` does the same after success. restart_backfill (`:875–885`) also deletes it. A one-million-message folder has one million staged live UID rows. The page budget does not bound these operations. This contradicts #757's short bounded transactions. Maintain counts with page commits and use durable bounded cleanup pages. Test the maximum rows per transaction, another User's write between pages, and restart in both success and abort cleanup. The work estimate comes from source; there is no measured latency claim. 2. **Arrivals discard all multi-run scan progress.** `cache/store.rs:1072–1095` requires latest UID and EXISTS to match scan start. Any change deletes all staged rows and resets the cursor. `crates/plugins/mail/src/sync.rs:930–936` then returns incomplete. A large sweep needs several 100-window jobs, each SEARCH range covering 1,024 UID values. If one new high UID arrives during each sweep, no sweep validates, so stale memberships and unread counts remain indefinitely. #757 explicitly requires convergence and concurrent-arrival tests. New arrivals above the fixed scan bound must not discard the sweep. Test more than 102,400 UID values, one old expunge and one arrival per sweep; check that continuations remove the stale member and retain new messages, including after restart. The page transactions themselves commit scan/prune data with their cursors. No crash-induced lost or duplicate message defect was confirmed from those transactions. Crash recovery needs execution evidence. Group the two findings in #757 because both concern the staged sweep lifecycle. See `audit-findings.md` and `review-mailperf.md` on `job/rev2-mailperf` for the complete review.

kayg referenced this issue

2026-10-02 20:05:31 +00:00

Mail: read-state changes leave sidebar unread counts stale #771

kayg referenced this issue

2026-10-03 04:32:08 +00:00

Mail: read-state changes leave sidebar unread counts stale #771

kayg referenced this issue from a commit

2026-10-03 04:41:25 +00:00

Fix Mail perf review findings F1-F5 (#771, #753, #757)

kayg commented

2026-10-03 07:08:38 +00:00

Author

Owner

The independent review found F4 and F5 at predecessor SHA 2e724c0529ef38623fd86c239d638371be5d9702. Current Mail store page commits maintain expunge_staged_count, validation reads that count in O(1), and cleanup deletes a bounded page while expunge_cleanup blocks a new sweep. expunge_cleanup_is_bounded_resumable_and_blocks_new_sweeps checks multi-page success/failure cleanup and crash rollback. For F5, the sweep keeps a fixed UID top and counts arrivals above it with a bounded SEARCH; expunge_sweep_converges_while_new_mail_keeps_arriving and expunge_validation_keeps_the_scan_when_only_new_uids_arrive cover convergence and cursor retention. Source/test review only so far; requested Mail gates are next.

The independent review found F4 and F5 at predecessor SHA `2e724c0529ef38623fd86c239d638371be5d9702`. Current Mail store page commits maintain `expunge_staged_count`, validation reads that count in O(1), and cleanup deletes a bounded page while `expunge_cleanup` blocks a new sweep. `expunge_cleanup_is_bounded_resumable_and_blocks_new_sweeps` checks multi-page success/failure cleanup and crash rollback. For F5, the sweep keeps a fixed UID top and counts arrivals above it with a bounded SEARCH; `expunge_sweep_converges_while_new_mail_keeps_arriving` and `expunge_validation_keeps_the_scan_when_only_new_uids_arrive` cover convergence and cursor retention. Source/test review only so far; requested Mail gates are next.

Rows
Columns

PERF: bound Mail delta and expunge work per sync job (#663) #757