perf(rpc): reduce allocs in receipt with block info and BlockWith{Receipt/Txs} - #3908
perf(rpc): reduce allocs in receipt with block info and BlockWith{Receipt/Txs}#3908EgeCaner wants to merge 10 commits into
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3908 +/- ##
==========================================
- Coverage 74.76% 74.60% -0.16%
==========================================
Files 461 461
Lines 40915 40910 -5
==========================================
- Hits 30590 30522 -68
- Misses 8232 8285 +53
- Partials 2093 2103 +10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
42b94aa to
01325a8
Compare
|
Claude finished @EgeCaner's task in 7m 1s —— View job Review:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
25480ae to
2aa57e9
Compare
|
Claude finished @EgeCaner's task in 5m 14s —— View job Re-review after push (
|
| Finding | Status |
|---|---|
rpc/v9/block.go sized by header.TransactionCount, indexed by blockTxns → panic on pre_confirmed |
✅ Fixed in 2aa57e9 — now len(blockTxns), matching v10 |
require.NotNil(t, expected.BlockNumber) → require.NotZero |
✅ My mistake, withdrawn — you're right. receipt_accepted_on_l1.json, receipt_accepted_on_l2.json, receipt_deploy.json and receipt_pre_confirmed.json all legitimately carry "block_number": 0, so NotZero would fail. No change needed. |
AdaptCoreTransaction exported in v9 with zero callers |
🟡 Still true, but subsumed by the finding below — applying it gives the function four callers |
AdaptTransaction = *AdaptCoreTransaction(t) doesn't remove the per-tx alloc |
🟡 Unchanged; see below |
New finding
Important
rpc/v9/transaction.go:548— the four single-transaction handlers changed fromreturn AdaptTransaction(txn), nil(1 alloc) toadaptedTxn := AdaptTransaction(txn); return &adaptedTxn(2 allocs + a struct copy), becauseAdaptCoreTransactionalready heap-allocates internally and taking the address of the returned copy allocates again.-gcflags='-m'showsmoved to heap: adaptedTxnat lines 548, 560, 590, 620 on top of&Transaction{...} escapes to heapat 1019/1038. CallingAdaptCoreTransaction(txn)directly at those four sites restores the single alloc and resolves the "exported with no callers" nit at the same time. Small in absolute terms (one extra alloc pergetTransactionByHashcall, not per tx in a block), but a regression in a perf PR with a one-line fix. Fix this →
Nit / follow-up (out of scope, pre-existing)
- The same header-vs-txs mismatch you just fixed still exists in
BlockWithTxHashes:rpc/v9/block.go:351andrpc/v10/block.go:195both domake([]*felt.Felt, header.TransactionCount)and then index byblockTxns. I re-confirmed the two values come from independent pre-confirmed snapshots (blockHeaderByID→PreConfirmedChain()atrpc/v9/helpers.go:119,blockTxnsByNumber→ a secondPreConfirmedChain()at:93), so a tx landing between the two reads panics that handler on a public RPC. Not introduced here and fine to leave for a separate PR — just noting the fix is the identical one-liner.
Everything else
The TXN_RECEIPT / TXN_RECEIPT_WITH_BLOCK_INFO split still looks right and matches the spec, and the receipt-side value returns are clean: AdaptReceipt returning by value feeds directly into the TransactionWithReceipt value field with no intermediate alloc, and AdaptReceiptWithBlockInfo is a single composite-literal alloc. Wire format unchanged (block_hash still omitted for pre-confirmed; block_number was *uint64,omitempty which never omitted pointer-to-zero, so genesis receipts serialize "block_number":0 as before). I also re-checked pre-confirmed snapshot consistency for the paths that read Block.TransactionCount and Block.Transactions from a single chain.Head() (v9/transaction.go:586, v10/block.go:254) — those are safe, since chain_storage.go replaces the slot wholesale rather than mutating in place.
The remaining BlockWithTxs/BlockWithReceipts alloc story is unchanged from my last pass: the value slices remove pointer indirection and one level of chasing during marshalling, but the per-tx Transaction alloc is still there inside AdaptCoreTransaction, so the alloc count for those endpoints matches main. Still think benchstat numbers (precedent: rpc/v10/addtxn_bench_test.go) would be the cleanest way to show which parts of this pay off — no objection to merging without them.
Verification: go vet ./rpc/v9/... ./rpc/v10/... clean (type-checks tests too). Escape analysis via go build -gcflags='-m' on both packages. I could not run go test (not in the allowed tool list), so statements about test behaviour come from reading, not execution.
· branch perf/rpc/receipt-cont
brbrr
left a comment
brbrr
left a comment
There was a problem hiding this comment.
LGTM. Please check my comments and the unresolved comments from the bot.
Also, can you provide a benchmark for this PR?
2aa57e9 to
4e760d1
Compare
|
Claude finished @EgeCaner's task in 5m 35s —— View job Re-review after force-push (
|
| Finding | Status |
|---|---|
AdaptTransaction = *AdaptCoreTransaction(t) doesn't remove the per-tx alloc — value adapter should be the primitive |
✅ Fixed — AdaptCoreTransaction / adaptInvoke / adaptDeclare / adaptDeployAccount now return Transaction by value in both v9 and v10 |
adaptedTxn := AdaptTransaction(txn); return &adaptedTxn was a 2-alloc path (@brbrr: "worth addressing") |
✅ Fixed by the flip above — now exactly one alloc per call, no memcpy of an already-heap struct |
@brbrr: iter.Seq[TransactionReceiptWithBlockInfo] instead of the pointer seq |
✅ Done in v9 + v10, incl. sendTransactionReceipt and TransactionReceiptByHash / getPendingTransactionReceipt returning by value |
@brbrr: stale // block_hash, block_number are optional… comment in rpc/v10/block.go |
✅ Removed |
rpc/v9/block.go sized by header.TransactionCount |
✅ Fixed earlier (len(blockTxns)) |
require.NotZero(t, expected.BlockNumber) |
✅ Withdrawn — fixtures legitimately carry "block_number": 0 |
AdaptCoreTransaction exported in v9 with ~no callers |
🟡 Still open, see nit below |
| @brbrr: benchmark numbers for the PR | 🔴 Not posted yet |
What the escape analysis now shows
The alloc reduction is real this time. go build -gcflags='-m' ./rpc/v9 ./rpc/v10 no longer reports any &Transaction{...} escapes to heap inside the adapters — the composite literals stay on the stack and are written straight into txs[index] / txsWithReceipts[index]. So BlockWithTxs/BlockWithReceipts go from N heap Transactions + 1 pointer slice to 1 value slice, which is the change the PR title claims. The four single-tx handlers still show moved to heap: adaptedTxn (v9 transaction.go:548,560,590,620), but that's now the only allocation on those paths — matching main's single alloc, no extra copy. Receipts are the same story: AdaptReceiptWithBlockInfo returns a value that lands directly in the caller's slot, and the subscription iterator no longer allocates per yielded receipt.
Wire compatibility re-checked on the new shape: neither TransactionReceipt nor TransactionReceiptWithBlockInfo (nor Transaction) declares MarshalJSON/UnmarshalJSON, so moving from pointer to value returns can't silently drop a pointer-receiver marshaller — encoding is identical. jsonrpc.Server only reads tuple[0] when the *jsonrpc.Error is nil (jsonrpc/server.go:553-566), so the return TransactionReceiptWithBlockInfo{}, err paths never serialize the zero struct, and registerMethod places no constraint on the result type. Embedding flattens as expected (block_hash still omitempty, block_number always emitted — same as the old non-nil *uint64). No consumers of the moved fields or the new type exist outside rpc/v9/rpc/v10.
Findings
Blocker / Important: none.
Nit
rpc/v9/transaction.go:1005-1008—AdaptTransactionandAdaptCoreTransactionare now byte-for-byte equivalent exported functions in v9, andAdaptCoreTransaction's only caller is the wrapper. Worth collapsing to one name (v10 keeps both legitimately because ofincludeProofFacts).
Follow-up (pre-existing, untouched here)
rpc/v9/block.go:351andrpc/v10/block.go:195still domake([]*felt.Felt, header.TransactionCount)and index byblockTxnsinBlockWithTxHashes— the header and the txs come from two independentPreConfirmedChain()reads, so a tx landing between them panics that handler. Identical one-liner to the fix already applied toBlockWithTxs; fine as a separate PR.
Housekeeping: 4e760d1 "leftover" is a one-line test change — worth squashing into 1c52093 before merge.
Verification: go vet ./rpc/v9/... ./rpc/v10/... clean (this type-checks the test files too), plus go build -gcflags='-m' on both packages for the escape claims above. I could not run go test or go test -bench (not in the allowed tool list), so statements about test behaviour come from reading, not execution — which also means I can't produce the benchstat numbers @brbrr asked for; that one needs to come from you.
· branch perf/rpc/receipt-cont
|
Claude finished @EgeCaner's task in 7m 18s —— View job Re-review after push (
|
| Finding | Status |
|---|---|
4 single-tx handlers did adaptedTxn := AdaptTransaction(...); return &adaptedTxn (2 allocs) |
✅ Fixed — TransactionByHash / TransactionByBlockIDAndIndex now return Transaction by value in both v9 and v10 |
v9 AdaptTransaction and AdaptCoreTransaction were duplicate exported functions |
✅ Fixed — AdaptCoreTransaction dropped from v9 (v10 correctly keeps both; AdaptTransaction there carries includeProofFacts) |
| @brbrr: benchmark numbers for the PR | 🔴 Still not posted — see below |
header.TransactionCount sizing in BlockWithTxHashes |
🟡 Pre-existing, out of scope (still at rpc/v9/block.go:351, rpc/v10/block.go:195) |
What I verified on the new value-return shape
The switch from *Transaction to Transaction at a JSON-RPC handler boundary has two non-obvious failure modes; both are clear here.
Zero struct never serializes. jsonrpc/server.go:564-566 sets res.Result = tuple[0].Interface() only after the *jsonrpc.Error return is confirmed nil, so the return Transaction{}, rpccore.ErrTxnHashNotFound paths can't leak an empty object where null used to go. registerMethod puts no constraint on the result type.
Addressability of the marshalled value. This is the real hazard: a struct handed to json.Marshal through an any is not addressable, so a pointer-receiver-only MarshalJSON on the type — or on any value field of it — would silently be skipped now, where the old *Transaction return kept everything addressable. Checked all of it:
- Neither
Transaction,TransactionReceipt, norTransactionReceiptWithBlockInfodeclaresMarshalJSON/MarshalTextin either version. - The only pointer-receiver
MarshalJSONreachable from these types is*ResourceBoundsMap(rpc/v9/transaction.go:259,rpc/v10/transaction_types.go:243), and it's referenced asResourceBounds *ResourceBoundsMap— a pointer field, so the marshaller is still reached regardless of addressability. Every other marshaller in the two packages (Sync,ExecuteInvocation,SimulateTransactionsResponse,LimitSlice) has a value receiver and isn't a field of these types.
So wire output is byte-identical. Embedding still flattens as before (block_hash omitempty, block_number always emitted — matching the old always-non-nil *uint64), and no consumer of the moved fields or the new type exists outside rpc/v9/rpc/v10 (only node/node.go imports these packages, and just for Validator()).
Allocation accounting. go build -gcflags='-m' ./rpc/v9 ./rpc/v10 now reports zero &Transaction{...} escapes to heap and zero moved to heap: adaptedTxn across both packages — the composite literals stay on the stack and are written straight into the caller's slot or into txs[index]. Worth noting the alloc that disappeared is the handler's own: reflect.Value.call heap-allocates the return frame for any handler with return values, so the boundary alloc exists in both shapes and merely grows by sizeof(Transaction); Value.Interface() then reuses that frame pointer rather than copying. Net for these four endpoints: one fewer allocation than main, and no memcpy of an already-heap struct.
Test churn is mechanical: assert.Nil → assert.Empty on the error paths is the exact value-type equivalent (testify's isEmpty compares against reflect.Zero), and require.Equal(t, &expected, got) → require.Equal(t, expected, got) preserves the assertion. Nothing weakened.
Remaining asks (unchanged, both yours to close)
- Benchmarks. @brbrr's request is still outstanding, and it's the one thing I can't produce —
go test -benchisn't in my allowed tool list.BlockWithTxs/BlockWithReceiptsare where the story is strongest now (N heapTransactions + pointer slice → one value slice); precedent for the harness isrpc/v10/addtxn_bench_test.go, andbenchstatagainstorigin/mainwould make the title concrete. - Housekeeping.
4e760d1 "leftover"is still a standalone one-line test change — worth squashing into1c52093before merge.
Verification: go vet ./rpc/... clean (all versions, tests type-checked), plus go build -gcflags='-m' on rpc/v9 and rpc/v10 for the escape claims. I could not run go test (not in the allowed tool list), so statements about test behaviour come from reading, not execution.
· branch perf/rpc/receipt-cont
|
Claude finished @EgeCaner's task in 3m 54s —— View job Re-review after push (
|
| allocs | 168-byte copies | |
|---|---|---|
value (0a6aac0) |
1 (interface boxing) | 2 — yield → loop var, loop var → heap box |
pointer (526e78e) |
1 (loop var itself) | 1 — yield → loop var |
go build -gcflags='-m' confirms the alloc just moved rather than doubling: moved to heap: receipt at rpc/v9/subscription_receipts.go:109,127 and rpc/v10/subscription_receipts.go:110,128, with no additional escape on the sendResponse path. Net: same alloc count, one memcpy per yielded receipt removed. iter.Seq[TransactionReceiptWithBlockInfo] is untouched, so @brbrr's request still stands as implemented — only the send helper's parameter flipped.
No aliasing hazard from &receipt. Two things had to hold and both do: the range variable is per-iteration under Go 1.22+ loopvar semantics (each &receipt is a distinct object, so a partially-overwritten receipt can't be observed), and sendResponse marshals synchronously and only hands wsConn.Write the resulting []byte — the pointer isn't retained past the call, so nothing can observe the receipt after the loop moves on.
Wire format still byte-identical. This is the one way a pointer↔value flip can silently change output: a pointer-receiver MarshalJSON is reachable through a pointer but skipped for a non-addressable value. Neither TransactionReceiptWithBlockInfo nor the embedded TransactionReceipt declares MarshalJSON/UnmarshalJSON/MarshalText in either version, and the only pointer-receiver marshaller reachable from the tree (*ResourceBoundsMap) is already held through a pointer field, so it was reached in both shapes. json.Marshal transparently dereferences non-nil pointers, so the emitted object is unchanged.
The test-helper change (for _, receipt → for i := range + &receipts[i]) is assertion-neutral: assertNextMessage marshals the expected value and string-compares against the wire bytes, which produces the same JSON either way.
Full-PR state
Everything from my previous passes still holds and nothing regressed: the TXN_RECEIPT / TXN_RECEIPT_WITH_BLOCK_INFO split matches the spec (block_hash omitempty, block_number always emitted — same as the old always-non-nil *uint64), go build -gcflags='-m' ./rpc/v9 ./rpc/v10 reports zero &Transaction{...} escapes to heap and zero moved to heap: adaptedTxn, the header.TransactionCount → len(blockTxns) fix is in place at rpc/v9/block.go:435, and no consumer of the moved fields exists outside rpc/v9/rpc/v10.
Remaining asks (both yours to close, unchanged)
- Benchmarks. @brbrr's request from Aug 7 is still open.
BlockWithTxs/BlockWithReceiptsare the strongest story (N heapTransactions + pointer slice → one value slice); harness precedent isrpc/v10/addtxn_bench_test.go, andbenchstatvsorigin/mainwould make the title concrete.go test -benchisn't in my allowed tool list, so I can't produce these. - Housekeeping.
4e760d1 "leftover"is still a standalone one-line test change — worth squashing into1c52093before merge.
Follow-up (pre-existing, out of scope): rpc/v9/block.go:351 and rpc/v10/block.go:195 still size by header.TransactionCount and index by blockTxns in BlockWithTxHashes; the two come from independent PreConfirmedChain() reads, so a tx landing between them panics that handler. Identical one-liner to the fix already applied here — fine as a separate PR.
Verification: go vet ./rpc/v9/... ./rpc/v10/... clean (this type-checks the test files too), plus go build -gcflags='-m' on both packages for every escape/alloc claim above. I could not run go test (not in the allowed tool list), so statements about test behaviour come from reading, not execution.
· branch perf/rpc/receipt-cont
…d receipt alloc in BlockWith(Receipt/Txs)
…pt handlers and iterators in v9/v10
…adapt helpers in v9/v10
526e78e to
0d165af
Compare
|
Claude finished @EgeCaner's task in 7m 18s —— View job Re-review after rebase + push (
|
| Finding | Status |
|---|---|
adaptTransactions returning []*Transaction with &adaptedTransaction per element |
✅ Fixed in 0d165af — []Transaction written in place, both v9 and v10 |
4 single-tx handlers did adaptedTxn := ...; return &adaptedTxn (2 allocs) |
✅ Fixed — value returns |
v9 AdaptTransaction / AdaptCoreTransaction duplicate exported functions |
✅ Fixed — AdaptCoreTransaction dropped from v9; v10 keeps both (includeProofFacts) |
header.TransactionCount sizing in BlockWithTxHashes (my long-running follow-up) |
✅ Moot — main refactored both handlers to transactionHashesOf(...) / TransactionHashesByBlockNumber; grep TransactionCount shows zero remaining make(..., header.TransactionCount) sites in either version |
| @brbrr: benchmark numbers | 🔴 Still not posted |
What I verified on the new state
Allocation accounting. go build -gcflags='-m' ./rpc/v9 ./rpc/v10 reports zero &Transaction{...} escapes to heap and zero moved to heap: adaptedTxn across both packages. BlockWithTxs and BlockWithReceipts are now one contiguous make([]Transaction, n) / make([]TransactionWithReceipt, n) with the composite literals written straight into adapted[index] — down from N heap Transactions + 1 pointer slice. subscription_transactions.go also picks up a free win: Transaction: *AdaptTransaction(txn) → AdaptTransaction(txn) drops both an alloc and a deref-copy at :169/:191.
Value slices removed a latent nil-deref, not just indirection. AdaptFeederBlockTrace reads block.Transactions[index].Type (rpc/v9/adapters.go:175, rpc/v10/adapt_trace.go:323). With []*Transaction a nil tail entry — which the old header.TransactionCount sizing could produce — would panic there; []Transaction can't. Both call sites are read-only, so no aliasing semantics changed. TransactionWithReceipt.Receipt going from pointer to value has no in-place mutators either: AdaptReceipt has no callers outside rpc/v9/rpc/v10 (rpc/v8 keeps its own 5-arg signature, untouched), and BlockWithReceipts is the only consumer.
Wire format still byte-identical. The one way a pointer→value flip silently changes output is a pointer-receiver MarshalJSON being skipped on a non-addressable value. Neither Transaction, TransactionReceipt, nor TransactionReceiptWithBlockInfo declares MarshalJSON/UnmarshalJSON/MarshalText in either version, and the only pointer-receiver marshaller in the tree (*ResourceBoundsMap) is held through a pointer field, so it's reached either way. Embedding flattens as before (block_hash omitempty, block_number always emitted — matching the old always-non-nil *uint64). jsonrpc/server.go:565 sets res.Result = tuple[0].Interface() only after the *jsonrpc.Error is confirmed nil, so the return Transaction{}, err / return TransactionReceiptWithBlockInfo{}, err paths can't leak an empty object where null used to go. The TransactionReceipt → TransactionReceiptWithBlockInfo split also removes any embedded-field ambiguity, which matters under the GOEXPERIMENT=jsonv2 test job since json/v2 rejects ambiguous inlined members.
Test churn is mechanical. assert.Nil → assert.Empty is the exact value-type equivalent (testify's isEmpty falls back to reflect.DeepEqual against reflect.Zero), require.Equal(t, &expected, got) → require.Equal(t, expected, got) preserves the assertion, and for _, receipt → for i := range + &receipts[i] in assertNextReceipts is assertion-neutral (assertNextMessage marshals and string-compares the wire bytes). Dropping require.NotNil(t, expected.BlockNumber) remains correct — the fixtures legitimately carry "block_number": 0.
Findings
Nit
rpc/v9/subscription_receipts.go:110—&receipton the range variable heap-allocates the loop variable once per yielded receipt (moved to heap: receiptat v9:109/:127, v10:110/:128). Againstmainthis path trades 2 allocs + 0 copies for 1 alloc + ~2 × 168-byte copies, so the commit message's "avoid 168-byte copies" is the delta vs0a6aac0, not vsmain. Hoisting the variable above the loop would make it one allocation per block — safe, sincesendResponsemarshals synchronously and doesn't retain the pointer. Second-order either way:sendResponseallocates amap[string]anyplus a marshal buffer per message, which dwarfs the struct copy.
Housekeeping
- Two commits still want squashing before merge:
8869d65 "leftover"(one-line test change) and0d165af "rebase fix"(folds into theadaptTransactionschange).
Remaining ask
Benchmarks — @brbrr's request from Aug 7 is still open, and this is the round where the story is cleanest to tell: BlockWithTxs / BlockWithReceipts go from N heap structs + a pointer slice to a single value slice, which escape analysis now confirms. Harness precedent is rpc/v10/addtxn_bench_test.go; benchstat vs origin/main would make the title concrete. go test -bench isn't in my allowed tool list, so this one has to come from you.
Verification: go vet ./rpc/... clean across all versions (this type-checks the test files too), plus go build -gcflags='-m' on rpc/v9 and rpc/v10 for every escape/alloc claim above. go test isn't in my allowed tool list, so statements about test behaviour come from reading, not execution — CI's golangci-lint and CodeQL are green on 0d165af; Juno Test was still in progress when I looked.
· branch perf/rpc/receipt-cont
k6 end-to-end (100k sequential iterations, 1 VU, warm node)Corpus: 10k uniform blocks (
Cold-start runs agreed directionally (up to −5.4% median on getBlockWithTxs). End-to-end the |
Micro BenchmarkBlockWithTxs
BlockWithReceipts
|
|
Claude finished @EgeCaner's task in 4m 29s —— View job Re-review after push (
|
| Item | Status |
|---|---|
| @brbrr: benchmark numbers | ✅ Closed — k6 end-to-end + per-block micro numbers posted above |
iter.Seq[TransactionReceiptWithBlockInfo], stale v10 comment, AdaptCoreTransaction duplication, 2-alloc handler returns, header.TransactionCount sizing |
✅ All closed in earlier rounds |
&receipt loop-var escape |
🟡 Nit, declined — fine |
On the benchmarks: the pre_confirmed column is the strongest evidence (−67% to −75% allocs, −20% to −37% time) and matches what escape analysis predicts, since that path has no DB/trie work to hide behind. The marshal rows showing +2.0%/+2.7% on BlockWithReceipts with fewer allocs are the one oddity — most likely a value slice of large TransactionWithReceipt structs costing more in reflect-driven traversal than the pointer chase it replaced, and plausibly within run-to-run noise at 3 significant figures. Not worth blocking on given the end-to-end numbers are neutral-to-positive, but if it reproduces under -count=10 | benchstat it'd be the one place this PR trades time for allocs. Those numbers predate this commit and are unaffected by it — 64afdce touches only the subscription path.
Housekeeping (unchanged)
Two commits still want squashing before merge: 8869d65 "leftover" (one-line test change) and 0d165af "rebase fix" (folds into the adaptTransactions change).
Follow-up, out of scope
rpc/v8/subscriptions.go:665 still has the old map[string]any sendResponse. Leaving v8 alone is the right call for a perf PR — just flagging that the three versions now differ in shape, and that v9 declares the generic types in subscriptions.go while v10 puts them in subscription_types.go.
Verification: go vet ./rpc/v9/... ./rpc/v10/... clean (this type-checks the test files too), plus go build -gcflags='-m' on both packages for every escape/alloc claim above. I could not run go test or go test -bench (not in my allowed tool list), so statements about test behaviour come from reading, not execution — the benchmark commentary is analysis of your posted numbers, not a re-run. CI on 64afdce: golangci-lint, CodeQL, Code Quality and Dependency Review green; Juno Test still in progress when I looked.
· branch perf/rpc/receipt-cont
| type SubscriptionParams[T any] struct { | ||
| Result T `json:"result"` | ||
| SubscriptionID string `json:"subscription_id"` | ||
| } | ||
|
|
||
| type SubscriptionResponse[T any] struct { | ||
| Version string `json:"jsonrpc"` | ||
| Method string `json:"method"` | ||
| Params SubscriptionParams[T] `json:"params"` |
There was a problem hiding this comment.
Saved per notification: the map[string]any header and its bucket (2), the heap box for the id string stored as any (1), and encoding/json's per-call map machinery — the reflected key collection, the sortable key/value slice, and per-key string resolution (~5) — a constant 8 allocations per message regardless of payload size (9 for value payloads like transactionStatus, which also skip one any box).
There was a problem hiding this comment.
| type | payload | sec/op | allocs/op |
|---|---|---|---|
| newHeads | full adapted header | 1.62µs → 1.26µs | 25 → 17 |
| events | 1 event (1 key, 1 data felt) | 975ns → 635ns | 16 → 8 |
| newTransaction | invoke v3, 2 calldata + 1 sig | 2.54µs → 2.16µs | 27 → 19 |
| newTransactionReceipts | receipt with 1 event | 1.32µs → 978ns | 20 → 12 |
| transactionStatus | hash + finality/execution status | 703ns → 355ns | 14 → 5 |
| reorg | 2 hashes + 2 block numbers | 649ns → 320ns | 12 → 4 |
brbrr
left a comment
brbrr
left a comment
There was a problem hiding this comment.
LGTM. I've left a single observation that could potentially remove another allocation. Leaving it up to you to decide if it's worth doing.
| func sendTransactionReceipt(wsConn jsonrpc.Conn, receipt *TransactionReceipt, id string) error { | ||
| func sendTransactionReceipt( | ||
| wsConn jsonrpc.Conn, | ||
| receipt *TransactionReceiptWithBlockInfo, |
There was a problem hiding this comment.
Can we pass the receipt by value here? It should prevent receipt to escape to heap. Same call in v9
2 participants
No description provided.