perf: reduce cache and channel pre-allocations - #590
Merged
Conversation
Trim over-provisioned default caches and the noise_socket send-job channel. All values remain tunable via CacheConfig. - Signal cache (wacore): 10K -> 2K entries - lid_pn_cache: 10K -> 2K - device_registry_cache: 5K -> 1K - message_retry_counts: 1K -> 500 - pdo_pending_requests: 500 -> 200 - noise_socket send_job channel: 32 -> 8 (backpressure > buffering) - transport event channel: 1024 -> 64 - remove unused new_pdo_cache() helper Validated with 5K msg/s benchmark (single DM). Coordination caches (session_locks, chat_lanes) intentionally kept at current defaults; eviction-while-held safety isn't covered by the bench shape.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR reduces default capacities for several caches and bounded channels across the codebase and removes a now-unused PDO cache constructor. Documentation comments were updated to reflect the new capacity bounds. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Collaborator
Author
|
@claude review |
|
Claude finished @jlucaso1's task in 1m 40s —— View job Review of PR #590
OverallClean, low-risk PR. The cache reductions are safe — all affected caches are lossy (fall back to DB on miss) or TTL-bounded, and every value remains tunable via
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/cache_config.rs`:
- Line 249: The pdo_pending_requests CacheEntryConfig currently uses
CacheEntryConfig::new(Some(Duration::from_secs(30)), 200) which risks silent
duplicate PDOs when entries are evicted while requests are in-flight; add
metrics instrumentation around pdo_pending_requests to emit (1) eviction
counters, (2) hit/miss counters and rates for dedup lookups, and (3) current
size/gauge so we can monitor pressure in production, and optionally increase the
capacity or switch to per-message in-flight tracking if eviction rates are
non-trivial; locate and update the cache initialization using
pdo_pending_requests and CacheEntryConfig::new to add the metrics hooks and
adjust the capacity or tracking strategy accordingly.
In `@src/socket/noise_socket.rs`:
- Around line 45-47: The comment and fixed buffer size for the channel (creation
of send_job_tx/send_job_rx with capacity 8 for SendJob) mistakenly claims jobs
are processed in "<1ms" whereas the sender actually awaits network I/O via
transport.send(frame).await; remove or rephrase the "<1ms" claim to avoid
inaccurate latency assumptions and make the channel capacity tunable (e.g.,
replace hardcoded 8 with a named constant or configuration parameter referenced
where the channel is created) so operators can adjust for real-world latency
spikes; update related comment near transport.send(frame).await to reflect it's
network-bound rather than CPU-bound.
In `@transports/tokio-transport/src/lib.rs`:
- Line 18: The constant EVENT_CHANNEL_CAPACITY is hardcoded to 64 which forces
read-side backpressure to kick in at a fixed threshold; change it to be
configurable at runtime (keep default 64) by reading a config/env value and
using that value wherever EVENT_CHANNEL_CAPACITY is consumed (e.g., channel
creation and any backpressure logic that references EVENT_CHANNEL_CAPACITY) so
deployments can override the queue size without code changes; ensure you
validate/parsethe runtime value (fallback to 64 on parse/absent) and update any
tests or constructors that assume the constant.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7495b631-7de9-408d-801d-47e503d9474f
📒 Files selected for processing (5)
src/cache_config.rssrc/pdo.rssrc/socket/noise_socket.rstransports/tokio-transport/src/lib.rswacore/src/store/signal_cache.rs
💤 Files with no reviewable changes (1)
- src/pdo.rs
![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: c8de8f2555
ℹ️ 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".
Benchmark Results67 unchanged benchmark(s)
|
This was referenced Jun 5, 2026
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
1 participant
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.
Summary
Trim over-provisioned default caches and the noise_socket send-job channel. All values remain tunable via
CacheConfig.Changes
lid_pn_cachedevice_registry_cachemessage_retry_countspdo_pending_requestsnew_pdo_cache()helperLossy caches fall back to DB on miss; time-bounded caches (TTL 30s-5m) tolerate pressure; channel overflow is backpressure, not corruption.
Deliberately out of scope
session_locks_capacity/chat_lanes_capacity— these hold live mutexes; eviction while referenced breaks serialization. Single-DM benchmark doesn't exercise distinct-chat fan-out, so the safety of reducing them is unvalidated. Left at current defaults.enc_bufinitial capacity — kept at 4096. The per-frame cost of the alternative pattern is indistinguishable from noise at 5K msg/s.Test plan
cargo fmt --allcargo clippy --all --testscargo test --workspace --exclude e2e-tests --exclude bench-integration