perf: PBJReader/PbjWriter optimization branch by ldintr · Pull Request #883 · hashgraph/pbj

ldintr · 2026-08-07T19:39:03Z

Description:

My branch for PBJ, it switches to PbjReader and PbjWriter instead of using an interface. parse and write are significantly faster. Here are some notes

parse and write have overloads that use a thread local storage object, prefer that to creating new PbjReader/PbjWriter
PbjWriter has a toPbjReader method which reuses the buffer. It can be used as long as you don't need to write to the writer object before the reader is done. Otherwise use toByteArray to clone the buffer
Bytes now has array() and arrayOffset() so PbjReader doesn't need to copy data
RandomAccessSequenceAdapter has been changed for the same reason (to skip a copy)
Many exceptions were deleted. PbjReader/PbjWriter doesn't throw checked exceptions. If you need them you can use throwOnError(), or use error() to get an error code
Some exceptions look at the error message, prefer using error codes in the future
Many functions that have ReadableSequentialData have either the signature change to PbjReader or had the function copied with the parameter changed to PbjReader. Same with WritableSequentialData and PbjWriter
pbj.ReaderWriter.useStackTrace defaults to true, use false to use premade exceptions/stacktraces which will run faster

Related issue(s):

Fixes #

Notes for reviewer:

parse and write were changed to realParse and realWrite and changed back in the cleanup. That was to help me develop easier. hasMore changed back to hasRemaining

Checklist

Documented (Code comments, README, etc.)
Tested (unit, integration, etc.)

github-actions · 2026-08-07T19:41:51Z

JUnit Test Report

521 files ± 0 521 suites ±0 31s ⏱️ ±0s
1 552 tests +15 1 548 ✅ +15 4 💤 ±0 0 ❌ ±0
10 770 runs +15 10 742 ✅ +15 28 💤 ±0 0 ❌ ±0

Results for commit a30bb06. ± Comparison against base commit 8770b39.

This pull request removes 6 and adds 21 tests. Note that renamed tests count towards both.

com.hedera.pbj.runtime.ProtoWriterToolsTest ‑ [1] FLOAT, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x000000005e49cd30@6df4d8f1, [0.1, 0.5, 100.0], 12, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x000000005e49cf58@735b1ad2
com.hedera.pbj.runtime.ProtoWriterToolsTest ‑ [1] STRING, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x000000005e4a91b8@230e163b, [string 1, testing here, testing there], com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x000000005e4a93e0@7a41f4a7
com.hedera.pbj.runtime.ProtoWriterToolsTest ‑ [2] BYTES, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x000000005e4a9608@7164e54, [010203, ff7f0f, 42da07370bff], com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x000000005e4a9830@20040c6e
com.hedera.pbj.runtime.ProtoWriterToolsTest ‑ [2] DOUBLE, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x000000005e49d180@88fc9cb, [0.1, 0.5, 100.0, 1.7653472635472653E240], 32, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x000000005e49d3a8@75693526
com.hedera.pbj.runtime.ProtoWriterToolsTest ‑ [3] BOOL, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x000000005e49d5d0@603a34e5, [true, false, false, true, true, true], 6, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x000000005e49d7f8@160d68b8
com.hedera.pbj.runtime.ProtoWriterToolsTest ‑ [4] ENUM, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x000000005e49da20@1d77d9c6, [0, 2, 1], 3, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x000000005e49dc48@2c5b4b91

com.hedera.pbj.runtime.ProtoParserToolsTest ‑ [10] "\U00010000Linear B Syllable"
com.hedera.pbj.runtime.ProtoParserToolsTest ‑ [11] "\U000e01ef4 byte char"
com.hedera.pbj.runtime.ProtoParserToolsTest ‑ [12] "�"
com.hedera.pbj.runtime.ProtoParserToolsTest ‑ [13] "Ŀ"
com.hedera.pbj.runtime.ProtoParserToolsTest ‑ [1] "Test Ascii"
com.hedera.pbj.runtime.ProtoParserToolsTest ‑ [2] "UTF16 ☃"
com.hedera.pbj.runtime.ProtoParserToolsTest ‑ [3] "Hangul Syllable Hwen 휀"
com.hedera.pbj.runtime.ProtoParserToolsTest ‑ [4] "Private Use E000 "
com.hedera.pbj.runtime.ProtoParserToolsTest ‑ [5] "Linear B Syllable \U00010000"
com.hedera.pbj.runtime.ProtoParserToolsTest ‑ [6] "4 byte char \U000e01ef"
…

♻️ This comment has been updated with latest results.

github-actions · 2026-08-07T19:48:04Z

Integration Test Report

1 files - 427 1 suites - 427 38s ⏱️ - 12m 49s
1 tests - 115 047 1 ✅ - 115 047 0 💤 ±0 0 ❌ ±0
1 runs - 115 291 1 ✅ - 115 291 0 💤 ±0 0 ❌ ±0

Results for commit a30bb06. ± Comparison against base commit 8770b39.

This pull request removes 115047 tests.

com.hedera.hapi.block.stream.input.tests.EventHeaderTest ‑ [1] NoToStringWrapper{com.hedera.hapi.block.stream.input.EventHeader}
com.hedera.hapi.block.stream.input.tests.EventHeaderTest ‑ [2] NoToStringWrapper{com.hedera.hapi.block.stream.input.EventHeader}
com.hedera.hapi.block.stream.input.tests.EventHeaderTest ‑ [3] NoToStringWrapper{com.hedera.hapi.block.stream.input.EventHeader}
com.hedera.hapi.block.stream.input.tests.EventHeaderTest ‑ [4] NoToStringWrapper{com.hedera.hapi.block.stream.input.EventHeader}
com.hedera.hapi.block.stream.input.tests.EventHeaderTest ‑ [5] NoToStringWrapper{com.hedera.hapi.block.stream.input.EventHeader}
com.hedera.hapi.block.stream.input.tests.EventHeaderTest ‑ [6] NoToStringWrapper{com.hedera.hapi.block.stream.input.EventHeader}
com.hedera.hapi.block.stream.input.tests.EventHeaderTest ‑ [7] NoToStringWrapper{com.hedera.hapi.block.stream.input.EventHeader}
com.hedera.hapi.block.stream.input.tests.EventHeaderTest ‑ testCodecGetDefaultValueMethod()
com.hedera.hapi.block.stream.input.tests.EventHeaderTest ‑ testTestEqualsAndHashCode()
com.hedera.hapi.block.stream.input.tests.RoundHeaderTest ‑ [1] NoToStringWrapper{com.hedera.hapi.block.stream.input.RoundHeader}
…

♻️ This comment has been updated with latest results.

Signed-off-by: ldintr <levo.d@swirldslabs.com>

anthony-swirldslabs · 2026-08-12T22:28:21Z

                .replace("$readMethod", field.type() == Field.FieldType.ENUM ? "value" : readMethod(field))
                .replace("$maxSize", field.maxSize() >= 0 ? String.valueOf(field.maxSize()) : "maxSize")
                .replace("$fieldName", field.name())
+                .replace("$divideString", divideAmount == 1 ? "" : "/%s".formatted(divideAmount))


%s is designed to handle Formattable objects, or it falls back to calling toString(). This means that it accepts an object as an argument. Which in turn means that the primitive integer variable divideAmount gets boxed into an Integer at runtime. This may be inefficient. Is there a reason not to use %d?

anthony-swirldslabs · 2026-08-12T22:35:45Z

+     * {@link #length()} bytes. Mutating the returned array breaks the immutability contract of this class.
+     *
+     * @return the internal backing byte array
+     */
+    @NonNull
+    public byte[] array() {


It's nice to mention that the API is unsafe in the javadoc, but the method name still looks too very inviting. An engineer may not think it through when using an auto-complete in their favorite IDE and choose this unsafe version vs. the safe toByteArray. I suggest to rename this method to arrayUnsafe() instead.

Could you provide more info when users may need the backing array, please? It would be great to avoid exposing the array in API in any way (as Bytes does)

Oh, wait, this is Bytes, not PbjReader or PbjWriter. So exposing the backing array is not acceptable

anthony-swirldslabs · 2026-08-12T22:50:16Z

+    /** The raw backing byte array. */
+    byte[] byteArray();


Similar to above, this method is public and it may expose the underlying array which may be considered immutable by e.g. Bytes. Both the name of this method and its javadoc need to change to make it less likely to misuse it in applications. E.g. byteArrayUnsafe might be a better name.

I am not sure this interface is the right way to provide access to a byte array to PbjReader. What is Bytes.toPbjReader() hands over its backing array to the created PbjReader without exposing any methods like array() or arrayOffset()? Also, need to make sure the array cannot be accessed in PbjReader in any legal way

anthony-swirldslabs · 2026-08-12T22:53:20Z

+    private final boolean useStacktrace =
+            !"false".equalsIgnoreCase(System.getProperty("pbj.ReaderWriter.useStackTrace"));


System properties cannot change at runtime. So reading it every time a new instance of the PbjReader is created is inefficient. You want to make this field static to read it just once during a JVM run.

Good catch, I updated both places

anthony-swirldslabs · 2026-08-12T22:56:15Z

+    public static final int EOF = -1,
+            DataEncoding = 1,


Minor: per Java Coding Conventions, static fields should use UPPER_CASE_SNAKE_NAMES. The current naming looks unnatural to a Java programmer and causes a confusion because the names can be seen as inner class names instead. It would be nice to rename all the error codes accordingly.

The comment applies to any and all static fields, in this class and elsewhere in this fix.

I'll probably change this to all caps. Does anyone prefer the way it is? If no one does Ill change

anthony-swirldslabs · 2026-08-13T00:22:00Z

+            int len = measureRecord(item);
+            PbjWriter writer = new PbjWriter(len, false);
+            write(item, writer);
+            return writer.internalArrayWrapped();


Internal array in a writer is generally mutable, so wrapping it in Bytes might violate its mutability contract. You want to carefully describe the intended usage in the javadoc to avoid misuse of this method.

anthony-swirldslabs · 2026-08-13T18:02:32Z

        writer.accept(out);
    }

+    public static void writeDelimited(


Minor: for arrays, we've introduced a separate ProtoArrayWriterTools. The reason for that was because this ProtoWriterTools.java was already too long a file. I suggest to move all the PbjWriter-backed methods into a new class for the same reason because it's difficult to work with source files that are thousands lines long.

I want them next to eachother for the moment so its easy to see theyre both the same implementation. I imagine we can't delete the WritableSequentialData version anytime soon? If we can I can move things elsewhere

anthony-swirldslabs · 2026-08-13T18:05:13Z

+    // return length or 0 on error
+    static int encodedLength(String sz) {


Minor: this API could be error-prone if sz.length() == 0. Please consider a different value to indicate an error.

I was worried about someone assuming positive only values and getting a negative count losing data if they're summing things up. I believe elsewhere I also return empty string on error so its consistent. I'll see if I can move this and others into pbj writer (or reader) so the error can be found in one place

anthony-swirldslabs · 2026-08-13T18:07:26Z

        }
    }

+    static void WriteUTF8(String str, PbjWriter out) {


In Java, method names should start with a lower-case letter. Apart from being a part of Java Coding Conventions, in practice this helps avoid confusing them with class names and constructor names.

Done. I missed that one :( most places had writeUTF8

anthony-swirldslabs · 2026-08-13T18:10:59Z

@@ -1 +1 @@
-0.15.0-SNAPSHOT
+0.pbj.1


This may be a part of your current testing/CN-repo-interoperability setup. However, we don't want to merge this change in version.txt into main.

artemananiev · 2026-08-13T23:31:12Z

    }

+    @NonNull
+    public PbjReader toPbjReader() {


Is this one really needed? There's clear way to create a PbjReader from Bytes, just use PbjReader constructor, no need to expose any Bytes internals to that class or whatever

I did it to keep it consistent with how people use bytes with toReadableSequentialData. I think I should delete it because the parse have Bytes overload, but I dont know if deleting this will make it harder to switch to my new code.

artemananiev · 2026-08-13T23:33:53Z

+     *
+     * @return start offset into the backing array
+     */
+    public int arrayOffset() {


This looks even more internal (and therefore subject to removal) than array(). I've always found ByteBuffer.arrayOffset() confusing, and this one looks pretty similar to that

artemananiev · 2026-08-13T23:49:31Z

+        boolean inUse = false;
+    }
+
+    ThreadLocal<WriteCache> tlsWriter = ThreadLocal.withInitial(WriteCache::new);


This means tons of thread locals, one per generated Codec class (there are many!)

Actually, two per class

Thats true in C++, I asked claude and it said one per thread and mentioned type erasers. I just double checked now by writing code and it seems like test spawn more workers than threads I have on my machine. But I can see it's not making more than one per thread

artemananiev · 2026-08-13T23:51:42Z

 */
 public interface Codec<T> {
+    class WriteCache {
+        PbjWriter writer = new PbjWriter();


This looks like an implementation optimization rather than a part of the contract. It would be better to place it to a subclass, e.g. introduce AbstractCodec implements Codec and change codec generation code to use AbstractCodec. This would not be perfect, either, but still slightly better than extra classes and instance variables in an interface, from my perspective

artemananiev · 2026-08-13T23:56:00Z

+            throws ParseException {
+        ReadCache cache = tlsReader.get();
+        if (cache.inUse) {
+            PbjReader reader = new PbjReader(input);


New PbjReader per recursive call. Is it possible to wrap a ReadableSequentialData input into a PbjReader just once and use it for all recursive calls?

In general, this parse/wrapParse approach looks somewhat confusing to me, especially at the interface level

artemananiev · 2026-08-13T23:56:57Z

+    }
+
+    @NonNull
+    default T parse(@NonNull InputStream in, boolean strictMode, boolean parseUnknownFields, int maxDepth, int maxSize)


Are all these new parse() methods actually needed?

There's code in CN that overloads just a few parameters, which is why I changed the generated code to realParse briefly, so I can have it run the expected code. I think having all these overloads would be better than rearchitecture the code

artemananiev · 2026-08-14T00:01:21Z

+        return di;
+    }
+
+    public static int fromUTF8(char[] dst, byte[] src, int offset, int pos, int length) {


JavaDoc, please

artemananiev · 2026-08-14T00:02:45Z

        return bytes;
    }

+    public static Bytes readBytes(PbjReader input, final long maxSize) {


JavaDoc, please.

Why isn't it consistent with the method above (doesn't throw ParseException, etc.)?

Signed-off-by: ldintr <levo.d@swirldslabs.com>

ldintr requested review from a team as code owners August 7, 2026 19:39

ldintr self-assigned this Aug 7, 2026

ldintr force-pushed the ldintr-cn-main branch from 035af99 to 4e85871 Compare August 7, 2026 19:40

ldintr changed the title ~~Ldintr cn main~~ opt: Ldintr cn main Aug 7, 2026

ldintr changed the title ~~opt: Ldintr cn main~~ opt: PBJReader/PbjWriter optimization branch Aug 7, 2026

ldintr changed the title ~~opt: PBJReader/PbjWriter optimization branch~~ perf: PBJReader/PbjWriter optimization branch Aug 7, 2026

ldintr added 13 commits August 7, 2026 16:02

Removing Interface

656e2e0

Signed-off-by: ldintr <levo.d@swirldslabs.com>

Edit tests to not allocate SlimBuffer so often

ae457ab

Signed-off-by: ldintr <levo.d@swirldslabs.com>

Remove Exceptions

81f92de

Signed-off-by: ldintr <levo.d@swirldslabs.com>

Removed many calls to addToList

6fe9870

Signed-off-by: ldintr <levo.d@swirldslabs.com>

Improve Strings

4ff6f41

Signed-off-by: ldintr <levo.d@swirldslabs.com>

SlimBuffer/SlimWriter Change

b690d8d

Signed-off-by: ldintr <levo.d@swirldslabs.com>

Changes for CN

9ab0b57

Signed-off-by: ldintr <levo.d@swirldslabs.com>

Premade Exceptions and minor changes

8f02557

Signed-off-by: ldintr <levo.d@swirldslabs.com>

Rename Slim to Pbj

55adca6

Signed-off-by: ldintr <levo.d@swirldslabs.com>

More test and many changes

140885f

Signed-off-by: ldintr <levo.d@swirldslabs.com>

Removed some toReadableSequentialData, and cached more

a7b4df7

Signed-off-by: ldintr <levo.d@swirldslabs.com>

Minor refactor, more test, and doc strings

ad69c3d

Signed-off-by: ldintr <levo.d@swirldslabs.com>

Cleanup

ace063d

Signed-off-by: ldintr <levo.d@swirldslabs.com>

ldintr force-pushed the ldintr-cn-main branch from 4e85871 to ace063d Compare August 7, 2026 20:02

anthony-swirldslabs reviewed Aug 13, 2026

View reviewed changes

artemananiev reviewed Aug 13, 2026

View reviewed changes

artemananiev reviewed Aug 14, 2026

View reviewed changes

Made suggest changes, and changes I made since the PR

a30bb06

Signed-off-by: ldintr <levo.d@swirldslabs.com>

ldintr force-pushed the ldintr-cn-main branch from 1e03946 to a30bb06 Compare August 14, 2026 17:07

		private final boolean useStacktrace =
		!"false".equalsIgnoreCase(System.getProperty("pbj.ReaderWriter.useStackTrace"));

		// return length or 0 on error
		static int encodedLength(String sz) {

		@@ -1 +1 @@
		0.15.0-SNAPSHOT
		0.pbj.1

Uh oh!

Conversation

ldintr commented Aug 7, 2026

Uh oh!

github-actions Bot commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

JUnit Test Report

Uh oh!

github-actions Bot commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Integration Test Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Aug 7, 2026 •

edited

Loading

github-actions Bot commented Aug 7, 2026 •

edited

Loading