Skip to content

perf: DH-23398: Improve barrage object serialization/deserialization. - #8396

Open
cpwright wants to merge 6 commits into
deephaven:mainfrom
cpwright:barrage-obj-serdes
Open

perf: DH-23398: Improve barrage object serialization/deserialization.#8396
cpwright wants to merge 6 commits into
deephaven:mainfrom
cpwright:barrage-obj-serdes

Conversation

@cpwright

Copy link
Copy Markdown
Contributor

No description provided.

@cpwright
cpwright requested a balanced review from Copilot August 19, 2026 23:59
@github-actions

github-actions Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

No docs changes detected for b09b415

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.

Pull request overview

Optimizes Barrage serialization and deserialization through bulk little-endian I/O and single-pass validity bitmap construction.

Changes:

  • Adds shared bulk readers/writers and validity-buffer packing.
  • Regenerates primitive chunk codecs, including optimized byte handling.
  • Extends round-trip benchmarks to String columns.

Reviewed changes

Copilot reviewed 34 out of 34 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
web/client-api/.../LittleEndianCodec.java Adds GWT char/byte codecs.
replication/static/.../ReplicateBarrageUtils.java Adds byte-reader fixups.
extensions/barrage/.../VarBinaryChunkWriter.java Bulk-writes offsets.
extensions/barrage/.../VarBinaryChunkReader.java Bulk-reads offsets.
extensions/barrage/.../UnionChunkWriter.java Uses shared validity packing.
extensions/barrage/.../ShortChunkWriter.java Updates generated writer.
extensions/barrage/.../ShortChunkReader.java Adds replicated payload regions.
extensions/barrage/.../RunEndEncodedChunkWriter.java Adopts validity API.
extensions/barrage/.../NullChunkWriter.java Packs all-null validity state.
extensions/barrage/.../MapChunkWriter.java Bulk-writes map offsets.
extensions/barrage/.../MapChunkReader.java Bulk-reads map offsets.
extensions/barrage/.../LongChunkWriter.java Updates generated writer.
extensions/barrage/.../LongChunkReader.java Adds replicated payload regions.
extensions/barrage/.../LittleEndianCodec.java Adds JVM char/byte codecs.
extensions/barrage/.../ListChunkWriter.java Bulk-writes list metadata.
extensions/barrage/.../ListChunkReader.java Bulk-reads list metadata.
extensions/barrage/.../IntChunkWriter.java Updates generated writer.
extensions/barrage/.../IntChunkReader.java Adds replicated payload regions.
extensions/barrage/.../FloatChunkWriter.java Updates generated writer.
extensions/barrage/.../FloatChunkReader.java Adds replicated payload regions.
extensions/barrage/.../FixedWidthObjectChunkWriter.java Adopts validity API.
extensions/barrage/.../DoubleChunkWriter.java Updates generated writer.
extensions/barrage/.../DoubleChunkReader.java Adds replicated payload regions.
extensions/barrage/.../DictionaryChunkWriter.java Adopts validity API.
extensions/barrage/.../CharChunkWriter.java Adds bulk char encoding.
extensions/barrage/.../CharChunkReader.java Adds bulk char decoding.
extensions/barrage/.../ByteChunkWriter.java Uses shared byte codec.
extensions/barrage/.../ByteChunkReader.java Directly transfers byte payloads.
extensions/barrage/.../BooleanChunkWriter.java Reuses validity bit-packing.
extensions/barrage/.../BigDecimalChunkWriter.java Adopts validity API.
extensions/barrage/.../BaseChunkWriter.java Adds validity and bulk-int writers.
extensions/barrage/.../BaseChunkReader.java Adds bulk-int reader.
extensions/barrage/benchmark/.../BarrageMessageRoundTripBenchmark.java Adds String benchmarks.
extensions/barrage/benchmark/build.gradle Adds String benchmark task.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@cpwright cpwright changed the title Barrage obj serdes perf: DH-23398: Improve barrage object serialization/deserialization. Aug 20, 2026
@cpwright
cpwright requested a balanced review from Copilot August 20, 2026 19:03

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.

Pull request overview

Copilot reviewed 36 out of 36 changed files in this pull request and generated no new comments.

Suppressed comments (1)

extensions/barrage/src/test/java/io/deephaven/extensions/barrage/chunk/BarrageColumnRoundTripTest.java:721

  • This full-window test only covers fixed-width payload buffers. The new BulkIntWriter / readIntBuffer paths for var-binary, list, and map offsets use a 1024-int window by default, while the ordinary 133-row round trips never fill it. Add a round trip with more than 1024 String rows (and preferably list/map coverage) so the full offset-window flush/read and following remainder are exercised.

@cpwright
cpwright requested a review from niloc132 August 20, 2026 19:19
@cpwright
cpwright marked this pull request as ready for review August 24, 2026 14:02
Copilot AI review requested due to automatic review settings August 24, 2026 14:54

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.

Pull request overview

Copilot reviewed 37 out of 37 changed files in this pull request and generated no new comments.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

extensions/barrage/src/main/java/io/deephaven/extensions/barrage/chunk/BaseChunkWriter.java:422

  • The full-window flush branch is not covered by the new round-trip tests. With the default 4096-byte buffer this requires more than 1024 offsets, while all object/list/map round trips still use 133 rows; the 5000-row test only exercises primitive payload writers. Please add a >1024-row variable-binary or list/map round trip so both this flush and the corresponding multi-window readIntBuffer path are verified.
            if (bufferPos == buffer.length) {
                flush();

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.

2 participants