⚡ Bolt: [performance improvement] Reduce eventqueue GC pressure#24
Conversation
The `PendingEventQueue` in `internal/core/stream` translates large wire chunks into many canonical events. Its compaction logic was reallocating an entire new slice with every shift if the queue capacity was over 32. Since these queues process rapidly arriving stream events, shifting head-to-tail down the existing array via `copy` is much cheaper than `make`+`copy` except when the array has bloated excessively (>1024 cap and <25% active). This PR: 💡 What: Replaced the eager `make` reallocation in `PendingEventQueue.compactIfNeeded` with in-place `copy` slice shifting, only reallocating when the buffer is severely bloated. 🎯 Why: To reduce GC pressure and allocation overhead on the hot stream queue path. 📊 Impact: pushPop benchmark throughput improved ~35% (175k ns -> 126k ns) and allocations dropped from 13 to 9 per run. 🔬 Measurement: Run `cd internal/core/stream && go test -bench=BenchmarkPendingEventQueue_pushPop -benchmem` Co-authored-by: matdev83 <211248003+matdev83@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Warning Review limit reached
More reviews will be available in 16 minutes and 5 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesPendingEventQueue Compaction Optimization
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes 🚥 Pre-merge checks | ✅ 8✅ Passed checks (8 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/core/stream/eventqueue.go`:
- Around line 96-97: The copy and reslice operation in the eventqueue compaction
logic leaves old entries in the truncated portion of the backing array, which
prevents garbage collection of popped events. After the reslice statement `q.buf
= q.buf[:alive]`, explicitly zero out the tail of the buffer from index alive to
the original length to clear the old event references and allow them to be
garbage collected.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: dfadfb1b-77b6-447e-a979-8d5925cd98f4
📒 Files selected for processing (2)
.jules/bolt.mdinternal/core/stream/eventqueue.go
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: qa
🧰 Additional context used
📓 Path-based instructions (3)
internal/core/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
internal/core/**/*.go: Core packages must not import official provider SDKs
Core packages must not import concrete plugins
Return errors, do not panic in request paths
Keep core files small and cohesive
Do not mix frontend codec logic, routing policy, and backend invocation in one package
Prefer explicit construction and registration over DI containers, reflection, or global registries
No package-level mutable global state in core code
Capability mismatches must fail explicitly; never silently drop required semantics
Advanced request/response mutation belongs behind hook interfaces, not inside core business logic
Narrow interfaces, small files, simple control flow (keep the core boring)
Files:
internal/core/stream/eventqueue.go
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Use small interfaces defined where they are consumed
Every I/O boundary takescontext.Context
Do not use Java-style interface prefixes; use idiomatic Go names such asStore,Router,Clock
Avoidanyunless unavoidable at a protocol boundary
Wrap errors with%wand preserve classification metadata
Avoid circular imports by design
Prefer the standard library unless a dependency clearly reduces complexity
Use small interfaces defined where they are consumed
Slices in JSON and returned values should use explicit empty initialization (s := []T{}ormake) to preventnullin JSON
Establish explicit ownership for goroutines, channels, buffers, and cancellation
Do not use Go's nativepluginpackage in v1
Add package docs where the boundary is non-obvious
Line length ~120+ characters; break at semantic boundaries. Single long lines preferred for embedded JSON/SSE to preserve stream integrityFor server, CLI, worker, or network code, check that context.Context is propagated correctly, cancellation is respected, and new goroutines cannot leak indefinitely
Files:
internal/core/stream/eventqueue.go
⚙️ CodeRabbit configuration file
**/*.go: Review as production Go code. Prioritize correctness, race conditions, goroutine leaks, context cancellation, timeout handling, error wrapping, nil-pointer risks, resource cleanup, defer placement, API compatibility, interface design, dependency boundaries, and testability. Avoid generic style comments when gofmt/golangci-lint already covers the issue.
Files:
internal/core/stream/eventqueue.go
internal/**
⚙️ CodeRabbit configuration file
internal/**: Focus on package boundaries, hidden coupling, unexported API design, concurrency safety, deterministic behavior, and whether logic belongs in this internal package.
Files:
internal/core/stream/eventqueue.go
🪛 markdownlint-cli2 (0.22.1)
.jules/bolt.md
[warning] 1-1: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🔇 Additional comments (1)
.jules/bolt.md (1)
1-3: LGTM!
The
PendingEventQueueininternal/core/streamtranslates large wire chunks into many canonical events. Its compaction logic was reallocating an entire new slice with every shift if the queue capacity was over 32. Since these queues process rapidly arriving stream events, shifting head-to-tail down the existing array viacopyis much cheaper thanmake+copyexcept when the array has bloated excessively (>1024 cap and <25% active).This PR:
💡 What: Replaced the eager
makereallocation inPendingEventQueue.compactIfNeededwith in-placecopyslice shifting, only reallocating when the buffer is severely bloated.🎯 Why: To reduce GC pressure and allocation overhead on the hot stream queue path.
📊 Impact: pushPop benchmark throughput improved ~35% (175k ns -> 126k ns) and allocations dropped from 13 to 9 per run.
🔬 Measurement: Run
cd internal/core/stream && go test -bench=BenchmarkPendingEventQueue_pushPop -benchmemPR created automatically by Jules for task 8043453630250216761 started by @matdev83