You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "yandrey321 (via GitHub)" <gi...@apache.org> on 2026/06/04 22:07:23 UTC
[PR] HDDS-15485 Avoid ByteBuffer.wrap on ChunkBuffer.put(byte[]) path [ozone]
yandrey321 opened a new pull request, #10438:
URL: https://github.com/apache/ozone/pull/10438
## What changes were proposed in this pull request?
Avoid ByteBuffer.wrap on ChunkBuffer.put(byte[]) path
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-15485
## How was this patch tested?
ozone freon, existing unit tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15485. Avoid ByteBuffer.wrap on ChunkBuffer.put(byte[]) path [ozone]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on PR #10438:
URL: https://github.com/apache/ozone/pull/10438#issuecomment-4675217013
Actually, the current runTestImpl(..) already testing put(byte[]). Before this change, it is actually testing put(ByteBuffer) since it use ByteBuffer.wrap(..). So, we need new tests for put(ByteBuffer).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15485. Avoid ByteBuffer.wrap on ChunkBuffer.put(byte[]) path [ozone]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #10438: HDDS-15485. Avoid ByteBuffer.wrap on ChunkBuffer.put(byte[]) path
URL: https://github.com/apache/ozone/pull/10438
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15485 Avoid ByteBuffer.wrap on ChunkBuffer.put(byte[]) path [ozone]
Posted by "yandrey321 (via GitHub)" <gi...@apache.org>.
yandrey321 commented on code in PR #10438:
URL: https://github.com/apache/ozone/pull/10438#discussion_r3383454418
##########
hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/common/ChunkBufferPutBenchmark.java:
##########
@@ -0,0 +1,346 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.common;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.text.DecimalFormat;
+import java.util.concurrent.ThreadLocalRandom;
+import org.apache.hadoop.ozone.common.JfrByteBufferAllocations.AllocationStats;
+
+/**
+ * Microbenchmark for ChunkBuffer.put(byte[]) direct copy vs ByteBuffer.wrap path.
+ *
+ * <p>Focused on the scenarios where HDDS-15485 shows the clearest benefit:
+ * <ul>
+ * <li>Throughput: 4KB stream fill with incremental buffer (64KB increment)</li>
+ * <li>Allocations: same 4KB / 64KB-increment incremental buffer path (JFR + wrap calls)</li>
+ * </ul>
+ *
+ * <p>Run from the repo root:
+ * <pre>
+ * mvn -pl hadoop-hdds/common -q test-compile exec:java \
+ * -Dexec.mainClass=org.apache.hadoop.ozone.common.ChunkBufferPutBenchmark \
+ * -Dexec.classpathScope=test \
+ * -Dexec.args="--add-opens jdk.jfr/jdk.jfr=ALL-UNNAMED --add-opens jdk.jfr/jdk.jfr.consumer=ALL-UNNAMED"
+ * </pre>
+ * JFR ByteBuffer counts are sampled; put-op count reports exact wrap calls.
+ * Wrap-path timings use a blackhole on each {@code ByteBuffer.wrap} so the JVM
+ * cannot eliminate short-lived wrapper allocations via escape analysis.
+ */
+public final class ChunkBufferPutBenchmark {
+
+ /** Prevents escape analysis from removing ByteBuffer.wrap allocations in the wrap path. */
+ private static volatile Object blackhole;
Review Comment:
@szetszwo I removed blackhole and rerun the test, I still see the same improvement:
```
mvn -pl hadoop-hdds/common -q test-compile exec:java \
-Dexec.mainClass=org.apache.hadoop.ozone.common.ChunkBufferPutBenchmark \
-Dexec.classpathScope=test \
-Dexec.args="--add-opens jdk.jfr/jdk.jfr=ALL-UNNAMED --add-opens jdk.jfr/jdk.jfr.consumer=ALL-UNNAMED"
ChunkBuffer.put(byte[]) microbenchmark (pre-allocated buffer, put-only)
JVM: 17 on aarch64
=== Throughput showcase ===
--- Incremental buffer showcase ---
Config: ozone.client.stream.buffer.size=4MB, ozone.client.stream.buffer.increment=64KB, io.file.buffer.size=4KB
Pattern: 4KB stream fill into IncrementalChunkBuffer (64KB steps)
Chunk=4096KB increment=64KB write=4KB
round 1:
direct put(byte[]): 42,262.7 MB/s | 92 ns/op | 20.00s | 216,385,536 ops
wrap put(ByteBuffer): 38,938.0 MB/s | 100 ns/op | 20.00s | 199,363,584 ops
improvement: 8.5% faster (1.09x) per 4KB write; throughput 8.5% (1.09x)
round 2:
direct put(byte[]): 41,901.0 MB/s | 93 ns/op | 20.00s | 214,533,120 ops
wrap put(ByteBuffer): 38,885.1 MB/s | 100 ns/op | 20.00s | 199,092,224 ops
improvement: 7.8% faster (1.08x) per 4KB write; throughput 7.8% (1.08x)
round 3:
direct put(byte[]): 41,889.6 MB/s | 93 ns/op | 20.00s | 214,474,752 ops
wrap put(ByteBuffer): 38,947.6 MB/s | 100 ns/op | 20.00s | 199,412,736 ops
improvement: 7.6% faster (1.08x) per 4KB write; throughput 7.6% (1.08x)
median improvement over 3 rounds: 7.8%
=== Allocation showcase ===
--- Incremental buffer showcase ---
Config: ozone.client.stream.buffer.size=4MB, ozone.client.stream.buffer.increment=64KB, io.file.buffer.size=4KB
Pattern: 4KB stream fill into IncrementalChunkBuffer (64KB steps)
Chunk=4096KB increment=64KB write=4KB
direct put(byte[]): 53,298,176 put ops | 0 ByteBuffer TLAB allocs | 0 alloc bytes
wrap put(ByteBuffer): 48,955,392 put ops | 724 ByteBuffer TLAB allocs | 40,544 alloc bytes
ByteBuffer.wrap calls on wrap path (1 per put): 48,955,392
direct path avoids 48,955,392 ByteBuffer.wrap calls per run
JFR confirms zero ByteBuffer TLAB allocations on direct path
JFR sampled ByteBuffer TLAB allocations avoided on direct path: 724
(JFR samples TLAB events; put-op count is the exact wrap-call metric)
```
Removing `-Dexec.args="--add-opens jdk.jfr/jdk.jfr=ALL-UNNAMED --add-opens jdk.jfr/jdk.jfr.consumer=ALL-UNNAMED"` param doubles the numbers:
```
mvn -pl hadoop-hdds/common -q test-compile exec:java \
-Dexec.mainClass=org.apache.hadoop.ozone.common.ChunkBufferPutBenchmark \
-Dexec.classpathScope=test
ChunkBuffer.put(byte[]) microbenchmark (pre-allocated buffer, put-only)
JVM: 17 on aarch64
=== Throughput showcase ===
--- Incremental buffer showcase ---
Config: ozone.client.stream.buffer.size=4MB, ozone.client.stream.buffer.increment=64KB, io.file.buffer.size=4KB
Pattern: 4KB stream fill into IncrementalChunkBuffer (64KB steps)
Chunk=4096KB increment=64KB write=4KB
round 1:
direct put(byte[]): 42,609.4 MB/s | 92 ns/op | 20.00s | 218,161,152 ops
wrap put(ByteBuffer): 36,828.7 MB/s | 106 ns/op | 20.00s | 188,563,456 ops
improvement: 15.7% faster (1.16x) per 4KB write; throughput 15.7% (1.16x)
round 2:
direct put(byte[]): 42,486.2 MB/s | 92 ns/op | 20.00s | 217,530,368 ops
wrap put(ByteBuffer): 37,137.1 MB/s | 105 ns/op | 20.00s | 190,142,464 ops
improvement: 14.4% faster (1.14x) per 4KB write; throughput 14.4% (1.14x)
round 3:
direct put(byte[]): 42,399.2 MB/s | 92 ns/op | 20.00s | 217,084,928 ops
wrap put(ByteBuffer): 36,557.4 MB/s | 107 ns/op | 20.00s | 187,174,912 ops
improvement: 16.0% faster (1.16x) per 4KB write; throughput 16.0% (1.16x)
median improvement over 3 rounds: 15.7%
=== Allocation showcase ===
--- Incremental buffer showcase ---
Config: ozone.client.stream.buffer.size=4MB, ozone.client.stream.buffer.increment=64KB, io.file.buffer.size=4KB
Pattern: 4KB stream fill into IncrementalChunkBuffer (64KB steps)
Chunk=4096KB increment=64KB write=4KB
direct put(byte[]): 50,421,760 put ops | 0 ByteBuffer TLAB allocs | 0 alloc bytes
wrap put(ByteBuffer): 48,524,288 put ops | 714 ByteBuffer TLAB allocs | 39,984 alloc bytes
ByteBuffer.wrap calls on wrap path (1 per put): 48,524,288
direct path avoids 48,524,288 ByteBuffer.wrap calls per run
JFR confirms zero ByteBuffer TLAB allocations on direct path
JFR sampled ByteBuffer TLAB allocations avoided on direct path: 714
(JFR samples TLAB events; put-op count is the exact wrap-call metric)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15485. Avoid ByteBuffer.wrap on ChunkBuffer.put(byte[]) path [ozone]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on PR #10438:
URL: https://github.com/apache/ozone/pull/10438#issuecomment-4683812170
> So, we need new tests for put(ByteBuffer).
> the idea of the benchmark ...
@yandrey321 , I mean we need unit tests for asserting correctness but not for performance.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15485. Avoid ByteBuffer.wrap on ChunkBuffer.put(byte[]) path [ozone]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #10438:
URL: https://github.com/apache/ozone/pull/10438#issuecomment-4879987827
This PR has been marked as stale due to 21 days of inactivity. Please comment or remove the stale label to keep it open. Otherwise, it will be automatically closed in 7 days.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15485 Avoid ByteBuffer.wrap on ChunkBuffer.put(byte[]) path [ozone]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #10438:
URL: https://github.com/apache/ozone/pull/10438#discussion_r3383215308
##########
hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/common/ChunkBufferPutBenchmark.java:
##########
@@ -0,0 +1,346 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.common;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.text.DecimalFormat;
+import java.util.concurrent.ThreadLocalRandom;
+import org.apache.hadoop.ozone.common.JfrByteBufferAllocations.AllocationStats;
+
+/**
+ * Microbenchmark for ChunkBuffer.put(byte[]) direct copy vs ByteBuffer.wrap path.
+ *
+ * <p>Focused on the scenarios where HDDS-15485 shows the clearest benefit:
+ * <ul>
+ * <li>Throughput: 4KB stream fill with incremental buffer (64KB increment)</li>
+ * <li>Allocations: same 4KB / 64KB-increment incremental buffer path (JFR + wrap calls)</li>
+ * </ul>
+ *
+ * <p>Run from the repo root:
+ * <pre>
+ * mvn -pl hadoop-hdds/common -q test-compile exec:java \
+ * -Dexec.mainClass=org.apache.hadoop.ozone.common.ChunkBufferPutBenchmark \
+ * -Dexec.classpathScope=test \
+ * -Dexec.args="--add-opens jdk.jfr/jdk.jfr=ALL-UNNAMED --add-opens jdk.jfr/jdk.jfr.consumer=ALL-UNNAMED"
+ * </pre>
+ * JFR ByteBuffer counts are sampled; put-op count reports exact wrap calls.
+ * Wrap-path timings use a blackhole on each {@code ByteBuffer.wrap} so the JVM
+ * cannot eliminate short-lived wrapper allocations via escape analysis.
+ */
+public final class ChunkBufferPutBenchmark {
+
+ /** Prevents escape analysis from removing ByteBuffer.wrap allocations in the wrap path. */
+ private static volatile Object blackhole;
Review Comment:
@yandrey321 , I am really appreciated you working the improvement. As you already known, Java has [Escape Analysis](https://medium.com/@houssyahya/escape-analysis-in-java-04039ee1dc8b) so the ByteBuffer objects actually are not allocated. Thus, there is no performance improvement here. I guess we should not add more code to avoid more bugs.
Below is the result obtained by removing the blackhole and changed THROUGHPUT_ROUNDS to 4. Got two positive and two negative results:
```
ChunkBuffer.put(byte[]) microbenchmark (pre-allocated buffer, put-only)
JVM: 17.0.19 on aarch64
=== Throughput showcase ===
--- Incremental buffer showcase ---
Config: ozone.client.stream.buffer.size=4MB, ozone.client.stream.buffer.increment=64KB, io.file.buffer.size=4KB
Pattern: 4KB stream fill into IncrementalChunkBuffer (64KB steps)
Chunk=4096KB increment=64KB write=4KB
(wrap path blackholes each ByteBuffer.wrap to model real allocation cost)
round 1:
direct put(byte[]): 37,761.8 MB/s | 103 ns/op | 20.00s | 193,341,440 ops
wrap put(ByteBuffer): 39,230.7 MB/s | 100 ns/op | 20.00s | 200,861,696 ops
improvement: -3.7% faster (0.96x) per 4KB write; throughput -3.7% (0.96x)
round 2:
direct put(byte[]): 38,976.4 MB/s | 100 ns/op | 20.00s | 199,559,168 ops
wrap put(ByteBuffer): 38,602.3 MB/s | 101 ns/op | 20.00s | 197,644,288 ops
improvement: 1.0% faster (1.01x) per 4KB write; throughput 1.0% (1.01x)
round 3:
direct put(byte[]): 38,392.9 MB/s | 102 ns/op | 20.00s | 196,572,160 ops
wrap put(ByteBuffer): 39,236.6 MB/s | 100 ns/op | 20.00s | 200,891,392 ops
improvement: -2.2% faster (0.98x) per 4KB write; throughput -2.2% (0.98x)
round 4:
direct put(byte[]): 39,011.6 MB/s | 100 ns/op | 20.00s | 199,740,416 ops
wrap put(ByteBuffer): 38,522.0 MB/s | 101 ns/op | 20.00s | 197,232,640 ops
improvement: 1.3% faster (1.01x) per 4KB write; throughput 1.3% (1.01x)
median improvement over 4 rounds: 1.0%
=== Allocation showcase ===
--- Incremental buffer showcase ---
Config: ozone.client.stream.buffer.size=4MB, ozone.client.stream.buffer.increment=64KB, io.file.buffer.size=4KB
Pattern: 4KB stream fill into IncrementalChunkBuffer (64KB steps)
Chunk=4096KB increment=64KB write=4KB
direct put(byte[]): 46,555,136 put ops | 0 ByteBuffer TLAB allocs | 0 alloc bytes
wrap put(ByteBuffer): 48,808,960 put ops | 900 ByteBuffer TLAB allocs | 50,400 alloc bytes
ByteBuffer.wrap calls on wrap path (1 per put): 48,808,960
direct path avoids 48,808,960 ByteBuffer.wrap calls per run
JFR confirms zero ByteBuffer TLAB allocations on direct path
JFR sampled ByteBuffer TLAB allocations avoided on direct path: 900
(JFR samples TLAB events; put-op count is the exact wrap-call metric)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15485 Avoid ByteBuffer.wrap on ChunkBuffer.put(byte[]) path [ozone]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #10438:
URL: https://github.com/apache/ozone/pull/10438#discussion_r3383994448
##########
hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/common/ChunkBufferPutBenchmark.java:
##########
@@ -0,0 +1,338 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.common;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.text.DecimalFormat;
+import java.util.concurrent.ThreadLocalRandom;
+import org.apache.hadoop.ozone.common.JfrByteBufferAllocations.AllocationStats;
+
+/**
+ * Microbenchmark for ChunkBuffer.put(byte[]) direct copy vs ByteBuffer.wrap path.
+ *
+ * <p>Focused on the scenarios where HDDS-15485 shows the clearest benefit:
+ * <ul>
+ * <li>Throughput: 4KB stream fill with incremental buffer (64KB increment)</li>
+ * <li>Allocations: same 4KB / 64KB-increment incremental buffer path (JFR + wrap calls)</li>
+ * </ul>
+ *
+ * <p>Run from the repo root:
+ * <pre>
+ * mvn -pl hadoop-hdds/common -q test-compile exec:java \
+ * -Dexec.mainClass=org.apache.hadoop.ozone.common.ChunkBufferPutBenchmark \
+ * -Dexec.classpathScope=test \
+ * -Dexec.args="--add-opens jdk.jfr/jdk.jfr=ALL-UNNAMED --add-opens jdk.jfr/jdk.jfr.consumer=ALL-UNNAMED"
+ * </pre>
+ * JFR ByteBuffer counts are sampled; put-op count reports exact wrap calls.
+ */
+public final class ChunkBufferPutBenchmark {
+
+ private static final int WARMUP_SECONDS = 10;
+ private static final int BENCHMARK_SECONDS = 20;
+ private static final int ALLOCATION_BENCHMARK_SECONDS = 5;
+ private static final int THROUGHPUT_ROUNDS = 3;
+ private static final DecimalFormat MBPS = new DecimalFormat("#,##0.0");
+ private static final DecimalFormat NS = new DecimalFormat("#,##0");
+ private static final DecimalFormat PCT = new DecimalFormat("#,##0.0");
+ private static final DecimalFormat RATIO = new DecimalFormat("0.00");
+ private static final DecimalFormat COUNT = new DecimalFormat("#,##0");
+
+ /** ozone.client.stream.buffer.size default. */
+ private static final int DEFAULT_CHUNK_SIZE = 4 * 1024 * 1024;
+
+ /** Showcase: ozone.client.stream.buffer.increment for incremental buffer. */
+ private static final int INCREMENTAL_BUFFER_INCREMENT = 64 * 1024;
+
+ /** Hadoop io.file.buffer.size / FSDataOutputStream default. */
+ private static final int HADOOP_FS_BUFFER_SIZE = 4 * 1024;
+
+ private static final ThreadLocal<byte[]> SOURCE = ThreadLocal.withInitial(() -> {
+ byte[] bytes = new byte[DEFAULT_CHUNK_SIZE];
+ ThreadLocalRandom.current().nextBytes(bytes);
+ return bytes;
+ });
+
+ private ChunkBufferPutBenchmark() {
+ }
+
+ public static void main(String[] args) throws IOException {
+ System.out.println("ChunkBuffer.put(byte[]) microbenchmark (pre-allocated buffer, put-only)");
+ System.out.println("JVM: " + System.getProperty("java.version")
+ + " on " + System.getProperty("os.arch"));
+ System.out.println();
+
+ final Scenario showcase = new Scenario(
+ "Incremental buffer showcase",
+ "ozone.client.stream.buffer.size=4MB, "
+ + "ozone.client.stream.buffer.increment=64KB, io.file.buffer.size=4KB",
+ "4KB stream fill into IncrementalChunkBuffer (64KB steps)",
Review Comment:
Do not hard code config and pattern. It should be built using other parmeters.
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/ChunkBufferImplWithByteBufferList.java:
##########
@@ -157,6 +157,39 @@ public ChunkBuffer put(ByteBuffer that) {
return this;
}
+ @Override
+ public ChunkBuffer put(byte[] b, int offset, int length) {
+ Objects.requireNonNull(b, "b == null");
+ Preconditions.checkArgument(offset >= 0 && length >= 0,
+ "offset = %s, length = %s", offset, length);
+ Preconditions.checkArgument(length <= b.length - offset,
+ "length = %s out of range for array.length = %s, offset = %s",
+ length, b.length, offset);
+ if (length > remaining()) {
+ final BufferOverflowException boe = new BufferOverflowException();
+ boe.initCause(new IllegalArgumentException(
+ "Failed to put since length = " + length
+ + " > this.remaining() = " + remaining()));
+ throw boe;
+ }
Review Comment:
Add a checkArgument(..) method:
```diff
diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/ChunkBuffer.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/ChunkBuffer.java
index 600a7b5679..ab36cd636d 100644
--- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/ChunkBuffer.java
+++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/ChunkBuffer.java
@@ -18,10 +18,13 @@
package org.apache.hadoop.ozone.common;
import java.io.IOException;
+import java.nio.BufferOverflowException;
import java.nio.ByteBuffer;
import java.nio.channels.GatheringByteChannel;
import java.util.List;
import java.util.Objects;
+
+import com.google.common.base.Preconditions;
import org.apache.hadoop.hdds.utils.db.CodecBuffer;
import org.apache.ratis.thirdparty.com.google.protobuf.ByteString;
import org.apache.ratis.util.UncheckedAutoCloseable;
@@ -64,6 +67,22 @@ static ChunkBuffer wrap(List<ByteBuffer> buffers) {
return new ChunkBufferImplWithByteBufferList(buffers);
}
+ default void checkArgument(byte[] b, int offset, int length) {
+ Objects.requireNonNull(b, "b == null");
+ Preconditions.checkArgument(offset >= 0 && length >= 0,
+ "offset = %s, length = %s", offset, length);
+ Preconditions.checkArgument(length <= b.length - offset,
+ "length = %s out of range for array.length = %s, offset = %s",
+ length, b.length, offset);
+ if (length > remaining()) {
+ final BufferOverflowException boe = new BufferOverflowException();
+ boe.initCause(new IllegalArgumentException(
+ "Failed to put since length = " + length
+ + " > this.remaining() = " + remaining()));
+ throw boe;
+ }
+ }
+
/** Similar to {@link ByteBuffer#position()}. */
int position();
diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/ChunkBufferImplWithByteBuffer.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/ChunkBufferImplWithByteBuffer.java
index 5fe6cd7b1a..5bc9300e90 100644
--- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/ChunkBufferImplWithByteBuffer.java
+++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/ChunkBufferImplWithByteBuffer.java
@@ -120,6 +120,7 @@ public ChunkBuffer put(ByteBuffer b) {
@Override
public ChunkBuffer put(byte[] b, int offset, int length) {
+ checkArgument(b, offset, length);
buffer.put(b, offset, length);
return this;
}
diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/ChunkBufferImplWithByteBufferList.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/ChunkBufferImplWithByteBufferList.java
index d9f8e41a51..c4c8c23cf3 100644
--- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/ChunkBufferImplWithByteBufferList.java
+++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/ChunkBufferImplWithByteBufferList.java
@@ -159,19 +159,7 @@ public ChunkBuffer put(ByteBuffer that) {
@Override
public ChunkBuffer put(byte[] b, int offset, int length) {
- Objects.requireNonNull(b, "b == null");
- Preconditions.checkArgument(offset >= 0 && length >= 0,
- "offset = %s, length = %s", offset, length);
- Preconditions.checkArgument(length <= b.length - offset,
- "length = %s out of range for array.length = %s, offset = %s",
- length, b.length, offset);
- if (length > remaining()) {
- final BufferOverflowException boe = new BufferOverflowException();
- boe.initCause(new IllegalArgumentException(
- "Failed to put since length = " + length
- + " > this.remaining() = " + remaining()));
- throw boe;
- }
+ checkArgument(b, offset, length);
final int end = offset + length;
int off = offset;
diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/IncrementalChunkBuffer.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/IncrementalChunkBuffer.java
index 39d6281373..8e9b982aff 100644
--- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/IncrementalChunkBuffer.java
+++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/IncrementalChunkBuffer.java
@@ -233,19 +233,7 @@ public ChunkBuffer put(ByteBuffer that) {
@Override
public ChunkBuffer put(byte[] b, int offset, int length) {
- Objects.requireNonNull(b, "b == null");
- Preconditions.checkArgument(offset >= 0 && length >= 0,
- "offset = %s, length = %s", offset, length);
- Preconditions.checkArgument(length <= b.length - offset,
- "length = %s out of range for array.length = %s, offset = %s",
- length, b.length, offset);
- if (length > remaining()) {
- final BufferOverflowException boe = new BufferOverflowException();
- boe.initCause(new IllegalArgumentException(
- "Failed to put since length = " + length
- + " > this.remaining() = " + remaining()));
- throw boe;
- }
+ checkArgument(b, offset, length);
final int end = offset + length;
for (int p = position(); offset < end; ) {
```
##########
hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/common/ChunkBufferPutBenchmark.java:
##########
@@ -0,0 +1,338 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.common;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.text.DecimalFormat;
+import java.util.concurrent.ThreadLocalRandom;
+import org.apache.hadoop.ozone.common.JfrByteBufferAllocations.AllocationStats;
+
+/**
+ * Microbenchmark for ChunkBuffer.put(byte[]) direct copy vs ByteBuffer.wrap path.
+ *
+ * <p>Focused on the scenarios where HDDS-15485 shows the clearest benefit:
+ * <ul>
+ * <li>Throughput: 4KB stream fill with incremental buffer (64KB increment)</li>
+ * <li>Allocations: same 4KB / 64KB-increment incremental buffer path (JFR + wrap calls)</li>
+ * </ul>
+ *
+ * <p>Run from the repo root:
+ * <pre>
+ * mvn -pl hadoop-hdds/common -q test-compile exec:java \
+ * -Dexec.mainClass=org.apache.hadoop.ozone.common.ChunkBufferPutBenchmark \
+ * -Dexec.classpathScope=test \
+ * -Dexec.args="--add-opens jdk.jfr/jdk.jfr=ALL-UNNAMED --add-opens jdk.jfr/jdk.jfr.consumer=ALL-UNNAMED"
+ * </pre>
+ * JFR ByteBuffer counts are sampled; put-op count reports exact wrap calls.
+ */
+public final class ChunkBufferPutBenchmark {
+
+ private static final int WARMUP_SECONDS = 10;
+ private static final int BENCHMARK_SECONDS = 20;
+ private static final int ALLOCATION_BENCHMARK_SECONDS = 5;
+ private static final int THROUGHPUT_ROUNDS = 3;
+ private static final DecimalFormat MBPS = new DecimalFormat("#,##0.0");
+ private static final DecimalFormat NS = new DecimalFormat("#,##0");
+ private static final DecimalFormat PCT = new DecimalFormat("#,##0.0");
+ private static final DecimalFormat RATIO = new DecimalFormat("0.00");
+ private static final DecimalFormat COUNT = new DecimalFormat("#,##0");
+
+ /** ozone.client.stream.buffer.size default. */
+ private static final int DEFAULT_CHUNK_SIZE = 4 * 1024 * 1024;
+
+ /** Showcase: ozone.client.stream.buffer.increment for incremental buffer. */
+ private static final int INCREMENTAL_BUFFER_INCREMENT = 64 * 1024;
+
+ /** Hadoop io.file.buffer.size / FSDataOutputStream default. */
+ private static final int HADOOP_FS_BUFFER_SIZE = 4 * 1024;
+
+ private static final ThreadLocal<byte[]> SOURCE = ThreadLocal.withInitial(() -> {
+ byte[] bytes = new byte[DEFAULT_CHUNK_SIZE];
+ ThreadLocalRandom.current().nextBytes(bytes);
+ return bytes;
+ });
+
+ private ChunkBufferPutBenchmark() {
+ }
+
+ public static void main(String[] args) throws IOException {
+ System.out.println("ChunkBuffer.put(byte[]) microbenchmark (pre-allocated buffer, put-only)");
+ System.out.println("JVM: " + System.getProperty("java.version")
+ + " on " + System.getProperty("os.arch"));
+ System.out.println();
+
+ final Scenario showcase = new Scenario(
+ "Incremental buffer showcase",
+ "ozone.client.stream.buffer.size=4MB, "
+ + "ozone.client.stream.buffer.increment=64KB, io.file.buffer.size=4KB",
+ "4KB stream fill into IncrementalChunkBuffer (64KB steps)",
+ DEFAULT_CHUNK_SIZE,
+ INCREMENTAL_BUFFER_INCREMENT,
+ HADOOP_FS_BUFFER_SIZE);
+
+ System.out.println("=== Throughput showcase ===");
+ runThroughputComparison(showcase);
+ System.out.println();
+
+ System.out.println("=== Allocation showcase ===");
+ runAllocationComparison(showcase);
+ }
+
+ private static void runThroughputComparison(Scenario scenario) {
+ printScenarioHeader(scenario);
+ warmupBothPaths(scenario);
+ final double[] improvements = new double[THROUGHPUT_ROUNDS];
+ for (int round = 0; round < THROUGHPUT_ROUNDS; round++) {
+ final boolean directFirst = round % 2 == 0;
+ final Result direct;
+ final Result wrap;
+ if (directFirst) {
+ direct = benchmarkThroughput(scenario, ChunkBufferPutBenchmark::loopStreamFillDirect);
+ wrap = benchmarkThroughput(scenario, ChunkBufferPutBenchmark::loopStreamFillWrap);
+ } else {
+ wrap = benchmarkThroughput(scenario, ChunkBufferPutBenchmark::loopStreamFillWrap);
+ direct = benchmarkThroughput(scenario, ChunkBufferPutBenchmark::loopStreamFillDirect);
+ }
+ improvements[round] = wrap.nsPerOp / direct.nsPerOp - 1.0;
+ System.out.printf(" round %d:%n", round + 1);
+ printThroughputComparison(scenario.writeSize, direct, wrap, " ");
+ }
+ final double medianImprovement = median(improvements) * 100.0;
+ System.out.printf(" median improvement over %d rounds: %s%%%n",
+ THROUGHPUT_ROUNDS, PCT.format(medianImprovement));
+ }
+
+ private static void warmupBothPaths(Scenario scenario) {
+ final byte[] source = SOURCE.get();
+ final int writesPerChunk = scenario.chunkSize / scenario.writeSize;
+ try (ChunkBuffer buffer = ChunkBuffer.allocate(scenario.chunkSize,
+ scenario.bufferIncrement)) {
+ for (int i = 0; i < 2; i++) {
+ loopStreamFillDirect(buffer, source, scenario.writeSize, writesPerChunk,
+ WARMUP_SECONDS);
+ loopStreamFillWrap(buffer, source, scenario.writeSize, writesPerChunk,
+ WARMUP_SECONDS);
+ }
+ }
+ }
+
+ private static double median(double[] values) {
+ final double[] sorted = values.clone();
+ java.util.Arrays.sort(sorted);
+ return sorted[sorted.length / 2];
+ }
+
+ private static void runAllocationComparison(Scenario scenario) throws IOException {
+ printScenarioHeader(scenario);
+ if (!JfrByteBufferAllocations.isAvailable()) {
+ System.out.println(" JFR unavailable; reporting wrap-call count from put ops only.");
+ }
+ final AllocationResult direct = benchmarkAllocations(scenario,
+ ChunkBufferPutBenchmark::loopStreamFillDirect);
+ final AllocationResult wrap = benchmarkAllocations(scenario,
+ ChunkBufferPutBenchmark::loopStreamFillWrap);
+ printAllocationComparison(direct, wrap);
+ }
+
+ private static void printScenarioHeader(Scenario scenario) {
+ System.out.println("--- " + scenario.name + " ---");
+ System.out.println("Config: " + scenario.config);
+ System.out.println("Pattern: " + scenario.pattern);
+ System.out.printf("Chunk=%dKB increment=%dKB write=%dKB%n",
+ scenario.chunkSize / 1024,
+ scenario.bufferIncrement / 1024,
+ scenario.writeSize / 1024);
+ }
+
+ private static void printThroughputComparison(int writeSize, Result direct, Result wrap,
+ String prefix) {
+ final double speedup = wrap.nsPerOp / direct.nsPerOp;
+ final double pctFaster = (speedup - 1.0) * 100.0;
+ final double throughputSpeedup = direct.mbPerSec / wrap.mbPerSec;
+ final double throughputPct = (throughputSpeedup - 1.0) * 100.0;
+ System.out.printf("%sdirect put(byte[]): %s MB/s | %s ns/op | %.2fs | %s ops%n",
+ prefix, MBPS.format(direct.mbPerSec), NS.format(direct.nsPerOp),
+ direct.elapsedSeconds, COUNT.format(direct.ops));
+ System.out.printf("%swrap put(ByteBuffer): %s MB/s | %s ns/op | %.2fs | %s ops%n",
+ prefix, MBPS.format(wrap.mbPerSec), NS.format(wrap.nsPerOp),
+ wrap.elapsedSeconds, COUNT.format(wrap.ops));
+ System.out.printf("%simprovement: %s%% faster (%sx) per %dKB write; "
+ + "throughput %s%% (%sx)%n",
+ prefix, PCT.format(pctFaster), RATIO.format(speedup), writeSize / 1024,
+ PCT.format(throughputPct), RATIO.format(throughputSpeedup));
+ }
+
+ private static void printAllocationComparison(AllocationResult direct,
+ AllocationResult wrap) {
+ System.out.printf(" direct put(byte[]): %s put ops | %s ByteBuffer TLAB allocs | %s alloc bytes%n",
+ COUNT.format(direct.putOps), COUNT.format(direct.byteBufferAllocCount),
+ COUNT.format(direct.byteBufferAllocBytes));
+ System.out.printf(" wrap put(ByteBuffer): %s put ops | %s ByteBuffer TLAB allocs | %s alloc bytes%n",
+ COUNT.format(wrap.putOps), COUNT.format(wrap.byteBufferAllocCount),
+ COUNT.format(wrap.byteBufferAllocBytes));
+ System.out.printf(" ByteBuffer.wrap calls on wrap path (1 per put): %s%n",
+ COUNT.format(wrap.putOps));
+ System.out.printf(" direct path avoids %s ByteBuffer.wrap calls per run%n",
+ COUNT.format(wrap.putOps));
+ if (wrap.byteBufferAllocCount > 0 && direct.byteBufferAllocCount == 0) {
+ System.out.printf(" JFR confirms zero ByteBuffer TLAB allocations on direct path%n");
+ }
+ if (wrap.byteBufferAllocCount > direct.byteBufferAllocCount) {
+ final long saved = wrap.byteBufferAllocCount - direct.byteBufferAllocCount;
+ System.out.printf(" JFR sampled ByteBuffer TLAB allocations avoided on direct path: %s%n",
+ COUNT.format(saved));
+ System.out.println(" (JFR samples TLAB events; put-op count is the exact wrap-call metric)");
+ }
+ }
+
+ private static Result benchmarkThroughput(Scenario scenario, TimedLoop loop) {
+ final byte[] source = SOURCE.get();
+ final int writesPerChunk = scenario.chunkSize / scenario.writeSize;
+ try (ChunkBuffer buffer = ChunkBuffer.allocate(scenario.chunkSize,
+ scenario.bufferIncrement)) {
+ loop.run(buffer, source, scenario.writeSize, writesPerChunk, WARMUP_SECONDS);
+ final LoopResult benchmark = loop.run(buffer, source, scenario.writeSize,
+ writesPerChunk, BENCHMARK_SECONDS);
+ return toResult(benchmark, scenario.writeSize);
+ }
+ }
+
+ private static AllocationResult benchmarkAllocations(Scenario scenario, TimedLoop loop)
+ throws IOException {
+ final byte[] source = SOURCE.get();
+ final int writesPerChunk = scenario.chunkSize / scenario.writeSize;
+ try (ChunkBuffer buffer = ChunkBuffer.allocate(scenario.chunkSize,
+ scenario.bufferIncrement)) {
+ loop.run(buffer, source, scenario.writeSize, writesPerChunk, WARMUP_SECONDS);
+ final LoopResult[] benchmark = new LoopResult[1];
+ final AllocationStats stats = JfrByteBufferAllocations.measure(
+ () -> benchmark[0] = loop.run(buffer, source, scenario.writeSize,
+ writesPerChunk, ALLOCATION_BENCHMARK_SECONDS));
+ final long putOps = benchmark[0].totalBytes / scenario.writeSize;
+ return new AllocationResult(putOps, stats.getByteBufferAllocCount(),
+ stats.getByteBufferAllocBytes());
+ }
+ }
+
+ private static Result toResult(LoopResult benchmark, int writeSize) {
+ final long elapsedNanos = benchmark.elapsedNanos;
+ final long benchmarkBytes = benchmark.totalBytes;
+ final double seconds = elapsedNanos / 1_000_000_000.0;
+ final double mbPerSec = benchmarkBytes / seconds / (1024.0 * 1024.0);
+ final long ops = benchmarkBytes / writeSize;
+ final double nsPerOp = (double) elapsedNanos / ops;
+ return new Result(mbPerSec, nsPerOp, seconds, ops);
+ }
+
+ @FunctionalInterface
+ private interface TimedLoop {
+ LoopResult run(ChunkBuffer buffer, byte[] source, int writeSize,
+ int writesPerChunk, int seconds);
Review Comment:
- Add a `verify` parameter and check for correctness.
- Set it to false in benchmark.
- Add unit tests by setting it to true.
```diff
@@ -246,8 +249,9 @@ LoopResult run(ChunkBuffer buffer, byte[] source, int writeSize,
private static LoopResult loopStreamFillDirect(ChunkBuffer buffer, byte[] source,
- int writeSize, int writesPerChunk, int seconds) {
+ int writeSize, int writesPerChunk, int seconds, boolean verify) {
long totalBytes = 0;
final long start = System.nanoTime();
final long deadline = start + seconds * 1_000_000_000L;
@@ -259,10 +263,17 @@ private static LoopResult loopStreamFillDirect(ChunkBuffer buffer, byte[] source
off += writeSize;
totalBytes += writeSize;
}
+ if (verify) {
+ assertBuffer(source, buffer);
+ }
}
return new LoopResult(totalBytes, System.nanoTime() - start);
}
+ static void assertBuffer(byte[] expected, ChunkBuffer computed) {
+ assertEquals(ByteBuffer.wrap(expected), computed.toByteString().asReadOnlyByteBuffer());
+ }
+
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15485 Avoid ByteBuffer.wrap on ChunkBuffer.put(byte[]) path [ozone]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #10438:
URL: https://github.com/apache/ozone/pull/10438#discussion_r3383215308
##########
hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/common/ChunkBufferPutBenchmark.java:
##########
@@ -0,0 +1,346 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.common;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.text.DecimalFormat;
+import java.util.concurrent.ThreadLocalRandom;
+import org.apache.hadoop.ozone.common.JfrByteBufferAllocations.AllocationStats;
+
+/**
+ * Microbenchmark for ChunkBuffer.put(byte[]) direct copy vs ByteBuffer.wrap path.
+ *
+ * <p>Focused on the scenarios where HDDS-15485 shows the clearest benefit:
+ * <ul>
+ * <li>Throughput: 4KB stream fill with incremental buffer (64KB increment)</li>
+ * <li>Allocations: same 4KB / 64KB-increment incremental buffer path (JFR + wrap calls)</li>
+ * </ul>
+ *
+ * <p>Run from the repo root:
+ * <pre>
+ * mvn -pl hadoop-hdds/common -q test-compile exec:java \
+ * -Dexec.mainClass=org.apache.hadoop.ozone.common.ChunkBufferPutBenchmark \
+ * -Dexec.classpathScope=test \
+ * -Dexec.args="--add-opens jdk.jfr/jdk.jfr=ALL-UNNAMED --add-opens jdk.jfr/jdk.jfr.consumer=ALL-UNNAMED"
+ * </pre>
+ * JFR ByteBuffer counts are sampled; put-op count reports exact wrap calls.
+ * Wrap-path timings use a blackhole on each {@code ByteBuffer.wrap} so the JVM
+ * cannot eliminate short-lived wrapper allocations via escape analysis.
+ */
+public final class ChunkBufferPutBenchmark {
+
+ /** Prevents escape analysis from removing ByteBuffer.wrap allocations in the wrap path. */
+ private static volatile Object blackhole;
Review Comment:
@yandrey321 , I really appreciate your effort! I also have learned a lot in reviewing this PR.
As you already known, Java has [Escape Analysis](https://medium.com/@houssyahya/escape-analysis-in-java-04039ee1dc8b) so the ByteBuffer objects actually may not be allocated. Thus, there is no performance improvement here. I guess we should not add more code to avoid more bugs.
Below is the result obtained by removing the blackhole and changed THROUGHPUT_ROUNDS to 4. Got two positive and two negative results:
```
ChunkBuffer.put(byte[]) microbenchmark (pre-allocated buffer, put-only)
JVM: 17.0.19 on aarch64
=== Throughput showcase ===
--- Incremental buffer showcase ---
Config: ozone.client.stream.buffer.size=4MB, ozone.client.stream.buffer.increment=64KB, io.file.buffer.size=4KB
Pattern: 4KB stream fill into IncrementalChunkBuffer (64KB steps)
Chunk=4096KB increment=64KB write=4KB
(wrap path blackholes each ByteBuffer.wrap to model real allocation cost)
round 1:
direct put(byte[]): 37,761.8 MB/s | 103 ns/op | 20.00s | 193,341,440 ops
wrap put(ByteBuffer): 39,230.7 MB/s | 100 ns/op | 20.00s | 200,861,696 ops
improvement: -3.7% faster (0.96x) per 4KB write; throughput -3.7% (0.96x)
round 2:
direct put(byte[]): 38,976.4 MB/s | 100 ns/op | 20.00s | 199,559,168 ops
wrap put(ByteBuffer): 38,602.3 MB/s | 101 ns/op | 20.00s | 197,644,288 ops
improvement: 1.0% faster (1.01x) per 4KB write; throughput 1.0% (1.01x)
round 3:
direct put(byte[]): 38,392.9 MB/s | 102 ns/op | 20.00s | 196,572,160 ops
wrap put(ByteBuffer): 39,236.6 MB/s | 100 ns/op | 20.00s | 200,891,392 ops
improvement: -2.2% faster (0.98x) per 4KB write; throughput -2.2% (0.98x)
round 4:
direct put(byte[]): 39,011.6 MB/s | 100 ns/op | 20.00s | 199,740,416 ops
wrap put(ByteBuffer): 38,522.0 MB/s | 101 ns/op | 20.00s | 197,232,640 ops
improvement: 1.3% faster (1.01x) per 4KB write; throughput 1.3% (1.01x)
median improvement over 4 rounds: 1.0%
=== Allocation showcase ===
--- Incremental buffer showcase ---
Config: ozone.client.stream.buffer.size=4MB, ozone.client.stream.buffer.increment=64KB, io.file.buffer.size=4KB
Pattern: 4KB stream fill into IncrementalChunkBuffer (64KB steps)
Chunk=4096KB increment=64KB write=4KB
direct put(byte[]): 46,555,136 put ops | 0 ByteBuffer TLAB allocs | 0 alloc bytes
wrap put(ByteBuffer): 48,808,960 put ops | 900 ByteBuffer TLAB allocs | 50,400 alloc bytes
ByteBuffer.wrap calls on wrap path (1 per put): 48,808,960
direct path avoids 48,808,960 ByteBuffer.wrap calls per run
JFR confirms zero ByteBuffer TLAB allocations on direct path
JFR sampled ByteBuffer TLAB allocations avoided on direct path: 900
(JFR samples TLAB events; put-op count is the exact wrap-call metric)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15485. Avoid ByteBuffer.wrap on ChunkBuffer.put(byte[]) path [ozone]
Posted by "yandrey321 (via GitHub)" <gi...@apache.org>.
yandrey321 commented on code in PR #10438:
URL: https://github.com/apache/ozone/pull/10438#discussion_r3389535438
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/ChunkBuffer.java:
##########
@@ -64,6 +67,22 @@ static ChunkBuffer wrap(List<ByteBuffer> buffers) {
return new ChunkBufferImplWithByteBufferList(buffers);
}
+ default void checkArgument(byte[] b, int offset, int length) {
+ Objects.requireNonNull(b, "b == null");
+ Preconditions.checkArgument(offset >= 0 && length >= 0,
+ "offset = %s, length = %s", offset, length);
+ Preconditions.checkArgument(length <= b.length - offset,
+ "length = %s out of range for array.length = %s, offset = %s",
+ length, b.length, offset);
+ if (length > remaining()) {
+ final BufferOverflowException boe = new BufferOverflowException();
+ boe.initCause(new IllegalArgumentException(
+ "Failed to put since length = " + length
+ + " > this.remaining() = " + remaining()));
+ throw boe;
+ }
Review Comment:
I added for checkArgument(ByteBuffer) and removed duplicated code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15485 Avoid ByteBuffer.wrap on ChunkBuffer.put(byte[]) path [ozone]
Posted by "amaliujia (via GitHub)" <gi...@apache.org>.
amaliujia commented on PR #10438:
URL: https://github.com/apache/ozone/pull/10438#issuecomment-4629190109
I read the Javadoc and [ByteBuffer.wrap](https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/nio/ByteBuffer.html#wrap(byte%5B%5D)) does not copy the bytes but more like creating a view. I assume this means there is no expensive copy operation.
Is the modification for this PR about performance improvement or something else?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15485 Avoid ByteBuffer.wrap on ChunkBuffer.put(byte[]) path [ozone]
Posted by "yandrey321 (via GitHub)" <gi...@apache.org>.
yandrey321 commented on code in PR #10438:
URL: https://github.com/apache/ozone/pull/10438#discussion_r3384490579
##########
hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/common/ChunkBufferPutBenchmark.java:
##########
@@ -0,0 +1,338 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.common;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.text.DecimalFormat;
+import java.util.concurrent.ThreadLocalRandom;
+import org.apache.hadoop.ozone.common.JfrByteBufferAllocations.AllocationStats;
+
+/**
+ * Microbenchmark for ChunkBuffer.put(byte[]) direct copy vs ByteBuffer.wrap path.
+ *
+ * <p>Focused on the scenarios where HDDS-15485 shows the clearest benefit:
+ * <ul>
+ * <li>Throughput: 4KB stream fill with incremental buffer (64KB increment)</li>
+ * <li>Allocations: same 4KB / 64KB-increment incremental buffer path (JFR + wrap calls)</li>
+ * </ul>
+ *
+ * <p>Run from the repo root:
+ * <pre>
+ * mvn -pl hadoop-hdds/common -q test-compile exec:java \
+ * -Dexec.mainClass=org.apache.hadoop.ozone.common.ChunkBufferPutBenchmark \
+ * -Dexec.classpathScope=test \
+ * -Dexec.args="--add-opens jdk.jfr/jdk.jfr=ALL-UNNAMED --add-opens jdk.jfr/jdk.jfr.consumer=ALL-UNNAMED"
+ * </pre>
+ * JFR ByteBuffer counts are sampled; put-op count reports exact wrap calls.
+ */
+public final class ChunkBufferPutBenchmark {
+
+ private static final int WARMUP_SECONDS = 10;
+ private static final int BENCHMARK_SECONDS = 20;
+ private static final int ALLOCATION_BENCHMARK_SECONDS = 5;
+ private static final int THROUGHPUT_ROUNDS = 3;
+ private static final DecimalFormat MBPS = new DecimalFormat("#,##0.0");
+ private static final DecimalFormat NS = new DecimalFormat("#,##0");
+ private static final DecimalFormat PCT = new DecimalFormat("#,##0.0");
+ private static final DecimalFormat RATIO = new DecimalFormat("0.00");
+ private static final DecimalFormat COUNT = new DecimalFormat("#,##0");
+
+ /** ozone.client.stream.buffer.size default. */
+ private static final int DEFAULT_CHUNK_SIZE = 4 * 1024 * 1024;
+
+ /** Showcase: ozone.client.stream.buffer.increment for incremental buffer. */
+ private static final int INCREMENTAL_BUFFER_INCREMENT = 64 * 1024;
+
+ /** Hadoop io.file.buffer.size / FSDataOutputStream default. */
+ private static final int HADOOP_FS_BUFFER_SIZE = 4 * 1024;
+
+ private static final ThreadLocal<byte[]> SOURCE = ThreadLocal.withInitial(() -> {
+ byte[] bytes = new byte[DEFAULT_CHUNK_SIZE];
+ ThreadLocalRandom.current().nextBytes(bytes);
+ return bytes;
+ });
+
+ private ChunkBufferPutBenchmark() {
+ }
+
+ public static void main(String[] args) throws IOException {
+ System.out.println("ChunkBuffer.put(byte[]) microbenchmark (pre-allocated buffer, put-only)");
+ System.out.println("JVM: " + System.getProperty("java.version")
+ + " on " + System.getProperty("os.arch"));
+ System.out.println();
+
+ final Scenario showcase = new Scenario(
+ "Incremental buffer showcase",
+ "ozone.client.stream.buffer.size=4MB, "
+ + "ozone.client.stream.buffer.increment=64KB, io.file.buffer.size=4KB",
+ "4KB stream fill into IncrementalChunkBuffer (64KB steps)",
+ DEFAULT_CHUNK_SIZE,
+ INCREMENTAL_BUFFER_INCREMENT,
+ HADOOP_FS_BUFFER_SIZE);
+
+ System.out.println("=== Throughput showcase ===");
+ runThroughputComparison(showcase);
+ System.out.println();
+
+ System.out.println("=== Allocation showcase ===");
+ runAllocationComparison(showcase);
+ }
+
+ private static void runThroughputComparison(Scenario scenario) {
+ printScenarioHeader(scenario);
+ warmupBothPaths(scenario);
+ final double[] improvements = new double[THROUGHPUT_ROUNDS];
+ for (int round = 0; round < THROUGHPUT_ROUNDS; round++) {
+ final boolean directFirst = round % 2 == 0;
+ final Result direct;
+ final Result wrap;
+ if (directFirst) {
+ direct = benchmarkThroughput(scenario, ChunkBufferPutBenchmark::loopStreamFillDirect);
+ wrap = benchmarkThroughput(scenario, ChunkBufferPutBenchmark::loopStreamFillWrap);
+ } else {
+ wrap = benchmarkThroughput(scenario, ChunkBufferPutBenchmark::loopStreamFillWrap);
+ direct = benchmarkThroughput(scenario, ChunkBufferPutBenchmark::loopStreamFillDirect);
+ }
+ improvements[round] = wrap.nsPerOp / direct.nsPerOp - 1.0;
+ System.out.printf(" round %d:%n", round + 1);
+ printThroughputComparison(scenario.writeSize, direct, wrap, " ");
+ }
+ final double medianImprovement = median(improvements) * 100.0;
+ System.out.printf(" median improvement over %d rounds: %s%%%n",
+ THROUGHPUT_ROUNDS, PCT.format(medianImprovement));
+ }
+
+ private static void warmupBothPaths(Scenario scenario) {
+ final byte[] source = SOURCE.get();
+ final int writesPerChunk = scenario.chunkSize / scenario.writeSize;
+ try (ChunkBuffer buffer = ChunkBuffer.allocate(scenario.chunkSize,
+ scenario.bufferIncrement)) {
+ for (int i = 0; i < 2; i++) {
+ loopStreamFillDirect(buffer, source, scenario.writeSize, writesPerChunk,
+ WARMUP_SECONDS);
+ loopStreamFillWrap(buffer, source, scenario.writeSize, writesPerChunk,
+ WARMUP_SECONDS);
+ }
+ }
+ }
+
+ private static double median(double[] values) {
+ final double[] sorted = values.clone();
+ java.util.Arrays.sort(sorted);
+ return sorted[sorted.length / 2];
+ }
+
+ private static void runAllocationComparison(Scenario scenario) throws IOException {
+ printScenarioHeader(scenario);
+ if (!JfrByteBufferAllocations.isAvailable()) {
+ System.out.println(" JFR unavailable; reporting wrap-call count from put ops only.");
+ }
+ final AllocationResult direct = benchmarkAllocations(scenario,
+ ChunkBufferPutBenchmark::loopStreamFillDirect);
+ final AllocationResult wrap = benchmarkAllocations(scenario,
+ ChunkBufferPutBenchmark::loopStreamFillWrap);
+ printAllocationComparison(direct, wrap);
+ }
+
+ private static void printScenarioHeader(Scenario scenario) {
+ System.out.println("--- " + scenario.name + " ---");
+ System.out.println("Config: " + scenario.config);
+ System.out.println("Pattern: " + scenario.pattern);
+ System.out.printf("Chunk=%dKB increment=%dKB write=%dKB%n",
+ scenario.chunkSize / 1024,
+ scenario.bufferIncrement / 1024,
+ scenario.writeSize / 1024);
+ }
+
+ private static void printThroughputComparison(int writeSize, Result direct, Result wrap,
+ String prefix) {
+ final double speedup = wrap.nsPerOp / direct.nsPerOp;
+ final double pctFaster = (speedup - 1.0) * 100.0;
+ final double throughputSpeedup = direct.mbPerSec / wrap.mbPerSec;
+ final double throughputPct = (throughputSpeedup - 1.0) * 100.0;
+ System.out.printf("%sdirect put(byte[]): %s MB/s | %s ns/op | %.2fs | %s ops%n",
+ prefix, MBPS.format(direct.mbPerSec), NS.format(direct.nsPerOp),
+ direct.elapsedSeconds, COUNT.format(direct.ops));
+ System.out.printf("%swrap put(ByteBuffer): %s MB/s | %s ns/op | %.2fs | %s ops%n",
+ prefix, MBPS.format(wrap.mbPerSec), NS.format(wrap.nsPerOp),
+ wrap.elapsedSeconds, COUNT.format(wrap.ops));
+ System.out.printf("%simprovement: %s%% faster (%sx) per %dKB write; "
+ + "throughput %s%% (%sx)%n",
+ prefix, PCT.format(pctFaster), RATIO.format(speedup), writeSize / 1024,
+ PCT.format(throughputPct), RATIO.format(throughputSpeedup));
+ }
+
+ private static void printAllocationComparison(AllocationResult direct,
+ AllocationResult wrap) {
+ System.out.printf(" direct put(byte[]): %s put ops | %s ByteBuffer TLAB allocs | %s alloc bytes%n",
+ COUNT.format(direct.putOps), COUNT.format(direct.byteBufferAllocCount),
+ COUNT.format(direct.byteBufferAllocBytes));
+ System.out.printf(" wrap put(ByteBuffer): %s put ops | %s ByteBuffer TLAB allocs | %s alloc bytes%n",
+ COUNT.format(wrap.putOps), COUNT.format(wrap.byteBufferAllocCount),
+ COUNT.format(wrap.byteBufferAllocBytes));
+ System.out.printf(" ByteBuffer.wrap calls on wrap path (1 per put): %s%n",
+ COUNT.format(wrap.putOps));
+ System.out.printf(" direct path avoids %s ByteBuffer.wrap calls per run%n",
+ COUNT.format(wrap.putOps));
+ if (wrap.byteBufferAllocCount > 0 && direct.byteBufferAllocCount == 0) {
+ System.out.printf(" JFR confirms zero ByteBuffer TLAB allocations on direct path%n");
+ }
+ if (wrap.byteBufferAllocCount > direct.byteBufferAllocCount) {
+ final long saved = wrap.byteBufferAllocCount - direct.byteBufferAllocCount;
+ System.out.printf(" JFR sampled ByteBuffer TLAB allocations avoided on direct path: %s%n",
+ COUNT.format(saved));
+ System.out.println(" (JFR samples TLAB events; put-op count is the exact wrap-call metric)");
+ }
+ }
+
+ private static Result benchmarkThroughput(Scenario scenario, TimedLoop loop) {
+ final byte[] source = SOURCE.get();
+ final int writesPerChunk = scenario.chunkSize / scenario.writeSize;
+ try (ChunkBuffer buffer = ChunkBuffer.allocate(scenario.chunkSize,
+ scenario.bufferIncrement)) {
+ loop.run(buffer, source, scenario.writeSize, writesPerChunk, WARMUP_SECONDS);
+ final LoopResult benchmark = loop.run(buffer, source, scenario.writeSize,
+ writesPerChunk, BENCHMARK_SECONDS);
+ return toResult(benchmark, scenario.writeSize);
+ }
+ }
+
+ private static AllocationResult benchmarkAllocations(Scenario scenario, TimedLoop loop)
+ throws IOException {
+ final byte[] source = SOURCE.get();
+ final int writesPerChunk = scenario.chunkSize / scenario.writeSize;
+ try (ChunkBuffer buffer = ChunkBuffer.allocate(scenario.chunkSize,
+ scenario.bufferIncrement)) {
+ loop.run(buffer, source, scenario.writeSize, writesPerChunk, WARMUP_SECONDS);
+ final LoopResult[] benchmark = new LoopResult[1];
+ final AllocationStats stats = JfrByteBufferAllocations.measure(
+ () -> benchmark[0] = loop.run(buffer, source, scenario.writeSize,
+ writesPerChunk, ALLOCATION_BENCHMARK_SECONDS));
+ final long putOps = benchmark[0].totalBytes / scenario.writeSize;
+ return new AllocationResult(putOps, stats.getByteBufferAllocCount(),
+ stats.getByteBufferAllocBytes());
+ }
+ }
+
+ private static Result toResult(LoopResult benchmark, int writeSize) {
+ final long elapsedNanos = benchmark.elapsedNanos;
+ final long benchmarkBytes = benchmark.totalBytes;
+ final double seconds = elapsedNanos / 1_000_000_000.0;
+ final double mbPerSec = benchmarkBytes / seconds / (1024.0 * 1024.0);
+ final long ops = benchmarkBytes / writeSize;
+ final double nsPerOp = (double) elapsedNanos / ops;
+ return new Result(mbPerSec, nsPerOp, seconds, ops);
+ }
+
+ @FunctionalInterface
+ private interface TimedLoop {
+ LoopResult run(ChunkBuffer buffer, byte[] source, int writeSize,
+ int writesPerChunk, int seconds);
Review Comment:
I added separate unit tests that would be run for regression testing, benchmark is for running on demand only.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15485. Avoid ByteBuffer.wrap on ChunkBuffer.put(byte[]) path [ozone]
Posted by "yandrey321 (via GitHub)" <gi...@apache.org>.
yandrey321 commented on PR #10438:
URL: https://github.com/apache/ozone/pull/10438#issuecomment-4680846580
> Actually, the current runTestImpl(..) already testing put(byte[]). Before this change, it is actually testing put(ByteBuffer) since it use ByteBuffer.wrap(..). So, we need new tests for put(ByteBuffer).
the idea of the benchmark was to show the performance and memory allocation difference between added put(bytes[], offset, len) vs default implementation via put(ByteBuffer.wrap()) that was used before. It is not generic benchmark for put(bytes[]) vs put(ByteBuffer)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15485 Avoid ByteBuffer.wrap on ChunkBuffer.put(byte[]) path [ozone]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #10438:
URL: https://github.com/apache/ozone/pull/10438#discussion_r3383215308
##########
hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/common/ChunkBufferPutBenchmark.java:
##########
@@ -0,0 +1,346 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.common;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.text.DecimalFormat;
+import java.util.concurrent.ThreadLocalRandom;
+import org.apache.hadoop.ozone.common.JfrByteBufferAllocations.AllocationStats;
+
+/**
+ * Microbenchmark for ChunkBuffer.put(byte[]) direct copy vs ByteBuffer.wrap path.
+ *
+ * <p>Focused on the scenarios where HDDS-15485 shows the clearest benefit:
+ * <ul>
+ * <li>Throughput: 4KB stream fill with incremental buffer (64KB increment)</li>
+ * <li>Allocations: same 4KB / 64KB-increment incremental buffer path (JFR + wrap calls)</li>
+ * </ul>
+ *
+ * <p>Run from the repo root:
+ * <pre>
+ * mvn -pl hadoop-hdds/common -q test-compile exec:java \
+ * -Dexec.mainClass=org.apache.hadoop.ozone.common.ChunkBufferPutBenchmark \
+ * -Dexec.classpathScope=test \
+ * -Dexec.args="--add-opens jdk.jfr/jdk.jfr=ALL-UNNAMED --add-opens jdk.jfr/jdk.jfr.consumer=ALL-UNNAMED"
+ * </pre>
+ * JFR ByteBuffer counts are sampled; put-op count reports exact wrap calls.
+ * Wrap-path timings use a blackhole on each {@code ByteBuffer.wrap} so the JVM
+ * cannot eliminate short-lived wrapper allocations via escape analysis.
+ */
+public final class ChunkBufferPutBenchmark {
+
+ /** Prevents escape analysis from removing ByteBuffer.wrap allocations in the wrap path. */
+ private static volatile Object blackhole;
Review Comment:
@yandrey321 , I am really appreciated you working the improvement. As you already known, Java has [Escape Analysis](https://medium.com/@houssyahya/escape-analysis-in-java-04039ee1dc8b) so the ByteBuffer objects actually are not allocated. Thus, there is no performance improvement here. I guess we should not add more code to avoid more bugs.
Below is the result obtained by removing the blackhole:
```
ChunkBuffer.put(byte[]) microbenchmark (pre-allocated buffer, put-only)
JVM: 17.0.19 on aarch64
=== Throughput showcase ===
...
=== Allocation showcase ===
--- Incremental buffer showcase ---
Config: ozone.client.stream.buffer.size=4MB, ozone.client.stream.buffer.increment=64KB, io.file.buffer.size=4KB
Pattern: 4KB stream fill into IncrementalChunkBuffer (64KB steps)
Chunk=4096KB increment=64KB write=4KB
direct put(byte[]): 46,555,136 put ops | 0 ByteBuffer TLAB allocs | 0 alloc bytes
wrap put(ByteBuffer): 48,808,960 put ops | 900 ByteBuffer TLAB allocs | 50,400 alloc bytes
ByteBuffer.wrap calls on wrap path (1 per put): 48,808,960
direct path avoids 48,808,960 ByteBuffer.wrap calls per run
⚠️⚠️⚠️ JFR confirms zero ByteBuffer TLAB allocations on direct path ⚠️⚠️⚠️
JFR sampled ByteBuffer TLAB allocations avoided on direct path: 900
(JFR samples TLAB events; put-op count is the exact wrap-call metric)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15485 Avoid ByteBuffer.wrap on ChunkBuffer.put(byte[]) path [ozone]
Posted by "amaliujia (via GitHub)" <gi...@apache.org>.
amaliujia commented on code in PR #10438:
URL: https://github.com/apache/ozone/pull/10438#discussion_r3388981077
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/ChunkBuffer.java:
##########
@@ -64,6 +67,22 @@ static ChunkBuffer wrap(List<ByteBuffer> buffers) {
return new ChunkBufferImplWithByteBufferList(buffers);
}
+ default void checkArgument(byte[] b, int offset, int length) {
+ Objects.requireNonNull(b, "b == null");
+ Preconditions.checkArgument(offset >= 0 && length >= 0,
+ "offset = %s, length = %s", offset, length);
+ Preconditions.checkArgument(length <= b.length - offset,
+ "length = %s out of range for array.length = %s, offset = %s",
+ length, b.length, offset);
+ if (length > remaining()) {
+ final BufferOverflowException boe = new BufferOverflowException();
+ boe.initCause(new IllegalArgumentException(
+ "Failed to put since length = " + length
+ + " > this.remaining() = " + remaining()));
+ throw boe;
+ }
Review Comment:
Can we share the same code here with https://github.com/apache/ozone/blob/cecddd660080a80e269a0c4991e4f16f6232cc90/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/ChunkBufferImplWithByteBufferList.java#L140-L146
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15485 Avoid ByteBuffer.wrap on ChunkBuffer.put(byte[]) path [ozone]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on PR #10438:
URL: https://github.com/apache/ozone/pull/10438#issuecomment-4636370717
@yandrey321 , Please post the numbers and how to reproduce it. Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15485 Avoid ByteBuffer.wrap on ChunkBuffer.put(byte[]) path [ozone]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on PR #10438:
URL: https://github.com/apache/ozone/pull/10438#issuecomment-4662897858
Just tried but got negative improvement:
```
ChunkBuffer.put(byte[]) microbenchmark (pre-allocated buffer, put-only)
JVM: 17.0.19 on aarch64
=== Throughput showcase ===
--- Incremental buffer showcase ---
Config: ozone.client.stream.buffer.size=4MB, ozone.client.stream.buffer.increment=64KB, io.file.buffer.size=4KB
Pattern: 4KB stream fill into IncrementalChunkBuffer (64KB steps)
Chunk=4096KB increment=64KB write=4KB
(wrap path blackholes each ByteBuffer.wrap to model real allocation cost)
round 1:
direct put(byte[]): 35,665.9 MB/s | 110 ns/op | 20.00s | 182,609,920 ops
wrap put(ByteBuffer): 36,778.3 MB/s | 106 ns/op | 20.00s | 188,305,408 ops
improvement: -3.0% faster (0.97x) per 4KB write; throughput -3.0% (0.97x)
round 2:
direct put(byte[]): 35,690.1 MB/s | 109 ns/op | 20.00s | 182,733,824 ops
wrap put(ByteBuffer): 36,511.7 MB/s | 107 ns/op | 20.00s | 186,940,416 ops
improvement: -2.3% faster (0.98x) per 4KB write; throughput -2.3% (0.98x)
round 3:
direct put(byte[]): 35,652.3 MB/s | 110 ns/op | 20.00s | 182,540,288 ops
wrap put(ByteBuffer): 37,125.0 MB/s | 105 ns/op | 20.00s | 190,080,000 ops
improvement: -4.0% faster (0.96x) per 4KB write; throughput -4.0% (0.96x)
median improvement over 3 rounds: -3.0%
=== Allocation showcase ===
--- Incremental buffer showcase ---
Config: ozone.client.stream.buffer.size=4MB, ozone.client.stream.buffer.increment=64KB, io.file.buffer.size=4KB
Pattern: 4KB stream fill into IncrementalChunkBuffer (64KB steps)
Chunk=4096KB increment=64KB write=4KB
direct put(byte[]): 51,306,496 put ops | 0 ByteBuffer TLAB allocs | 0 alloc bytes
wrap put(ByteBuffer): 46,695,424 put ops | 860 ByteBuffer TLAB allocs | 48,160 alloc bytes
ByteBuffer.wrap calls on wrap path (1 per put): 46,695,424
direct path avoids 46,695,424 ByteBuffer.wrap calls per run
JFR confirms zero ByteBuffer TLAB allocations on direct path
JFR sampled ByteBuffer TLAB allocations avoided on direct path: 860
(JFR samples TLAB events; put-op count is the exact wrap-call metric)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15485 Avoid ByteBuffer.wrap on ChunkBuffer.put(byte[]) path [ozone]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #10438:
URL: https://github.com/apache/ozone/pull/10438#discussion_r3383215308
##########
hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/common/ChunkBufferPutBenchmark.java:
##########
@@ -0,0 +1,346 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.common;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.text.DecimalFormat;
+import java.util.concurrent.ThreadLocalRandom;
+import org.apache.hadoop.ozone.common.JfrByteBufferAllocations.AllocationStats;
+
+/**
+ * Microbenchmark for ChunkBuffer.put(byte[]) direct copy vs ByteBuffer.wrap path.
+ *
+ * <p>Focused on the scenarios where HDDS-15485 shows the clearest benefit:
+ * <ul>
+ * <li>Throughput: 4KB stream fill with incremental buffer (64KB increment)</li>
+ * <li>Allocations: same 4KB / 64KB-increment incremental buffer path (JFR + wrap calls)</li>
+ * </ul>
+ *
+ * <p>Run from the repo root:
+ * <pre>
+ * mvn -pl hadoop-hdds/common -q test-compile exec:java \
+ * -Dexec.mainClass=org.apache.hadoop.ozone.common.ChunkBufferPutBenchmark \
+ * -Dexec.classpathScope=test \
+ * -Dexec.args="--add-opens jdk.jfr/jdk.jfr=ALL-UNNAMED --add-opens jdk.jfr/jdk.jfr.consumer=ALL-UNNAMED"
+ * </pre>
+ * JFR ByteBuffer counts are sampled; put-op count reports exact wrap calls.
+ * Wrap-path timings use a blackhole on each {@code ByteBuffer.wrap} so the JVM
+ * cannot eliminate short-lived wrapper allocations via escape analysis.
+ */
+public final class ChunkBufferPutBenchmark {
+
+ /** Prevents escape analysis from removing ByteBuffer.wrap allocations in the wrap path. */
+ private static volatile Object blackhole;
Review Comment:
@yandrey321 , I am really appreciated you working the improvement. As you already known, Java has [Escape Analysis](https://medium.com/@houssyahya/escape-analysis-in-java-04039ee1dc8b) so the ByteBuffer objects actually are not allocated. Thus, there is no performance improvement here. I guess we should not add more code to avoid more bugs.
Below is the result obtained by removing the blackhole and changed THROUGHPUT_ROUNDS to 4. Got two positive and two negative results:
```
ChunkBuffer.put(byte[]) microbenchmark (pre-allocated buffer, put-only)
JVM: 17.0.19 on aarch64
=== Throughput showcase ===
--- Incremental buffer showcase ---
Config: ozone.client.stream.buffer.size=4MB, ozone.client.stream.buffer.increment=64KB, io.file.buffer.size=4KB
Pattern: 4KB stream fill into IncrementalChunkBuffer (64KB steps)
Chunk=4096KB increment=64KB write=4KB
(wrap path blackholes each ByteBuffer.wrap to model real allocation cost)
round 1:
direct put(byte[]): 37,761.8 MB/s | 103 ns/op | 20.00s | 193,341,440 ops
wrap put(ByteBuffer): 39,230.7 MB/s | 100 ns/op | 20.00s | 200,861,696 ops
improvement: -3.7% faster (0.96x) per 4KB write; throughput -3.7% (0.96x)
round 2:
direct put(byte[]): 38,976.4 MB/s | 100 ns/op | 20.00s | 199,559,168 ops
wrap put(ByteBuffer): 38,602.3 MB/s | 101 ns/op | 20.00s | 197,644,288 ops
improvement: 1.0% faster (1.01x) per 4KB write; throughput 1.0% (1.01x)
round 3:
direct put(byte[]): 38,392.9 MB/s | 102 ns/op | 20.00s | 196,572,160 ops
wrap put(ByteBuffer): 39,236.6 MB/s | 100 ns/op | 20.00s | 200,891,392 ops
improvement: -2.2% faster (0.98x) per 4KB write; throughput -2.2% (0.98x)
round 4:
direct put(byte[]): 39,011.6 MB/s | 100 ns/op | 20.00s | 199,740,416 ops
wrap put(ByteBuffer): 38,522.0 MB/s | 101 ns/op | 20.00s | 197,232,640 ops
improvement: 1.3% faster (1.01x) per 4KB write; throughput 1.3% (1.01x)
median improvement over 4 rounds: 1.0%
=== Allocation showcase ===
--- Incremental buffer showcase ---
Config: ozone.client.stream.buffer.size=4MB, ozone.client.stream.buffer.increment=64KB, io.file.buffer.size=4KB
Pattern: 4KB stream fill into IncrementalChunkBuffer (64KB steps)
Chunk=4096KB increment=64KB write=4KB
direct put(byte[]): 46,555,136 put ops | 0 ByteBuffer TLAB allocs | 0 alloc bytes
wrap put(ByteBuffer): 48,808,960 put ops | 900 ByteBuffer TLAB allocs | 50,400 alloc bytes
ByteBuffer.wrap calls on wrap path (1 per put): 48,808,960
direct path avoids 48,808,960 ByteBuffer.wrap calls per run
JFR confirms zero ByteBuffer TLAB allocations on direct path
⚠️⚠️⚠️ JFR sampled ByteBuffer TLAB allocations avoided on direct path: 900 ⚠️⚠️⚠️
(JFR samples TLAB events; put-op count is the exact wrap-call metric)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15485 Avoid ByteBuffer.wrap on ChunkBuffer.put(byte[]) path [ozone]
Posted by "yandrey321 (via GitHub)" <gi...@apache.org>.
yandrey321 commented on PR #10438:
URL: https://github.com/apache/ozone/pull/10438#issuecomment-4632687039
> before this change every put(byte[], offset, length ):
>
> Allocates a new ByteBuffer view (HeapByteBuffer over the same backing array). Dispatches to put(ByteBuffer), which for list/incremental buffers runs a segment loop that adjusts the wrapper’s position/limit and copies via ByteBuffer.put(ByteBuffer).
>
> On the client write path, BlockOutputStream.write(byte[], …) calls currentBuffer.put(b, off, writeLen) once per slice that fits in the current chunk buffer. Large writes can do that many times per write() call.
>
> wrap itself is “cheap” (no array copy), but per call it still pays object allocation + dispatch into the ByteBuffer put path. On a hot path called for every slice of every write(byte[]), that cost is measurable under load.
>
> the fix is micro-optimization, but should reduce memory allocations on hot path and reduce number of objects for garbage collection.
I update the description:
before this change every put(byte[], offset, length ):
Allocates a new ByteBuffer view (HeapByteBuffer over the same backing array).
Dispatches to put(ByteBuffer), which for list/incremental buffers runs a segment loop that adjusts the wrapper’s position/limit and copies via ByteBuffer.put(ByteBuffer).
On the client write path, BlockOutputStream.write(byte[], …) calls currentBuffer.put(b, off, writeLen) once per slice that fits in the current chunk buffer. Large writes can do that many times per write() call.
wrap itself is “cheap” (no array copy), but per call it still pays object allocation + dispatch into the ByteBuffer put path. On a hot path called for every slice of every write(byte[]), that cost is measurable under load.
the fix is micro-optimization, but should reduce memory allocations on hot path and reduce number of objects for garbage collection.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15485 Avoid ByteBuffer.wrap on ChunkBuffer.put(byte[]) path [ozone]
Posted by "amaliujia (via GitHub)" <gi...@apache.org>.
amaliujia commented on PR #10438:
URL: https://github.com/apache/ozone/pull/10438#issuecomment-4671106047
After reading the core changes of this PR, I feel like maybe we do not need an extra config for kill switch, mainly because for the new implementation of `put(byte[] b, int offset, int length)`, we always has the reference implementation from `ChunkBuffer put(ByteBuffer that)`. It is also one line by one line mirroring for `ChunkBufferImplWithByteBufferList` and `IncrementalChunkBuffer`. The risk then becomes lower.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15485. Avoid ByteBuffer.wrap on ChunkBuffer.put(byte[]) path [ozone]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #10438:
URL: https://github.com/apache/ozone/pull/10438#issuecomment-4949293278
Thank you for your contribution. This PR is being closed due to inactivity. Please contact a maintainer if you would like to reopen it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15485 Avoid ByteBuffer.wrap on ChunkBuffer.put(byte[]) path [ozone]
Posted by "yandrey321 (via GitHub)" <gi...@apache.org>.
yandrey321 commented on PR #10438:
URL: https://github.com/apache/ozone/pull/10438#issuecomment-4636309254
> @yandrey321 , Could you justify the word "measurable"?
Its around ~0.5-1% on the real system, in microbenchmark it shows ~7% better throughput. It also slightly reduce work for GC (hard to measure). It is not a major performance improvement but a small optimization that improves write path for a scenario when client passes a byte array.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15485 Avoid ByteBuffer.wrap on ChunkBuffer.put(byte[]) path [ozone]
Posted by "amaliujia (via GitHub)" <gi...@apache.org>.
amaliujia commented on PR #10438:
URL: https://github.com/apache/ozone/pull/10438#issuecomment-4666847247
I will also review the code today when getting a chance.
@yandrey321 really appreciate your work! I am thinking given that this is a hot and critical path, does it make sense that we introduce a config to have a kill-switch and could fall back to `ByteBuffer.wrap`, in case there is any bugs we couldn't discover now? After all the previous `ByteBuffer.wrap` was easy to reason, but now we introduce a lot of new code in the path.
WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15485. Avoid ByteBuffer.wrap on ChunkBuffer.put(byte[]) path [ozone]
Posted by "yandrey321 (via GitHub)" <gi...@apache.org>.
yandrey321 commented on PR #10438:
URL: https://github.com/apache/ozone/pull/10438#issuecomment-4694467921
> > So, we need new tests for put(ByteBuffer).
>
> > the idea of the benchmark ...
>
> @yandrey321 , I mean we need unit tests for asserting correctness but not for performance.
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15485 Avoid ByteBuffer.wrap on ChunkBuffer.put(byte[]) path [ozone]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on PR #10438:
URL: https://github.com/apache/ozone/pull/10438#issuecomment-4636122251
> ... that cost is measurable under load.
@yandrey321 , Could you justify the word "measurable"?
In Java, we usually don't care about (short live) object allocations. Otherwise, all the classes such as String, Integer, etc cannot be used.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15485 Avoid ByteBuffer.wrap on ChunkBuffer.put(byte[]) path [ozone]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #10438:
URL: https://github.com/apache/ozone/pull/10438#discussion_r3383215308
##########
hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/common/ChunkBufferPutBenchmark.java:
##########
@@ -0,0 +1,346 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.common;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.text.DecimalFormat;
+import java.util.concurrent.ThreadLocalRandom;
+import org.apache.hadoop.ozone.common.JfrByteBufferAllocations.AllocationStats;
+
+/**
+ * Microbenchmark for ChunkBuffer.put(byte[]) direct copy vs ByteBuffer.wrap path.
+ *
+ * <p>Focused on the scenarios where HDDS-15485 shows the clearest benefit:
+ * <ul>
+ * <li>Throughput: 4KB stream fill with incremental buffer (64KB increment)</li>
+ * <li>Allocations: same 4KB / 64KB-increment incremental buffer path (JFR + wrap calls)</li>
+ * </ul>
+ *
+ * <p>Run from the repo root:
+ * <pre>
+ * mvn -pl hadoop-hdds/common -q test-compile exec:java \
+ * -Dexec.mainClass=org.apache.hadoop.ozone.common.ChunkBufferPutBenchmark \
+ * -Dexec.classpathScope=test \
+ * -Dexec.args="--add-opens jdk.jfr/jdk.jfr=ALL-UNNAMED --add-opens jdk.jfr/jdk.jfr.consumer=ALL-UNNAMED"
+ * </pre>
+ * JFR ByteBuffer counts are sampled; put-op count reports exact wrap calls.
+ * Wrap-path timings use a blackhole on each {@code ByteBuffer.wrap} so the JVM
+ * cannot eliminate short-lived wrapper allocations via escape analysis.
+ */
+public final class ChunkBufferPutBenchmark {
+
+ /** Prevents escape analysis from removing ByteBuffer.wrap allocations in the wrap path. */
+ private static volatile Object blackhole;
Review Comment:
@yandrey321 , I am really appreciated you working the improvement. As you already known, Java has [Escape Analysis](https://medium.com/@houssyahya/escape-analysis-in-java-04039ee1dc8b) so the ByteBuffer objects actually are not allocated. Thus, there is no performance improvement here. I guess we should not add more code to avoid more bugs.
Below is the result obtained by removing the blackhole:
```
ChunkBuffer.put(byte[]) microbenchmark (pre-allocated buffer, put-only)
JVM: 17.0.19 on aarch64
=== Throughput showcase ===
...
=== Allocation showcase ===
--- Incremental buffer showcase ---
Config: ozone.client.stream.buffer.size=4MB, ozone.client.stream.buffer.increment=64KB, io.file.buffer.size=4KB
Pattern: 4KB stream fill into IncrementalChunkBuffer (64KB steps)
Chunk=4096KB increment=64KB write=4KB
direct put(byte[]): 46,555,136 put ops | 0 ByteBuffer TLAB allocs | 0 alloc bytes
wrap put(ByteBuffer): 48,808,960 put ops | 900 ByteBuffer TLAB allocs | 50,400 alloc bytes
ByteBuffer.wrap calls on wrap path (1 per put): 48,808,960
direct path avoids 48,808,960 ByteBuffer.wrap calls per run
JFR confirms zero ByteBuffer TLAB allocations on direct path
⚠️⚠️⚠️ JFR sampled ByteBuffer TLAB allocations avoided on direct path: 900 ⚠️⚠️⚠️
(JFR samples TLAB events; put-op count is the exact wrap-call metric)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15485 Avoid ByteBuffer.wrap on ChunkBuffer.put(byte[]) path [ozone]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #10438:
URL: https://github.com/apache/ozone/pull/10438#discussion_r3383215308
##########
hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/common/ChunkBufferPutBenchmark.java:
##########
@@ -0,0 +1,346 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.common;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.text.DecimalFormat;
+import java.util.concurrent.ThreadLocalRandom;
+import org.apache.hadoop.ozone.common.JfrByteBufferAllocations.AllocationStats;
+
+/**
+ * Microbenchmark for ChunkBuffer.put(byte[]) direct copy vs ByteBuffer.wrap path.
+ *
+ * <p>Focused on the scenarios where HDDS-15485 shows the clearest benefit:
+ * <ul>
+ * <li>Throughput: 4KB stream fill with incremental buffer (64KB increment)</li>
+ * <li>Allocations: same 4KB / 64KB-increment incremental buffer path (JFR + wrap calls)</li>
+ * </ul>
+ *
+ * <p>Run from the repo root:
+ * <pre>
+ * mvn -pl hadoop-hdds/common -q test-compile exec:java \
+ * -Dexec.mainClass=org.apache.hadoop.ozone.common.ChunkBufferPutBenchmark \
+ * -Dexec.classpathScope=test \
+ * -Dexec.args="--add-opens jdk.jfr/jdk.jfr=ALL-UNNAMED --add-opens jdk.jfr/jdk.jfr.consumer=ALL-UNNAMED"
+ * </pre>
+ * JFR ByteBuffer counts are sampled; put-op count reports exact wrap calls.
+ * Wrap-path timings use a blackhole on each {@code ByteBuffer.wrap} so the JVM
+ * cannot eliminate short-lived wrapper allocations via escape analysis.
+ */
+public final class ChunkBufferPutBenchmark {
+
+ /** Prevents escape analysis from removing ByteBuffer.wrap allocations in the wrap path. */
+ private static volatile Object blackhole;
Review Comment:
@yandrey321 , I am really appreciated your effort! I also have learned a lot in reviewing this PR.
As you already known, Java has [Escape Analysis](https://medium.com/@houssyahya/escape-analysis-in-java-04039ee1dc8b) so the ByteBuffer objects actually may not be allocated. Thus, there is no performance improvement here. I guess we should not add more code to avoid more bugs.
Below is the result obtained by removing the blackhole and changed THROUGHPUT_ROUNDS to 4. Got two positive and two negative results:
```
ChunkBuffer.put(byte[]) microbenchmark (pre-allocated buffer, put-only)
JVM: 17.0.19 on aarch64
=== Throughput showcase ===
--- Incremental buffer showcase ---
Config: ozone.client.stream.buffer.size=4MB, ozone.client.stream.buffer.increment=64KB, io.file.buffer.size=4KB
Pattern: 4KB stream fill into IncrementalChunkBuffer (64KB steps)
Chunk=4096KB increment=64KB write=4KB
(wrap path blackholes each ByteBuffer.wrap to model real allocation cost)
round 1:
direct put(byte[]): 37,761.8 MB/s | 103 ns/op | 20.00s | 193,341,440 ops
wrap put(ByteBuffer): 39,230.7 MB/s | 100 ns/op | 20.00s | 200,861,696 ops
improvement: -3.7% faster (0.96x) per 4KB write; throughput -3.7% (0.96x)
round 2:
direct put(byte[]): 38,976.4 MB/s | 100 ns/op | 20.00s | 199,559,168 ops
wrap put(ByteBuffer): 38,602.3 MB/s | 101 ns/op | 20.00s | 197,644,288 ops
improvement: 1.0% faster (1.01x) per 4KB write; throughput 1.0% (1.01x)
round 3:
direct put(byte[]): 38,392.9 MB/s | 102 ns/op | 20.00s | 196,572,160 ops
wrap put(ByteBuffer): 39,236.6 MB/s | 100 ns/op | 20.00s | 200,891,392 ops
improvement: -2.2% faster (0.98x) per 4KB write; throughput -2.2% (0.98x)
round 4:
direct put(byte[]): 39,011.6 MB/s | 100 ns/op | 20.00s | 199,740,416 ops
wrap put(ByteBuffer): 38,522.0 MB/s | 101 ns/op | 20.00s | 197,232,640 ops
improvement: 1.3% faster (1.01x) per 4KB write; throughput 1.3% (1.01x)
median improvement over 4 rounds: 1.0%
=== Allocation showcase ===
--- Incremental buffer showcase ---
Config: ozone.client.stream.buffer.size=4MB, ozone.client.stream.buffer.increment=64KB, io.file.buffer.size=4KB
Pattern: 4KB stream fill into IncrementalChunkBuffer (64KB steps)
Chunk=4096KB increment=64KB write=4KB
direct put(byte[]): 46,555,136 put ops | 0 ByteBuffer TLAB allocs | 0 alloc bytes
wrap put(ByteBuffer): 48,808,960 put ops | 900 ByteBuffer TLAB allocs | 50,400 alloc bytes
ByteBuffer.wrap calls on wrap path (1 per put): 48,808,960
direct path avoids 48,808,960 ByteBuffer.wrap calls per run
JFR confirms zero ByteBuffer TLAB allocations on direct path
JFR sampled ByteBuffer TLAB allocations avoided on direct path: 900
(JFR samples TLAB events; put-op count is the exact wrap-call metric)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15485 Avoid ByteBuffer.wrap on ChunkBuffer.put(byte[]) path [ozone]
Posted by "yandrey321 (via GitHub)" <gi...@apache.org>.
yandrey321 commented on code in PR #10438:
URL: https://github.com/apache/ozone/pull/10438#discussion_r3384491376
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/ChunkBufferImplWithByteBufferList.java:
##########
@@ -157,6 +157,39 @@ public ChunkBuffer put(ByteBuffer that) {
return this;
}
+ @Override
+ public ChunkBuffer put(byte[] b, int offset, int length) {
+ Objects.requireNonNull(b, "b == null");
+ Preconditions.checkArgument(offset >= 0 && length >= 0,
+ "offset = %s, length = %s", offset, length);
+ Preconditions.checkArgument(length <= b.length - offset,
+ "length = %s out of range for array.length = %s, offset = %s",
+ length, b.length, offset);
+ if (length > remaining()) {
+ final BufferOverflowException boe = new BufferOverflowException();
+ boe.initCause(new IllegalArgumentException(
+ "Failed to put since length = " + length
+ + " > this.remaining() = " + remaining()));
+ throw boe;
+ }
Review Comment:
fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15485 Avoid ByteBuffer.wrap on ChunkBuffer.put(byte[]) path [ozone]
Posted by "yandrey321 (via GitHub)" <gi...@apache.org>.
yandrey321 commented on PR #10438:
URL: https://github.com/apache/ozone/pull/10438#issuecomment-4661954338
> > Its around ~0.5-1% on the real system, in microbenchmark it shows ~7% better throughput. ...
>
> @yandrey321 , Please post the actual numbers and how to reproduce it. Thanks.
>
> > In Java, we usually don't care about (short live) object allocations. Otherwise, all the classes such as String, Integer, etc cannot be used.
>
> BTW, ByteBuffer.warp here is very similar to Java auto-boxing for primitive types; see https://docs.oracle.com/javase/tutorial/java/data/autoboxing.html
I added benchmark test (that can be run on demand) and updated description with results of test run on my mac (ARM cpu).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org