Skip to content

perf(block): precompute OTEL for chunker hot paths - #2243

Merged
levb merged 2 commits into
mainfrom
lev-telemetry-precompute2
Mar 27, 2026
Merged

levb merged 2 commits into
mainfrom
lev-telemetry-precompute2

Conversation

@levb

@levb levb commented Mar 27, 2026

Copy link
Copy Markdown
Contributor

Replace per-call attribute.String(...) allocation in every Slice/ReadAt/runFetch with precomputed metric.MeasurementOption values built once at package init.

  • Stopwatch.end: build one attribute.NewSet and reuse across all three instrument calls (histogram, sum, count) instead of three separate metric.WithAttributes(kv...) allocations.

  • New PrecomputeAttrs(kv...): builds a reusable MeasurementOption.

  • New Stopwatch.Record(ctx, total, precomputedAttrs): zero per-call attribute allocation alternative to Success/Failure.

  • Exported Success/Failure attribute vars for use with PrecomputeAttrs.

  • Add precomputedAttrs struct with all Slice/fetch attribute combos.

  • Package-level chunkerAttrs var built once at init.

  • All timer.Success(ctx, n, attribute.String(...)) calls replaced with timer.Record(ctx, n, a.successFromCache) etc.

FullFetchChunker.Slice on cache hit (64 MiB cache, 4K blocks, 4 MiB chunks):

                         │    baseline     │         precomputed attrs          │
                         │     sec/op      │   sec/op     vs base              │
ChunkerSlice_CacheHit-16     20.57µ ± 2%   19.85µ ± 3%  -3.49% (p=0.009 n=6)

                         │    baseline     │         precomputed attrs              │
                         │      B/op       │     B/op      vs base                  │
ChunkerSlice_CacheHit-16    9.211Ki ± 0%   8.047Ki ± 0%  -12.64% (p=0.002 n=6)

                         │    baseline     │        precomputed attrs             │
                         │   allocs/op     │ allocs/op   vs base                  │
ChunkerSlice_CacheHit-16     16.000 ± 0%    4.000 ± 0%  -75.00% (p=0.002 n=6)

75% fewer allocations (16 → 4) and 13% less memory per Slice call on the NBD page fault hot path.

Lev Brouk and others added 2 commits March 27, 2026 06:23
Replace per-call `attribute.String(...)` allocation in every
`Slice`/`ReadAt`/`runFetch` with precomputed `metric.MeasurementOption`
values built once at package init.

- `Stopwatch.end`: build one `attribute.NewSet` and reuse across all three
  instrument calls (histogram, sum, count) instead of three separate
  `metric.WithAttributes(kv...)` allocations.
- New `PrecomputeAttrs(kv...)`: builds a reusable `MeasurementOption`.
- New `Stopwatch.Record(ctx, total, precomputedAttrs)`: zero per-call
  attribute allocation alternative to `Success`/`Failure`.
- Exported `Success`/`Failure` attribute vars for use with `PrecomputeAttrs`.

- Add `precomputedAttrs` struct with all Slice/fetch attribute combos.
- Package-level `chunkerAttrs` var built once at init.
- All `timer.Success(ctx, n, attribute.String(...))` calls replaced with
  `timer.Record(ctx, n, a.successFromCache)` etc.

FullFetchChunker.Slice on cache hit (64 MiB cache, 4K blocks, 4 MiB chunks):

```
                         │    baseline     │         precomputed attrs          │
                         │     sec/op      │   sec/op     vs base              │
ChunkerSlice_CacheHit-16     20.57µ ± 2%   19.85µ ± 3%  -3.49% (p=0.009 n=6)

                         │    baseline     │         precomputed attrs              │
                         │      B/op       │     B/op      vs base                  │
ChunkerSlice_CacheHit-16    9.211Ki ± 0%   8.047Ki ± 0%  -12.64% (p=0.002 n=6)

                         │    baseline     │        precomputed attrs             │
                         │   allocs/op     │ allocs/op   vs base                  │
ChunkerSlice_CacheHit-16     16.000 ± 0%    4.000 ± 0%  -75.00% (p=0.002 n=6)
```

75% fewer allocations (16 → 4) and 13% less memory per Slice call on the
NBD page fault hot path.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- renamed `Record` -> `RecordRaw`
- dropped the `a` alias
- refactored `end()` to call `RecordRaw`
@levb levb added the improvement Improvement for current functionality label Mar 27, 2026
@dobrac

dobrac commented Mar 27, 2026

Copy link
Copy Markdown
Contributor

Superseeds: #2236

// RecordRaw records an operation using a precomputed attribute option, it does
// not include any previous attributes passed at Begin(). Zero-allocation
// alternative to Success/Failure for hot paths.
func (t Stopwatch) RecordRaw(ctx context.Context, total int64, precomputedAttrs metric.MeasurementOption) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

RecordRaw silently drops t.kv (the base attributes stored by Begin(kv ...)). All current call sites pass no args to Begin(), so this is fine today. But since RecordRaw is now exported, a future caller could do timer := factory.Begin(sandboxID, ...) and then call RecordRaw, silently losing those base dimensions from all three metric recordings without any compile-time or runtime warning.

Consider either: (a) merging t.kv into the precomputed option inside RecordRaw when t.kv is non-nil, or (b) keeping RecordRaw unexported and only exposing it through the typed Success/Failure methods.

@levb
levb enabled auto-merge (squash) March 27, 2026 13:32
@levb
levb merged commit 701b085 into main Mar 27, 2026
65 of 67 checks passed
@levb
levb deleted the lev-telemetry-precompute2 branch March 27, 2026 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Improvement for current functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants