HDDS-15592 use heap allocation in ChunkBuffer by yandrey321 · Pull Request #10536 · apache/ozone

yandrey321 · 2026-06-17T15:31:46Z

What changes were proposed in this pull request?

When ChunkBuffer allocates heap buffer UnsafeByteOperations.unsafeWrap() returns a BoundedByteString with a backing array, enabling a single System.arraycopy into the gRPC/Netty wire buffer instead of the slow byte-by-byte NioByteString path for direct buffers.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-15592

How was this patch tested?

Benchmark run on m3 mac/ARM with 14 cores producing the following results: Heap allocation is consistently +8% to +18% faster across all test.

4096 KB writes

threads	direct MB/s	heap MB/s	Δ%
1	5,344	5,929	+10.9%
2	10,112	11,183	+10.6%
4	17,971	19,836	+10.4%
7	24,696	29,157	+18.1%
14	28,025	32,222	+15.0%
28	27,709	31,067	+12.1%
42	27,617	31,235	+13.1%

2048 KB writes

threads	direct MB/s	heap MB/s	Δ%
1	5,144	5,801	+12.8%
2	9,920	11,310	+14.0%
4	18,087	20,006	+10.6%
7	24,876	29,258	+17.6%
14	27,661	31,677	+14.5%
28	27,594	31,170	+13.0%
42	26,703	30,894	+15.7%

1024 KB writes

threads	direct MB/s	heap MB/s	Δ%
1	5,125	5,803	+13.2%
2	10,245	11,269	+10.0%
4	18,160	19,540	+7.6%
7	24,973	29,282	+17.3%
14	27,541	32,346	+17.4%
28	27,222	31,110	+14.3%
42	27,824	31,328	+12.6%

512 KB writes

threads	direct MB/s	heap MB/s	Δ%
1	5,239	5,825	+11.2%
2	10,236	11,338	+10.8%
4	18,378	20,177	+9.8%
7	25,233	29,494	+16.9%
14	28,008	32,575	+16.3%
28	27,846	31,433	+12.9%
42	28,935	31,229	+7.9%

256 KB writes

threads	direct MB/s	heap MB/s	Δ%
1	5,050	5,840	+15.6%
2	10,238	11,375	+11.1%
4	18,100	19,652	+8.6%
7	25,221	29,532	+17.1%
14	28,072	32,755	+16.7%
28	28,564	31,270	+9.5%
42	27,745	31,299	+12.8%

adoroszlai · 2026-06-19T16:42:24Z


+  /**
+   * When true, {@link #allocate} uses direct ByteBuffers instead of heap ByteBuffers.
+   * Set only by {@link BlockOutputStreamWriteBenchmark} to compare allocation strategies


hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/ChunkBuffer.java:36: warning - Tag @link: reference not found: BlockOutputStreamWriteBenchmark

https://github.com/yandrey321/ozone/actions/runs/27780341047/job/82205691733#step:13:2213

adoroszlai · 2026-06-19T16:45:04Z

+  @VisibleForTesting
+  // CHECKSTYLE:OFF VisibilityModifier
+  AtomicBoolean ALLOCATE_DIRECT = new AtomicBoolean(false);
+  // CHECKSTYLE:ON VisibilityModifier


Please:

avoid checkstyle suppression

do not add @VisibleForTesting

I wanted something lightweight for a benchmark that proves performance improvements.

But this is not the benchmark code.

this is a switch that is used in the benchmark to find out the diff between heap and direct allocations

Making it configurable in prod is not a big change: adoroszlai@79cbbf6

Why should it be configurable at all? The purpose of the benchmarks is to produce evidence so we can make a conclusive decision.

The benchmark needs to test various implementations. Adding the benchmark code to the master branch in the main Ozone repo requires that those implementations also exist there. Choosing between the implementations using an internal toggle that even requires CHECKSTYLE:OFF-style hacks is ugly.

Results may depend on hardware/software environment. Providing this simple toggle configuration does not add much complexity, but enables users to test in their own environment if needed.

I would prefer adding performance tests in a separate repo:

various POC implementations can be freely added without affecting production code

JMH can be used without license problems (HDDS-6202)

can test different versions of Ozone

performance tests would not be confused with functional tests (see TestUnhealthyContainersDerbyPerformance)

I endorse the idea of an ozone-dev style public repo for POC, benchmarks, & profiling; it would be particularly great if we could keep reference to the benchmark and profiling results themselves for later development. But if the second repo is also Apache, does that let us off the GPL hook?

But if the second repo is also Apache, does that let us off the GPL hook?

Yes, since we would not be distributing (releasing) code from that repo.

https://www.apache.org/legal/resolved.html

Removed the switch and use heap allocations by default.

errose28 · 2026-06-22T16:52:00Z

cc @rnblough

errose28 · 2026-06-22T17:50:16Z

In this PR the benchmark is a shell script but in #10546 it is a java class. We should be consistent with how we create benchmarks and where we put them.

removed the shell script

errose28 · 2026-06-22T17:51:48Z

+  @VisibleForTesting
+  // CHECKSTYLE:OFF VisibilityModifier
+  AtomicBoolean ALLOCATE_DIRECT = new AtomicBoolean(false);
+  // CHECKSTYLE:ON VisibilityModifier


Why should it be configurable at all? The purpose of the benchmarks is to produce evidence so we can make a conclusive decision.

szetszwo · 2026-07-09T17:33:07Z

@yandrey321 , it seems that the benchmark is a mock but does not involve real network IO. If it is it the case, the improvement probably would become negative when network IO is involved.

szetszwo · 2026-07-09T17:54:12Z

When ChunkBuffer allocates heap buffer UnsafeByteOperations.unsafeWrap() returns a BoundedByteString ...

For direct buffer, UnsafeByteOperations.unsafeWrap returns NioByteString but not BoundedByteString.

Output:

class org.apache.ratis.thirdparty.com.google.protobuf.NioByteString
class com.google.protobuf.NioByteString

  public static void main(String[] args) {
    final ByteBuffer buffer = ByteBuffer.allocateDirect(4);
    buffer.putInt(0);
    System.out.println(UnsafeByteOperations.unsafeWrap(buffer).getClass());
    System.out.println(com.google.protobuf.UnsafeByteOperations.unsafeWrap(buffer).getClass());
  }

yandrey321 · 2026-07-09T18:06:50Z

byte-by-byte NioByteString

Yes, in case of direct memory allocation we re getting NioByteString and JVM use slower memory copy function comparing that the one available for BoundedByteString which is used for heap allocated memory. Benchmark proofs better throughput for heap allocated buffers.

yandrey321 · 2026-07-09T18:10:56Z

@yandrey321 , it seems that the benchmark is a mock but does not involve real network IO. If it is it the case, the improvement probably would become negative when network IO is involved.

The fix aims to decrease CPU consumption on the client side and improve theoretical end-to-end throughput. In case of slow disks the improvement would be less, but faster disks and less latency on the network side would produce better improvement.

Its a small low risk incremental fix. Please also note that the benchmark was run on m1 mac with unified memory architecture, on amd64 the throughput gain should be better.

szetszwo · 2026-07-09T18:47:08Z

... NioByteString and JVM use slower memory copy function ...

... gRPC/Netty wire buffer ...

For NioByteString (i.e. direct buffer), there is no buffer copying to gRPC/Netty wire buffer.

szetszwo · 2026-07-09T18:51:42Z

Performance tip from Google:

If your application creates and discards thousands of buffers per second, use heap buffers. If your architecture relies on steady, high-throughput network frameworks (like Netty) or file channels, use a reusable pool of direct buffers to get the best of both worlds.

yandrey321 · 2026-07-10T14:33:21Z

Performance tip from Google:

If your application creates and discards thousands of buffers per second, use heap buffers. If your architecture relies on steady, high-throughput network frameworks (like Netty) or file channels, use a reusable pool of direct buffers to get the best of both worlds.

@szetszwo the benchmark clearly shows that in Ozone context heap allocation provides better throughput.

yandrey321 · 2026-07-10T16:20:01Z

... NioByteString and JVM use slower memory copy function ...

... gRPC/Netty wire buffer ...

For NioByteString (i.e. direct buffer), there is no buffer copying to gRPC/Netty wire buffer.

Just used async-profiler to check, and it shows a copy does happen for NioByteString — in fact two copies, versus one for the heap-backed BoundedByteString.

Setup: the client write path with mocked container RPCs, where the request is serialized via request.writeTo(...) into a pooled direct Netty ByteBuf — the same OutputStreamEncoder + framing gRPC's MessageFramer uses. Same benchmark run twice, only flipping ChunkBuffer allocation (allocateDirect → NioByteString vs allocateHeap → BoundedByteString). CPU event, 1 MB writes, 2 threads.

Direct / NioByteString.writeTo — 15.7% of total CPU:

CodedOutputStream$OutputStreamEncoder.writeTo
 └─ NioByteString.writeTo
          ├─ slice → slice → <init>×4 → jbyte_disjoint_arraycopy        # copy #1: off-heap → temp heap byte[]
          └─ writeLazy → ByteBufOutputStream.write
                                    → AbstractByteBuf.setBytes → Unsafe.copyMemory   # copy #2: heap → direct wire ByteBuf

Heap / BoundedByteString(LiteralByteString).writeTo — 0.7% of total CPU:

CodedOutputStream$OutputStreamEncoder.writeTo
 └─ LiteralByteString.writeTo → writeLazy → ByteBufOutputStream.write
                                                   → AbstractByteBuf.setBytes → Unsafe.copyMemory   # single copy: backing byte[] → wire ByteBuf

Both paths terminate in setBytes → Unsafe.copyMemory into the Netty wire buffer, so neither is zero-copy here. The difference is that OutputStreamEncoder cannot bulk-transfer an array-less (direct) buffer, so for NioByteString it first stages the off-heap bytes into a temporary heap buffer (slice/arraycopy) and only then copies into the pooled direct ByteBuf. That extra staging copy is why the NioByteString serialization path costs ~22× more CPU (15.7% vs 0.7% of total), which lines up with the ~11% end-to-end throughput difference I measured (10,409 vs 11,637 MB/s).

So the direct-buffer path isn't avoiding the wire-buffer copy — it's adding one.

ss77892 · 2026-07-10T17:34:15Z

I see a significant performance degradation on the client side. The scenario is with MinIO Warp running with 10 concurrent threads through the S3 gateway:
before patch:
warp_benchmark_report_20260709_165607.txt: * Average: 458.75 MiB/s, 4.59 obj/s
warp_benchmark_report_20260709_174454.txt: * Average: 469.64 MiB/s, 4.70 obj/s
warp_benchmark_report_20260709_185000.txt: * Average: 458.72 MiB/s, 4.59 obj/s
warp_benchmark_report_20260709_190148.txt: * Average: 465.89 MiB/s, 4.66 obj/s

after patch:
warp_benchmark_report_20260709_200025.txt: * Average: 389.08 MiB/s, 3.89 obj/s
warp_benchmark_report_20260709_201203.txt: * Average: 398.94 MiB/s, 3.99 obj/s
warp_benchmark_report_20260710_065601.txt: * Average: 407.81 MiB/s, 4.08 obj/s
warp_benchmark_report_20260710_070741.txt: * Average: 409.70 MiB/s, 4.10 obj/s

Definitely, the assumption that the patch should increase the performance is wrong. It reintroduces the client heap-pressure problem that HDDS-9843 specifically moved these buffers away from.

yandrey321 · 2026-07-10T17:49:50Z

I see a significant performance degradation on the client side. The scenario is with MinIO Warp running with 10 concurrent threads through the S3 gateway: before patch: warp_benchmark_report_20260709_165607.txt: * Average: 458.75 MiB/s, 4.59 obj/s warp_benchmark_report_20260709_174454.txt: * Average: 469.64 MiB/s, 4.70 obj/s warp_benchmark_report_20260709_185000.txt: * Average: 458.72 MiB/s, 4.59 obj/s warp_benchmark_report_20260709_190148.txt: * Average: 465.89 MiB/s, 4.66 obj/s

after patch: warp_benchmark_report_20260709_200025.txt: * Average: 389.08 MiB/s, 3.89 obj/s warp_benchmark_report_20260709_201203.txt: * Average: 398.94 MiB/s, 3.99 obj/s warp_benchmark_report_20260710_065601.txt: * Average: 407.81 MiB/s, 4.08 obj/s warp_benchmark_report_20260710_070741.txt: * Average: 409.70 MiB/s, 4.10 obj/s

Definitely, the assumption that the patch should increase the performance is wrong. It reintroduces the client heap-pressure problem that HDDS-9843 specifically moved these buffers away from.

@ss77892 what params did you use for warp test? Let me check if there is way to address this.

ss77892 · 2026-07-10T17:55:06Z

WARP_REGION="us-east-1"
WARP_ACCESS_KEY="minioadmin"
WARP_SECRET_KEY="minioadmin"
WARP_OBJECTS=3000
WARP_OBJ_SIZES="100mib"
WARP_CONCURRENT=10
WARP_DURATION="420s"
warp --no-color put --access-key "${WARP_ACCESS_KEY}" --secret-key "${WARP_SECRET_KEY}" --lookup path --host "${WARP_HOST}" --region "${WARP_REGION}" --obj.size "${WARP_OBJ_SIZES}" --concurrent "${WARP_CONCURRENT}" --duration "${WARP_DURATION}"

szetszwo · 2026-07-10T18:11:27Z

Setup: the client write path with mocked container RPCs, ...

@yandrey321 , Were you using Netty/gRPC or just a Mock?

yandrey321 · 2026-07-10T18:15:34Z

Setup: the client write path with mocked container RPCs, ...

@yandrey321 , Were you using Netty/gRPC or just a Mock?

the mock is for network communication, so no Netty, but protobuf serialization is not mocked. I saw the similar async profiler call stacks for freon testing in a sense of overhead of NioByteString to protobuf copy

szetszwo · 2026-07-10T18:38:42Z

the mock is for network communication, so no Netty, but protobuf serialization is not mocked. I saw the similar async profiler call stacks for freon testing in a sense of overhead of NioByteString to protobuf copy

Please try with netty. Testing with a mock cannot see the real thing.

As @ss77892 mentioned, there is a significant performance degradation when using heap buffers.

yandrey321 · 2026-07-10T19:09:28Z

LiteralByteString

I can reproduce Sergey's findings with variation of the existing mock by introducing the back pressure and reducing the heap size for benchmark, when I do that I see ~30% of GC time increase and more objects retained in the heap. It seems like the best fix for this issue is to use Ratis data streaming.

yandrey321 added 2 commits June 17, 2026 11:23

HDDS-15592 use heap allocation in ChunkBuffer

5e8e7a5

fixed checkstyle issues

6076524

adoroszlai reviewed Jun 19, 2026

View reviewed changes

errose28 reviewed Jun 22, 2026

View reviewed changes

Addressed review comments

17f2f4f

yandrey321 requested review from adoroszlai and errose28 June 24, 2026 02:35

removed the shell script for running benchmark

411de3b

added an option to add backpressure in the benchmark

23d52c2

Uh oh!

Conversation

yandrey321 commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

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!

errose28 commented Jun 22, 2026

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!

szetszwo commented Jul 9, 2026

Uh oh!

szetszwo commented Jul 9, 2026

Uh oh!

yandrey321 commented Jul 9, 2026

Uh oh!

yandrey321 commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

szetszwo commented Jul 9, 2026

Uh oh!

szetszwo commented Jul 9, 2026

Uh oh!

yandrey321 commented Jul 10, 2026

Uh oh!

yandrey321 commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ss77892 commented Jul 10, 2026

Uh oh!

yandrey321 commented Jul 10, 2026

Uh oh!

ss77892 commented Jul 10, 2026

Uh oh!

szetszwo commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yandrey321 commented Jul 10, 2026

Uh oh!

szetszwo commented Jul 10, 2026

Uh oh!

yandrey321 commented Jul 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

yandrey321 commented Jun 17, 2026 •

edited

Loading

yandrey321 commented Jul 9, 2026 •

edited

Loading

yandrey321 commented Jul 10, 2026 •

edited

Loading

szetszwo commented Jul 10, 2026 •

edited

Loading