Skip to content

perf: reduce allocations on hot paths#38

Merged
tac0turtle merged 1 commit intomainfrom
reduce-allocations
Feb 26, 2026
Merged

perf: reduce allocations on hot paths#38
tac0turtle merged 1 commit intomainfrom
reduce-allocations

Conversation

@tac0turtle
Copy link
Contributor

@tac0turtle tac0turtle commented Feb 25, 2026

  • Cache Prometheus WithLabelValues at init (metrics.go): eliminates per-height map lookups and slice allocs for sync state, backfill stages, and store operations.
  • Move transient error needles to package-level var (celestia_node.go): avoids per-retry []string heap allocation.
  • Cache bearer auth metadata map at construction (celestia_app.go): avoids map+string alloc per gRPC call.
  • Replace map[string]any with struct in MarshalBlob (service.go): ~200 bytes less per blob marshal.
  • Preallocate blob slices in GetBlobs, BlobGetAll, gRPC GetAll: reduces append doublings on typical result sets.
  • Optimize notifier filterEvent: skip allocation when all/no blobs match; use exact-capacity slice for partial matches.
  • Skip namespace map allocation for header-only subscriptions.
  • Use slice indexing in httpToWS instead of TrimPrefix.

Overview

Summary by CodeRabbit

Release Notes

  • Refactor
    • Enhanced memory efficiency across blob retrieval and storage operations through optimized allocation patterns.
    • Improved metrics collection performance with optimized label caching and pre-resolution.
    • Optimized authentication credential handling with cached metadata.
    • Refined error detection patterns.

- Cache Prometheus WithLabelValues at init (metrics.go): eliminates
  per-height map lookups and slice allocs for sync state, backfill
  stages, and store operations.
- Move transient error needles to package-level var (celestia_node.go):
  avoids per-retry []string heap allocation.
- Cache bearer auth metadata map at construction (celestia_app.go):
  avoids map+string alloc per gRPC call.
- Replace map[string]any with struct in MarshalBlob (service.go):
  ~200 bytes less per blob marshal.
- Preallocate blob slices in GetBlobs, BlobGetAll, gRPC GetAll:
  reduces append doublings on typical result sets.
- Optimize notifier filterEvent: skip allocation when all/no blobs
  match; use exact-capacity slice for partial matches.
- Skip namespace map allocation for header-only subscriptions.
- Use slice indexing in httpToWS instead of TrimPrefix.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link

coderabbitai bot commented Feb 25, 2026

📝 Walkthrough

Walkthrough

This PR optimizes memory allocation and caching across multiple layers: preallocation of slices reduces reallocations in the API and storage layers, metadata for bearer tokens is cached in gRPC credentials, transient error substrings are moved to package-level storage, metrics pre-resolves known labels to avoid per-call lookups, and JSON marshaling uses a struct instead of dynamic maps.

Changes

Cohort / File(s) Summary
API Layer Optimizations
pkg/api/grpc/blob_service.go, pkg/api/notifier.go, pkg/api/service.go
Preallocation of blob slices with capacity calculations, optimized Subscribe path to defer nil map initialization, filterEvent avoids allocations when all/no blobs match or only partially match, and blob marshaling now uses struct-based JSON representation instead of map allocations.
Fetch/Network Optimizations
pkg/fetch/celestia_app.go, pkg/fetch/celestia_node.go
Bearer token metadata cached in gRPC credentials via newBearerCreds constructor, string prefix trimming replaced with index slicing for WebSocket conversion, and transient RPC error substrings moved to package-level transientNeedles slice.
Metrics Pre-resolution
pkg/metrics/metrics.go
Major refactoring introducing static sync state labels, pre-resolved gauge arrays, and hybrid metric access patterns with fallback to Vec-based lookups. PromRecorder struct expanded with new Vec metrics and maps of pre-resolved observers/counters for known label values. Per-label pre-resolution occurs in NewPromRecorder initialization.
Storage Optimization
pkg/store/sqlite.go
GetBlobs slice preallocated with capacity 64 to reduce reallocations during blob accumulation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

Poem

🐰 Hop, hop, the allocations go down,
Pre-sizing slices all around the town,
Cached metadata hops without a care,
Metrics resolve what once floated in air,
Memory blooms when we don't allocate twice! 🌱

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title 'perf: reduce allocations on hot paths' clearly and accurately summarizes the primary objective of the changeset, which involves multiple optimizations across several files to reduce memory allocations on frequently-called code paths.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch reduce-allocations

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@tac0turtle tac0turtle marked this pull request as ready for review February 26, 2026 07:42
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
pkg/api/service.go (1)

73-73: Cap the preallocation hint defensively.

Line 73 scales capacity directly with len(namespaces). A small cap keeps the optimization but avoids large upfront allocations on pathological inputs.

♻️ Suggested tweak
-	allBlobs := make([]types.Blob, 0, len(namespaces)*8) // preallocate for typical workload
+	const maxNamespacePrealloc = 64
+	nsCap := len(namespaces)
+	if nsCap > maxNamespacePrealloc {
+		nsCap = maxNamespacePrealloc
+	}
+	allBlobs := make([]types.Blob, 0, nsCap*8) // preallocate for typical workload
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/api/service.go` at line 73, The preallocation for allBlobs (created as
allBlobs := make([]types.Blob, 0, len(namespaces)*8)) should be defensively
capped to avoid huge allocations for pathological namespace counts; compute a
capacity value from len(namespaces)*8 then clamp it to a reasonable max (e.g.,
1024 or another project-appropriate constant) before passing it into make so
allBlobs uses the smaller clamped capacity; update the allocation site where
allBlobs is created to use the clamped cap.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@pkg/api/service.go`:
- Line 73: The preallocation for allBlobs (created as allBlobs :=
make([]types.Blob, 0, len(namespaces)*8)) should be defensively capped to avoid
huge allocations for pathological namespace counts; compute a capacity value
from len(namespaces)*8 then clamp it to a reasonable max (e.g., 1024 or another
project-appropriate constant) before passing it into make so allBlobs uses the
smaller clamped capacity; update the allocation site where allBlobs is created
to use the clamped cap.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c8b5ebc and d9516bc.

📒 Files selected for processing (7)
  • pkg/api/grpc/blob_service.go
  • pkg/api/notifier.go
  • pkg/api/service.go
  • pkg/fetch/celestia_app.go
  • pkg/fetch/celestia_node.go
  • pkg/metrics/metrics.go
  • pkg/store/sqlite.go

@tac0turtle tac0turtle merged commit a51d28b into main Feb 26, 2026
4 checks passed
@tac0turtle tac0turtle deleted the reduce-allocations branch February 26, 2026 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant