Skip to content

rpc: eth_getProof support MarshalFastJSONTo - #24105

Merged
awskii merged 46 commits into
mainfrom
alex/getproof_marshal_fast_json_to_37_2
Sep 19, 2026
Merged

awskii merged 46 commits into
mainfrom
alex/getproof_marshal_fast_json_to_37_2

Conversation

@AskAlexSharov

@AskAlexSharov AskAlexSharov commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator

eth_getProof through rpc.Server.ServeHTTP, 9 account-proof nodes and 7 nodes per slot, base #24086 vs this PR (n3, AMD Ryzen 9 5900X, GOMAXPROCS=4, 10 runs):

slots time throughput allocated per request allocs peak RSS
0 29.1µs → 17.2µs (−41%) 0.29 → 0.49 GiB/s 18.3 KiB → 8.6 KiB 81 → 65 22.4 → 22.1 MiB
10 137µs → 44.5µs (−67%) 0.52 → 1.60 GiB/s 91 KiB → 8.8 KiB 159 → 63 22.9 → 22.1 MiB
100 1.06ms → 283µs (−73%) 0.60 → 2.25 GiB/s 760 KiB → 8.5 KiB 881 → 63 29.5 → 25.4 MiB
1024 (maxGetProofKeys) 9.86ms → 2.74ms (−72%) 0.65 → 2.35 GiB/s 7.07 MiB → 8.5 KiB 8274 → 63 60.8 → 67.3 MiB

Replaces #24089: based on #24086 instead of the RPCLogs stack, and without a JSONAppender interface. Quantities go through WriteQuotedText(encoding.TextAppender), so no hexutil type needs its own stream method.

Stacked on #24086.

The stream hex-encodes straight into its own buffer; the tracer logger writes memory words, storage values and return data through it.
…etCode

writeResponse streams a MarshalFastJSONTo result into the response, and hexutil.Bytes writes itself with WriteHex.
…ng MarshalFastJSONTo

The json/v2 benchmark moves to its own go1.27 file. The transport failure test also covers a MarshalFastJSONTo result that fails before writing.
A result writes one value, so the lazy open guard is not needed yet; responseWriter keeps main's method shape.
… racing the service loop

TestBlockServiceQueuesClockBoundaryBlockForRetry called processScheduledBlock while the service's loop could hold the job; the direct call then returned at once and calls was still 1.
The transport failure test body stays unchanged behind a helper, and the typed-nil case joins the streaming test.
…nchmark

A slice value is boxed and copied by reflect.New on every call, which charged json/v2 two allocations the server path never pays.
writeResponse and streamable methods share writeLazyResponse: the envelope goes first, LazyFieldStream opens "result" on the first value, and an error becomes "error". responseWriter is gone; json.Encoder output reaches the stream through a small io.Writer.
…hrough WriteRawBytes

WriteRawBytes appended the value, which already sat in the buffer tail, onto itself.
…n_to_37

# Conflicts:
#	common/hexutil/bytes.go
Every field name Erigon writes is a source literal or a hex string, so
appending it raw is enough; ERIGON_ASSERT catches a name that is not.
runMethod writes the error into the stream, so it returns it too: handleCall counts the failed
request and handleCallMsg logs it, as before the lazy response. A typed nil takes the reflection
path for every fast-JSON interface.
…alFastJSONTo

The stream gains WriteHexUint64 and WriteHexQuantity, so a quantity needs no intermediate buffer.
A []byte hint escapes through the interface call: 1024 storage slots allocated 1024 times.
Lets a field and its value chain: s.WriteObjectField("gas").WriteHex(v).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Direct RPC response paths still do not propagate errors latched during quoted text encoding.

Get a fresh assessment by requesting another Copilot review.

Review details
  • Files reviewed: 9/9 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread rpc/json.go
AskAlexSharov and others added 2 commits September 18, 2026 15:45
…to_37_2' into alex/jsonstream_raw_field_37

# Conflicts:
#	rpc/jsonstream/lazy_field_stream.go
#	rpc/jsonstream/stack_stream.go
#	rpc/jsonstream/stream.go
Reason: Field names are hardcoded in Erigon (or hex)

n5, 8 cores: `"gas"` 7.25ns -> 3.47ns, a 66-char hex key 19.1ns ->
4.86ns.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The field-name fast path can emit malformed JSON, and the narrowed pooling API breaks existing callers.

Get a fresh assessment by requesting another Copilot review.

Review details
  • Files reviewed: 12/12 changed files
  • Comments generated: 2
  • Review effort level: Balanced

Comment thread rpc/jsonstream/write_string.go
Comment thread rpc/jsonstream/factory.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Text-appender failure recovery can emit malformed buffered JSON-RPC output.

Get a fresh assessment by requesting another Copilot review.

Review details
  • Files reviewed: 12/12 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread rpc/json.go

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The serialization behavior and error paths are covered by focused tests with no unresolved correctness issues.

Review details
  • Files reviewed: 13/13 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@AskAlexSharov
AskAlexSharov added this pull request to the merge queue Sep 19, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Sep 19, 2026
@awskii
awskii added this pull request to the merge queue Sep 19, 2026
Merged via the queue into main with commit 6d0a97c Sep 19, 2026
143 checks passed
@awskii
awskii deleted the alex/getproof_marshal_fast_json_to_37_2 branch September 19, 2026 05:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants