Skip to content

perf: reverse writer into a reusable buffer - #1003

Open
jasperpotts wants to merge 22 commits into
1000-array-parser-part2from
1001-reverse-writer
Open

jasperpotts wants to merge 22 commits into
1000-array-parser-part2from
1001-reverse-writer

Conversation

@jasperpotts

@jasperpotts jasperpotts commented Oct 30, 2026 •

Copy link
Copy Markdown
Member

What and why

This PR is step 3 of docs/fast-codegen-plan.md (sections 5.4 and 6.4), opt-in with pbj.fast_write: a back-to-front writer into a reusable buffer, which needs no measuring pass and allocates nothing.

Performance (AMD EPYC 9275F): the reverse writer is 9-20% faster for objects encoded while still in cache, with 0 B/op, but 2.7-4.3× slower on all of a block's items out of cache, and slower whenever sizes are already cached. So it is kept as an off-by-default option (tier B).

  • ReverseBuffer (io.buffer): a reusable, growable buffer written from the end toward the start.
  • New additive API, Codec.writeReverse(T, ReverseBuffer): codecs without fast_write measure, then write forward into the buffer.
  • Generated writeReverseTo for fast_write messages: it writes the unknown fields first, then the fields in descending order and list elements in reverse. Each value goes before its length and tag, so nested lengths are known without measuring. Maps are written as entries in reverse sorted-key order and wrapper types as value, then length and tag; only messages with maps of enums write themselves forward into the buffer.
  • Off by default (tier B): toBytes and the other write methods never use the reverse writer; only writeReverse does.
  • Tests:
    • every generated test checks that writeReverse gives exactly toBytes' bytes, into a new and into a reset buffer;
    • AllocationTest checks 0 B/op into a warm buffer;
    • BlockItemBench gains itemWriteReverseFresh, itemWriteReverseCached and transactionWriteReverseFresh.
  • Also on this branch: the array-parser method-size fix from perf: byte array parser with in-order fast paths, part 2 #1002, merged in.

Generated code for TransactionID (excerpt):

// [4] - nonce
final var v_nonce = data.nonce();
if (v_nonce != 0) {
    out.varint(v_nonce);
    out.writeByte(0x20); // tag: field 4, wire type 0
}
...
// [1] - transactionValidStart
final var v_transactionValidStart = data.transactionValidStart();
if (v_transactionValidStart != null) {
    final int mark = out.mark();
    com.hedera.hapi.node.base.codec.TimestampProtoCodec.writeReverseTo(v_transactionValidStart, out);
    out.lengthSince(mark);
    out.writeByte(0x0A); // tag: field 1, wire type 2
}

Tests

  • pbj-core: spotlessApply qualityGate build passes.
  • pbj-integration-tests: ./gradlew build and ./gradlew build -PpbjPerformance=max both pass, 326,406 tests each. The reverse writer runs on all 579 HAPI and test messages, and its bytes equal toBytes for every test object.

Result on AMD EPYC: kept as an off-by-default option

Decision: keep pbj.fast_write as a tier B option, off by default, and document when it pays. Investigating the out-of-cache slowdown on HAPI is follow-up work and does not block merging.

BlockItemKindBench, everything on (-PpbjPerformance=max), 3 forks, -prof gc, µs per batch:

STATE_CHANGES items Forward, fresh Reverse, fresh Forward, sizes cached Reverse, sizes cached
200 items, in cache 89.4 71.5 (−20%) 63.2 81.5 (+29%)
all 21,986 items, out of cache 13,844 39,334 (2.8× slower) 9,200 39,095 (4.3× slower)
TRANSACTION_RESULT items Forward, fresh Reverse, fresh Forward, sizes cached Reverse, sizes cached
200 items, in cache 14.6 13.2 (−9.4%) 10.7 13.3 (+24%)
all 21,974 items, out of cache 1,755 5,648 (3.2× slower) 1,527 4,060 (2.7× slower)
  • Wins: objects encoded right after they are built, while still in cache: no measuring pass.
  • Loses: large out-of-cache workloads such as HAPI's blocks, because it reads each object graph last field first, against allocation order (JFR puts trivial getters such as OneOf.kind() at the top). It also loses whenever sizes are already cached.
  • Follow-ups that changed nothing measurable: writing maps and wrapper types in reverse, and batching capacity checks (ReverseBuffer.ensure plus unchecked writes).
  • Bug fix: the unknown-field write reserved space after taking the buffer's array, which broke when the buffer grew. UnknownFieldsTest.testUnknownFieldsWriteReverseIntoSmallBuffer covers it.
  • Docs: docs/codec-protobuf.md ("Reverse Writer") and docs/usage-guide.md describe the API, give these numbers and tell users to measure on their own data before turning it on.

Allocation: 0 B/op for reverse writes into a warm buffer.

Generated code size (full HAPI build, -PpbjPerformance=max)

#1002 This PR
All classes 60,424,468 61,397,386

Stacked on #1002.

Fixes #1001

🤖 Generated with Claude Code

jasperpotts and others added 2 commits October 29, 2026 17:51
Adds step 3 of docs/fast-codegen-plan.md (sections 5.4 and 6.4), opt-in with pbj.fast_write:
- ReverseBuffer: a reusable, growable buffer written back to front; marks count bytes written, so
  they survive growth; results via bytes() (a view), toBytes() or copyTo();
- Codec.writeReverse(T, ReverseBuffer), additive public API; codecs without fast_write measure and
  write forward into the buffer;
- generated writeReverseTo for fast_write messages: unknown fields, then fields in descending order
  and list elements in reverse, each value before its length and tag, so nothing is measured.
  Messages with maps or wrapper types write themselves forward into the buffer.

Every generated test checks writeReverse against toBytes; AllocationTest checks 0 bytes per reverse
write into a warm buffer; BlockItemBench gains reverse-write cases.

Fixes #1001

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Jasper Potts <1466205+jasperpotts@users.noreply.github.com>
@jasperpotts jasperpotts self-assigned this Oct 30, 2026
@github-actions

github-actions Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

JUnit Test Report

   556 files  +2     556 suites  +2   1m 10s ⏱️ -1s
 2 079 tests +6   2 075 ✅ +6   4 💤 ±0  0 ❌ ±0 
11 611 runs  +6  11 583 ✅ +6  28 💤 ±0  0 ❌ ±0 

Results for commit 7a209a4. ± Comparison against base commit 3b16f3d.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Integration Test Report

    632 files      632 suites   46m 34s ⏱️
163 976 tests 163 976 ✅ 0 💤 0 ❌
326 641 runs  326 641 ✅ 0 💤 0 ❌

Results for commit 7a209a4.

♻️ This comment has been updated with latest results.

Signed-off-by: Jasper Potts <1466205+jasperpotts@users.noreply.github.com>

# Conflicts:
#	pbj-integration-tests/src/jmh/java/com/hedera/pbj/integration/jmh/BlockItemBench.java
@jasperpotts

Copy link
Copy Markdown
Member Author

Merged in the review follow-ups from below in the stack: exact-size bulk varint writes (#994), unique writer local names (#995), dead-code removal (#996), map entries without try/finally plus tests (#997), and strict string decoding without per-call allocations (#1010). On #999 the array parser's ArrayCursor.string now uses the shared Utf8Tools.decodeUtf8Strict. The plan was updated too (#991). Re-verified: pbj-core spotless, quality gate and build, and the integration build with options off and with -PpbjPerformance=max, all passing. The benchmark tables in this PR's description predate these changes; writes and string parsing are now faster than shown (see #994 and #1010).

jasperpotts and others added 4 commits October 29, 2026 23:16
…everse

ReverseBuffer.reserve can grow the buffer and replace its array, so the
generated code must call it before out.array(). Adds a test that writes
an object carrying unknown fields into a 16-byte buffer.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Jasper Potts <1466205+jasperpotts@users.noreply.github.com>
Signed-off-by: Jasper Potts <1466205+jasperpotts@users.noreply.github.com>
…e per run of scalars

- Maps are written as their entries in reverse of the sorted key order,
  value before key, and wrapper types as the wrapped value, its length
  and tag, so messages with them no longer fall back to measuring and
  writing forward. Only maps of enums still do; HAPI has none.
- A run of adjacent scalar fields calls ReverseBuffer.ensure once with
  its largest encoded size, then writes with the new unchecked
  primitives.
- BlockItemKindBench times both writers on one kind of block item at a
  time.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Jasper Potts <1466205+jasperpotts@users.noreply.github.com>
… CPU caches

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Jasper Potts <1466205+jasperpotts@users.noreply.github.com>
@jasperpotts

Copy link
Copy Markdown
Member Author

Review follow-ups (second review, sections 1.1 and 3), all on this branch:

  • Bug fix (1.1): the unknown-field write took out.array() before out.reserve(...), which can grow the buffer and replace its array. It now reserves first. UnknownFieldsTest.testUnknownFieldsWriteReverseIntoSmallBuffer writes an object carrying unknown fields into a 16-byte buffer; without the fix it throws ArrayIndexOutOfBoundsException.
  • Maps and wrapper types are written in reverse (3.2). Maps are written as entries in reverse sorted-key order, value before key; wrapper types as the wrapped value, then length and tag. No HAPI message falls back any more (only maps of enums would).
  • Batched capacity checks (3.3). A run of adjacent scalar fields calls ReverseBuffer.ensure once with its largest encoded size, then writes with new unchecked primitives.
  • Per-kind breakdown (3.1): BlockItemKindBench times both writers on one kind of block item at a time, optionally limited to a few items that fit in the CPU caches.

Results (AMD EPYC, max options)

The follow-ups changed nothing measurable, so the fallbacks were not the cause. Per kind, with sizes cached (µs for all items of the kind):

Kind Items Forward Reverse before follow-ups Reverse after
STATE_CHANGES 21,986 9,189 35,275 38,963
TRANSACTION_RESULT 21,974 1,539 3,757 4,121
TRACE_DATA 839 321 353 346
SIGNED_TRANSACTION 21,981 283 275 269

The same jar on Apple Silicon shows reverse about equal to forward. JFR on EPYC put OneOf.kind() (a trivial getter) at the top of the reverse profile and nowhere in the forward one: the cost is memory access. Limiting the items to 200, which fit in the caches:

EPYC Forward cached Reverse cached Forward fresh Reverse fresh
STATE_CHANGES, 200 items 63.2 81.5 (+29%) 89.4 71.5 (−20%)
STATE_CHANGES, all 21,986 9,200 39,095 (+325%) 13,844 39,334 (+184%)
TRANSACTION_RESULT, 200 items 10.7 13.3 (+24%) 14.6 13.2 (−9.4%)
TRANSACTION_RESULT, all 21,974 1,527 4,060 (+166%) 1,755 5,648 (+222%)

Corrected cause: the reverse writer visits every object graph last field first, the opposite of the order the objects were allocated in. Out of cache, on EPYC, that costs 2-4×. In cache, it saves the measuring pass: 9-20% faster than step 1 on freshly built objects, 24-29% slower when sizes are already cached.

Recommendation: park this PR. It meets the plan's 10% bar only for objects encoded right after they are built and still in cache, and it is 2-4× slower whenever they aren't, which a whole-block write can't rule out. I have not reverted it out of the stack (section 0.3), since that touches #1005, #1007, #1015 and #1016; say if you want it out.

jasperpotts and others added 6 commits October 30, 2026 01:05
Signed-off-by: Jasper Potts <1466205+jasperpotts@users.noreply.github.com>
Signed-off-by: Jasper Potts <1466205+jasperpotts@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Jasper Potts <1466205+jasperpotts@users.noreply.github.com>
Signed-off-by: Jasper Potts <1466205+jasperpotts@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Jasper Potts <1466205+jasperpotts@users.noreply.github.com>
…riteReverse

Codec.writeReverse, and the generated fallback for maps of enums, reserve the
measured size inside a ReverseBuffer that already holds output and write
forward into it. A size mismatch would overwrite earlier output or leave stale
bytes, where the exactly-sized array of toBytes() would throw. Check the bytes
written against the measured size and throw IllegalStateException if they
differ.

This also fixes the default byte array writeImpl for hand-written codecs, which
returned the end offset instead of the number of bytes written when
startOffset was not zero.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Jasper Potts <1466205+jasperpotts@users.noreply.github.com>
Signed-off-by: Jasper Potts <1466205+jasperpotts@users.noreply.github.com>
jasperpotts and others added 6 commits October 30, 2026 13:45
Signed-off-by: Jasper Potts <1466205+jasperpotts@users.noreply.github.com>
ReverseBuffer writes downward into a buffer it owns, so a varint of up to 8
bytes can be written with one little-endian 8-byte store ending at its last
byte: the store also covers bytes below the varint, which are free space, so
it needs no end guard. This replaces sizeOfUnsignedVarInt64 plus the unrolled
writer, about a third of reverse encode time in profiles. Varints of 9 or 10
bytes, and varints with fewer than 8 bytes of room below them, keep the
existing path.

ReverseBufferVarintTest checks every varint length against the reference
writer, with and without room below and checked and unchecked, and that the
output already written is untouched. InlineBudgetTest covers the per-field
ReverseBuffer methods.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Jasper Potts <1466205+jasperpotts@users.noreply.github.com>
The 8-byte single-store varint did not win on AMD EPYC: block item reverse
encode was 1-4% slower (5 forks; 659 vs 621 ns cached, 686 vs 662 ns fresh
with the other changes, 646 vs 621 and 661 vs 662 ns without the varint read
change). Restores the unrolled writer and keeps ReverseBufferVarintTest, which
checks every varint length against the reference writer.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Jasper Potts <1466205+jasperpotts@users.noreply.github.com>
Signed-off-by: Jasper Potts <1466205+jasperpotts@users.noreply.github.com>
Signed-off-by: Jasper Potts <1466205+jasperpotts@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Jasper Potts <1466205+jasperpotts@users.noreply.github.com>
@jasperpotts
jasperpotts marked this pull request as ready for review November 2, 2026 20:10
@jasperpotts
jasperpotts requested review from a team as code owners November 2, 2026 20:10
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