Repository navigation
perf(acp): avoid repeated observer batch serialization - #9413
Closed
sanket1729 wants to merge 1 commit into
Closed
sanket1729 wants to merge 1 commit into
sanket1729 wants to merge 1 commit into
Conversation
Reuse cached event sizes when packing observer frames. Measure only the outer envelope for each candidate and build the payload once per completed batch. Co-authored-by: Codex <[email protected]> Signed-off-by: Sanket Kanjalkar <[email protected]>
🔐 Codex Security Review
|
This was referenced Nov 12, 2026
sanket1729
added a commit
that referenced
this pull request
Nov 13, 2026
Replaces #9424 with the same release-profile change on a branch in this repository so the review workflows can run. ## Summary Reduce release codegen units from 16 to 4 and enable thin link-time optimization (LTO). Local measurements show about **7% smaller binaries**, with a **14% increase in dependency-cached release rebuild time** (+3.1 seconds). Clean release build time was roughly flat. `codegen-units = 4` seems like a good balance. With thin LTO fixed, one unit cut stripped sizes a further 7–10%, but increased clean build time by 38% and cached release rebuild time by 67.5%. CPU and Activity trials showed no clear speedup. Fewer codegen units let LLVM optimize larger portions of each crate together. Lower counts typically favor runtime performance and smaller binaries over compilation speed. Thin LTO also enables optimization across crate boundaries. Runtime gains depend on the workload. Release keeps `opt-level = 3`, the highest numeric optimization level and Cargo's existing default. Development builds stay unchanged. Keep the custom `ci` profile at 16 codegen units with LTO off, preserving its existing settings. Sprig keeps its separate size-focused profile. CI jobs that use ordinary `--release` will use the new settings. These measurements cover `buzz` and `buzz-acp`, not the desktop bundle. The desktop app has its own Cargo workspace; its bundled CLI picks up this change after a pin update. Our end-to-end runtime trials were broadly flat; the measurements below demonstrate the size and build-time tradeoff. ### Related issue N/A — no duplicate release LTO/codegen PR found. This PR contains only the profile change and is independent of #9413. ### Testing #### Baseline versus four codegen units Compared identical source and lockfile at `b4de4479485fc3d57628ed509ed40c4278e63132`, using Rust 1.98.1 on an Apple M5 Max (18 cores, 128 GiB RAM). Both arms used `opt-level=3`, disabled incremental compilation and no compiler-cache wrapper. The baseline used LTO off and 16 codegen units; the candidate used thin LTO and four. | Median / artifact | Baseline | Thin LTO, 4 units | Change | | --- | ---: | ---: | ---: | | Clean release build (2 runs each) | 47.98 s | 46.71 s | -2.7% | | Dependency-cached release rebuild (3 each) | 22.55 s | 25.68 s | +13.9% | | No-op release build (5 each) | 0.469 s | 0.461 s | Roughly unchanged | | `buzz` executable | 14.516 MiB | 13.580 MiB | -6.4% | | `buzz-acp` executable | 14.580 MiB | 13.487 MiB | -7.5% | Build command: `cargo build --offline --locked --release -p buzz-acp -p buzz-cli --timings`. Set `CARGO_INCREMENTAL=0`, `CARGO_PROFILE_RELEASE_LTO=false|thin` and `CARGO_PROFILE_RELEASE_CODEGEN_UNITS=16|4` for each arm, with separate `CARGO_TARGET_DIR` directories. Clean runs used empty build directories in sequential ABBA order, with downloaded dependencies and filesystem caches warm. Rebuilds cleaned only `buzz-acp` and `buzz-cli` outside the timed region. Sizes are Cargo-built executables before the desktop packager's stripping step. Stripping copies with `xcrun strip -S -x` kept the size benefit: `buzz` fell from 12.550 to 11.805 MiB (-5.9%), and `buzz-acp` from 12.360 to 11.501 MiB (-7.0%). This does not measure a signed desktop installer. Both release executables passed `--help` smoke checks. Profile inspection confirmed the custom `ci` and `sprig` settings are unchanged. #### One codegen unit versus four A separate follow-up compared four versus one on `a314975ce7bf9aa00baebfef874b6e69474a95cf`, which includes the merged observer CPU/locking changes (#9381). Both arms used thin LTO, `opt-level=3`, the same Rust 1.98.1 toolchain and hardware, and identical source and lockfile. This is a separate source snapshot from the first comparison; the tables should not be combined into a three-arm comparison. | Median / artifact | Thin LTO, 4 units | Thin LTO, 1 unit | Change | | --- | ---: | ---: | ---: | | Clean release build (2 runs each) | 45.93 s | 63.35 s | +37.9% | | Dependency-cached release rebuild (3 each) | 22.38 s | 37.48 s | +67.5% | | Stripped `buzz` | 11.789 MiB | 10.923 MiB | -7.3% | | Stripped `buzz-acp` | 11.485 MiB | 10.320 MiB | -10.1% | | Activity p95 latency (median of 5 runs each) | 952 ms | 961 ms | No clear change | Both sets of binaries were stripped identically with `xcrun strip -S -x`. Repeat the build command above with `CARGO_PROFILE_RELEASE_LTO=thin` and `CARGO_PROFILE_RELEASE_CODEGEN_UNITS=4` or `1`, retaining the same cache and ordering controls. All 60 CPU trials and 10 Activity trials passed. The CPU cases exercised production packing and queue ingestion functions; their differences were within run variation. Activity exercised real ACP subprocesses, observer publication, signing/encryption, the browser broker and production UI against a modeled relay. All 500 markers rendered exactly once, and all 2,250 heavy events were delivered. Its one-second publication interval limits visible latency; these local results are not production percentiles. Generated with AI assistance --------- Signed-off-by: Sanket Kanjalkar <[email protected]> Co-authored-by: Codex <[email protected]>
sanket1729
added a commit
that referenced
this pull request
Nov 14, 2026
Replaces #9413 with the packing optimization refreshed against main on a branch in this repository. ## Summary Busy agent activity streams repeatedly build and serialize a growing JSON batch just to check whether the next event fits. Reuse cached event sizes, measure the newest outer envelope, and build the batch payload once per completed frame. Preserve the wire format, greedy packing, per-channel order, channel barriers, byte budget, and one-frame-per-second relay cadence. Exact-limit coverage checks escaping, UTF-8, optional metadata, and array separators. Move the queue tests into a separate module to satisfy the file-size rule; their test names stay unchanged. ### Related issue Related to #7987. This builds on the merged fitting/locking work in #9381 and retention limits in #9436. Closest earlier batching work: #4917. No duplicate packing fix found among open PRs. ### Testing The ignored probe fills the production queue before timing, drains every event, and checks complete FIFO delivery, frame byte limits, and empty byte accounting. ```sh CARGO_INCREMENTAL=0 CARGO_PROFILE_RELEASE_LTO=thin CARGO_PROFILE_RELEASE_CODEGEN_UNITS=4 \ cargo test --release -p buzz-acp --lib observer::packing_tests::publish_queue_drain_probe -- \ --exact --ignored --nocapture BUZZ_OBSERVER_PROBE_EVENTS=100 cargo test --release -p buzz-acp --lib \ observer::packing_tests::publish_queue_drain_probe -- --exact --ignored --nocapture ``` Five alternating fresh release processes per arm and workload compared baseline `59ae028a3` with candidate `237b928b9` on macOS arm64, Rust 1.98.1. Both arms used `opt-level=3`, thin LTO and four codegen units, with incremental compilation and compiler-cache wrappers off. Each executable was rebuilt separately and its probe identity verified. Medians: | Events | Before | After | Frames in both arms | | --- | ---: | ---: | ---: | | 100 | 6.760 ms | 0.108 ms | 1 | | 10,000 | 1,597.509 ms | 7.844 ms | 30 | The one-event control measured 2.04 → 2.21 microseconds. The timed region covers packing only; ingestion, validation, encryption, publish pacing, relay delivery, and rendering are excluded. These local results do not establish an overall app latency improvement. No timing threshold was added to CI. Native and deployed-relay latency, independent review, and human verification remain pending. Keep this PR in draft. Generated with AI assistance Signed-off-by: Sanket Kanjalkar <[email protected]> Co-authored-by: Codex <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Superseded by #9483, which refreshes this optimization against main on a branch in this repository. Updated measurements are in the replacement PR.
Summary
Busy agent activity streams repeatedly build and serialize a growing JSON batch just to check whether the next event fits. Reuse cached event sizes, measure only the newest outer envelope, and build the batch payload once per completed frame.
Preserve the wire format, greedy packing, per-channel order, channel barriers, byte budget, and one-frame-per-second relay cadence. Exact-limit coverage checks escaping, UTF-8, optional metadata, and array separators.
Related issue
Related to #7987 and #9381. This PR targets
mainindependently of #9381; it changes frame packing rather than payload fitting or observer replay ownership. Closest earlier batching work: #4917.Testing
The new ignored probe fills the production queue before timing, drains every event, and checks complete FIFO delivery, frame byte limits, and empty byte accounting.
Five alternating fresh release processes per arm and workload compared
cb69ad9e4with this commit on macOS arm64. Other repository checks ran on the host. Medians:The single-event control stayed about 2 microseconds. The timed region covers packing only; ingestion, validation, encryption, publish pacing, relay delivery, and rendering are excluded. These results do not establish an overall app latency improvement. No timing threshold was added to CI.
Native and deployed-relay latency, independent review, and human verification remain pending. Keep this PR in draft.
Generated with AI assistance