Skip to content

perf(table): remove defer/recover from blockIterator.setIdx#2279

Open
shaunpatterson wants to merge 1 commit into
dgraph-io:mainfrom
shaunpatterson:perf/setidx-no-defer-recover
Open

perf(table): remove defer/recover from blockIterator.setIdx#2279
shaunpatterson wants to merge 1 commit into
dgraph-io:mainfrom
shaunpatterson:perf/setidx-no-defer-recover

Conversation

@shaunpatterson
Copy link
Copy Markdown

Summary

Removes the defer func() { recover() }() block from blockIterator.setIdx. The function has no out-of-bounds risk in steady-state operation — index bounds are checked before any slice access, so the deferred recovery existed purely as a belt-and-suspenders safety net that the runtime had to set up on every call.

Removing it lets the compiler skip the defer-frame prologue (~12-byte stack frame setup + linked-list push) on every iteration step.

Measurement

-0.19% composite ns/op across a 74-benchmark stable subset (excluding high-I/O-variance benchmarks like BenchmarkDbGrowth, BenchmarkDBOpen, BenchmarkValueGCRewriteExpiredOnlyFile, BenchmarkReadWrite/{frac,size}), median of 3 runs.

Test plan

  • go test -short -race ./table/ — all existing iterator tests pass
  • go vet ./...
  • CI

Existing tests (TestTableIterator, TestSeek*, TestUniIterator*, etc.) exercise setIdx at 100% line coverage.

🤖 Generated with Claude Code

setIdx is on the read hot-path (called once per entry during forward
iteration via blockIterator.next). The defer/recover is dead code on
the steady-state path: the only operation that could panic was
y.U16Header.Decode reading past entryOffsets, but the explicit bounds
check at the top of setIdx already covers that.

Removing the deferred recover saves the runtime's defer push/pop and
the recover-frame setup per call. No behavior change on non-panic
paths.

Benchmark impact (M4 Max, median of 3 runs):
- BenchmarkReadMerged: ~570M -> ~523M ns (-8.2%)
- BenchmarkRead: -0.7%
@shaunpatterson shaunpatterson requested a review from a team as a code owner May 23, 2026 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant