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/17 15:31:49 UTC
[PR] HDDS-15592 use heap allocation in ChunkBuffer [ozone]
yandrey321 opened a new pull request, #10536:
URL: https://github.com/apache/ozone/pull/10536
## What changes were proposed in this pull request?
When ChunkBuffer allocates heap buffer UnsafeByteOperations.unsafeWrap() returns a BoundedByteString with a backing array, enabling a single System.arraycopy into the gRPC/Netty wire buffer instead of the slow byte-by-byte NioByteString path for direct buffers.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-15592
## How was this patch tested?
Benchmark run on m3 mac/ARM with 14 cores producing the following results:
--- scaling summary writeSize=4096KB ---
threads | direct MB/s | heap MB/s | heap/direct | direct eff. | heap eff.
---------|--------------|--------------|------------|------------|------------
1 | 5,098.0 | 5,624.6 | 1.10x | 1.00 | 1.00
2 | 9,326.6 | 10,766.9 | 1.15x | 0.91 | 0.96
4 | 17,293.4 | 18,886.2 | 1.09x | 0.85 | 0.84
7 | 24,220.6 | 27,652.1 | 1.14x | 0.68 | 0.70
14 | 25,092.3 | 31,294.6 | 1.25x | 0.35 | 0.40
28 | 26,017.8 | 28,899.0 | 1.11x | 0.18 | 0.18
42 | 27,288.1 | 30,836.3 | 1.13x | 0.13 | 0.13
--- scaling summary writeSize=3072KB ---
threads | direct MB/s | heap MB/s | heap/direct | direct eff. | heap eff.
---------|--------------|--------------|------------|------------|------------
1 | 6,441.7 | 7,407.9 | 1.15x | 1.00 | 1.00
2 | 12,718.8 | 14,405.9 | 1.13x | 0.99 | 0.97
4 | 24,420.3 | 27,779.9 | 1.14x | 0.95 | 0.94
7 | 39,524.5 | 44,928.1 | 1.14x | 0.88 | 0.87
14 | 47,319.0 | 58,903.6 | 1.24x | 0.52 | 0.57
28 | 41,418.1 | 50,173.2 | 1.21x | 0.23 | 0.24
42 | 39,672.7 | 50,588.1 | 1.28x | 0.15 | 0.16
--- scaling summary writeSize=2048KB ---
threads | direct MB/s | heap MB/s | heap/direct | direct eff. | heap eff.
---------|--------------|--------------|------------|------------|------------
1 | 5,092.0 | 5,753.0 | 1.13x | 1.00 | 1.00
2 | 9,601.4 | 10,889.3 | 1.13x | 0.94 | 0.95
4 | 17,719.7 | 18,854.6 | 1.06x | 0.87 | 0.82
7 | 24,087.2 | 27,649.2 | 1.15x | 0.68 | 0.69
14 | 26,875.0 | 30,787.4 | 1.15x | 0.38 | 0.38
28 | 26,819.5 | 30,021.1 | 1.12x | 0.19 | 0.19
42 | 27,593.0 | 30,644.9 | 1.11x | 0.13 | 0.13
--- scaling summary writeSize=1024KB ---
threads | direct MB/s | heap MB/s | heap/direct | direct eff. | heap eff.
---------|--------------|--------------|------------|------------|------------
1 | 5,059.7 | 5,682.8 | 1.12x | 1.00 | 1.00
2 | 9,736.1 | 10,943.2 | 1.12x | 0.96 | 0.96
4 | 17,766.6 | 19,292.5 | 1.09x | 0.88 | 0.85
7 | 24,588.8 | 28,444.3 | 1.16x | 0.69 | 0.72
14 | 27,671.9 | 31,524.6 | 1.14x | 0.39 | 0.40
28 | 26,259.2 | 30,340.7 | 1.16x | 0.19 | 0.19
42 | 27,058.7 | 30,937.3 | 1.14x | 0.13 | 0.13
--- scaling summary writeSize=512KB ---
threads | direct MB/s | heap MB/s | heap/direct | direct eff. | heap eff.
---------|--------------|--------------|------------|------------|------------
1 | 4,843.5 | 5,502.6 | 1.14x | 1.00 | 1.00
2 | 9,165.2 | 10,428.1 | 1.14x | 0.95 | 0.95
4 | 17,641.7 | 19,249.4 | 1.09x | 0.91 | 0.87
7 | 24,383.0 | 27,974.6 | 1.15x | 0.72 | 0.73
14 | 27,056.3 | 30,301.9 | 1.12x | 0.40 | 0.39
28 | 27,436.6 | 30,545.3 | 1.11x | 0.20 | 0.20
42 | 27,118.8 | 30,685.3 | 1.13x | 0.13 | 0.13
--- scaling summary writeSize=256KB ---
threads | direct MB/s | heap MB/s | heap/direct | direct eff. | heap eff.
---------|--------------|--------------|------------|------------|------------
1 | 4,985.1 | 5,655.3 | 1.13x | 1.00 | 1.00
2 | 9,738.0 | 10,646.6 | 1.09x | 0.98 | 0.94
4 | 17,132.0 | 19,142.1 | 1.12x | 0.86 | 0.85
7 | 24,196.4 | 28,211.3 | 1.17x | 0.69 | 0.71
14 | 27,048.4 | 31,631.4 | 1.17x | 0.39 | 0.40
28 | 27,276.7 | 28,319.7 | 1.04x | 0.20 | 0.18
42 | 26,891.1 | 30,444.4 | 1.13x | 0.13 | 0.13
--
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-15592 use heap allocation in ChunkBuffer [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #10536:
URL: https://github.com/apache/ozone/pull/10536#discussion_r3443710526
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/ChunkBuffer.java:
##########
@@ -17,18 +17,30 @@
package org.apache.hadoop.ozone.common;
+import com.google.common.annotations.VisibleForTesting;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.channels.GatheringByteChannel;
import java.util.List;
import java.util.Objects;
+import java.util.concurrent.atomic.AtomicBoolean;
import org.apache.hadoop.hdds.utils.db.CodecBuffer;
import org.apache.ratis.thirdparty.com.google.protobuf.ByteString;
import org.apache.ratis.util.UncheckedAutoCloseable;
/** Buffer for a block chunk. */
public interface ChunkBuffer extends ChunkBufferToByteString, UncheckedAutoCloseable {
+ /**
+ * When true, {@link #allocate} uses direct ByteBuffers instead of heap ByteBuffers.
+ * Set only by {@link BlockOutputStreamWriteBenchmark} to compare allocation strategies
+ * within a single benchmark run; must never be modified in production code.
+ */
+ @VisibleForTesting
+ // CHECKSTYLE:OFF VisibilityModifier
+ AtomicBoolean ALLOCATE_DIRECT = new AtomicBoolean(false);
+ // CHECKSTYLE:ON VisibilityModifier
Review Comment:
But this is not the benchmark 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-15592 use heap allocation in ChunkBuffer [ozone]
Posted by "yandrey321 (via GitHub)" <gi...@apache.org>.
yandrey321 closed pull request #10536: HDDS-15592 use heap allocation in ChunkBuffer
URL: https://github.com/apache/ozone/pull/10536
--
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-15592 use heap allocation in ChunkBuffer [ozone]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on PR #10536:
URL: https://github.com/apache/ozone/pull/10536#issuecomment-4927870288
@yandrey321 , it seems that the benchmark is a mock but does not involve real network IO. If it is it the case, the improvement probably would become negative when network IO is involved.
--
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-15592 use heap allocation in ChunkBuffer [ozone]
Posted by "errose28 (via GitHub)" <gi...@apache.org>.
errose28 commented on PR #10536:
URL: https://github.com/apache/ozone/pull/10536#issuecomment-4770749013
cc @rnblough
--
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-15592 use heap allocation in ChunkBuffer [ozone]
Posted by "yandrey321 (via GitHub)" <gi...@apache.org>.
yandrey321 commented on code in PR #10536:
URL: https://github.com/apache/ozone/pull/10536#discussion_r3464191137
##########
hadoop-hdds/client/dev-support/run-block-output-stream-write-benchmark.sh:
##########
Review Comment:
removed the shell script
--
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-15592 use heap allocation in ChunkBuffer [ozone]
Posted by "yandrey321 (via GitHub)" <gi...@apache.org>.
yandrey321 commented on PR #10536:
URL: https://github.com/apache/ozone/pull/10536#issuecomment-4938667726
> LiteralByteString
I can reproduce Sergey's findings with variation of the existing mock by introducing the back pressure and reducing the heap size for benchmark, when I do that I see ~30% of GC time increase and more objects retained in the heap. It seems like the best fix for this issue is to use Ratis data streaming.
--
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-15592 use heap allocation in ChunkBuffer [ozone]
Posted by "yandrey321 (via GitHub)" <gi...@apache.org>.
yandrey321 commented on code in PR #10536:
URL: https://github.com/apache/ozone/pull/10536#discussion_r3464168345
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/ChunkBuffer.java:
##########
@@ -17,18 +17,30 @@
package org.apache.hadoop.ozone.common;
+import com.google.common.annotations.VisibleForTesting;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.channels.GatheringByteChannel;
import java.util.List;
import java.util.Objects;
+import java.util.concurrent.atomic.AtomicBoolean;
import org.apache.hadoop.hdds.utils.db.CodecBuffer;
import org.apache.ratis.thirdparty.com.google.protobuf.ByteString;
import org.apache.ratis.util.UncheckedAutoCloseable;
/** Buffer for a block chunk. */
public interface ChunkBuffer extends ChunkBufferToByteString, UncheckedAutoCloseable {
+ /**
+ * When true, {@link #allocate} uses direct ByteBuffers instead of heap ByteBuffers.
+ * Set only by {@link BlockOutputStreamWriteBenchmark} to compare allocation strategies
+ * within a single benchmark run; must never be modified in production code.
+ */
+ @VisibleForTesting
+ // CHECKSTYLE:OFF VisibilityModifier
+ AtomicBoolean ALLOCATE_DIRECT = new AtomicBoolean(false);
+ // CHECKSTYLE:ON VisibilityModifier
Review Comment:
Removed the switch and use heap allocations by default.
--
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-15592 use heap allocation in ChunkBuffer [ozone]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on PR #10536:
URL: https://github.com/apache/ozone/pull/10536#issuecomment-4928032093
> When ChunkBuffer allocates heap buffer UnsafeByteOperations.unsafeWrap() returns a BoundedByteString ...
For direct buffer, UnsafeByteOperations.unsafeWrap returns NioByteString but not BoundedByteString.
- Output:
```
class org.apache.ratis.thirdparty.com.google.protobuf.NioByteString
class com.google.protobuf.NioByteString
```
```java
public static void main(String[] args) {
final ByteBuffer buffer = ByteBuffer.allocateDirect(4);
buffer.putInt(0);
System.out.println(UnsafeByteOperations.unsafeWrap(buffer).getClass());
System.out.println(com.google.protobuf.UnsafeByteOperations.unsafeWrap(buffer).getClass());
}
```
--
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-15592 use heap allocation in ChunkBuffer [ozone]
Posted by "yandrey321 (via GitHub)" <gi...@apache.org>.
yandrey321 commented on PR #10536:
URL: https://github.com/apache/ozone/pull/10536#issuecomment-4938037576
> I see a significant performance degradation on the client side. The scenario is with MinIO Warp running with 10 concurrent threads through the S3 gateway: before patch: warp_benchmark_report_20260709_165607.txt: * Average: 458.75 MiB/s, 4.59 obj/s warp_benchmark_report_20260709_174454.txt: * Average: 469.64 MiB/s, 4.70 obj/s warp_benchmark_report_20260709_185000.txt: * Average: 458.72 MiB/s, 4.59 obj/s warp_benchmark_report_20260709_190148.txt: * Average: 465.89 MiB/s, 4.66 obj/s
>
> after patch: warp_benchmark_report_20260709_200025.txt: * Average: 389.08 MiB/s, 3.89 obj/s warp_benchmark_report_20260709_201203.txt: * Average: 398.94 MiB/s, 3.99 obj/s warp_benchmark_report_20260710_065601.txt: * Average: 407.81 MiB/s, 4.08 obj/s warp_benchmark_report_20260710_070741.txt: * Average: 409.70 MiB/s, 4.10 obj/s
>
> Definitely, the assumption that the patch should increase the performance is wrong. It reintroduces the client heap-pressure problem that [HDDS-9843](https://issues.apache.org/jira/browse/HDDS-9843) specifically moved these buffers away from.
@ss77892 what params did you use for warp test? Let me check if there is way to address this.
--
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-15592 use heap allocation in ChunkBuffer [ozone]
Posted by "ss77892 (via GitHub)" <gi...@apache.org>.
ss77892 commented on PR #10536:
URL: https://github.com/apache/ozone/pull/10536#issuecomment-4937922048
I see a significant performance degradation on the client side. The scenario is with MinIO Warp running with 10 concurrent threads through the S3 gateway:
before patch:
warp_benchmark_report_20260709_165607.txt: * Average: 458.75 MiB/s, 4.59 obj/s
warp_benchmark_report_20260709_174454.txt: * Average: 469.64 MiB/s, 4.70 obj/s
warp_benchmark_report_20260709_185000.txt: * Average: 458.72 MiB/s, 4.59 obj/s
warp_benchmark_report_20260709_190148.txt: * Average: 465.89 MiB/s, 4.66 obj/s
after patch:
warp_benchmark_report_20260709_200025.txt: * Average: 389.08 MiB/s, 3.89 obj/s
warp_benchmark_report_20260709_201203.txt: * Average: 398.94 MiB/s, 3.99 obj/s
warp_benchmark_report_20260710_065601.txt: * Average: 407.81 MiB/s, 4.08 obj/s
warp_benchmark_report_20260710_070741.txt: * Average: 409.70 MiB/s, 4.10 obj/s
Definitely, the assumption that the patch should increase the performance is wrong. It reintroduces the client heap-pressure problem that HDDS-9843 specifically moved these buffers away from.
--
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-15592 use heap allocation in ChunkBuffer [ozone]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on PR #10536:
URL: https://github.com/apache/ozone/pull/10536#issuecomment-4928486295
Performance tip from Google:
> If your application creates and discards thousands of buffers per second, use heap buffers. If your architecture relies on steady, high-throughput network frameworks (like [Netty](https://stackoverflow.com/questions/38638411/netty-4-1-iobuffer-v-s-directbuffer-v-s-heapbuffer)) or file channels, use a reusable pool of direct buffers to get the best of both worlds.
--
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-15592 use heap allocation in ChunkBuffer [ozone]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on PR #10536:
URL: https://github.com/apache/ozone/pull/10536#issuecomment-4928450313
> ... NioByteString and JVM use slower memory copy function ...
> ... gRPC/Netty wire buffer ...
For NioByteString (i.e. direct buffer), there is no buffer copying to gRPC/Netty wire buffer.
--
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-15592 use heap allocation in ChunkBuffer [ozone]
Posted by "yandrey321 (via GitHub)" <gi...@apache.org>.
yandrey321 commented on code in PR #10536:
URL: https://github.com/apache/ozone/pull/10536#discussion_r3464166955
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/ChunkBuffer.java:
##########
@@ -17,18 +17,30 @@
package org.apache.hadoop.ozone.common;
+import com.google.common.annotations.VisibleForTesting;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.channels.GatheringByteChannel;
import java.util.List;
import java.util.Objects;
+import java.util.concurrent.atomic.AtomicBoolean;
import org.apache.hadoop.hdds.utils.db.CodecBuffer;
import org.apache.ratis.thirdparty.com.google.protobuf.ByteString;
import org.apache.ratis.util.UncheckedAutoCloseable;
/** Buffer for a block chunk. */
public interface ChunkBuffer extends ChunkBufferToByteString, UncheckedAutoCloseable {
+ /**
+ * When true, {@link #allocate} uses direct ByteBuffers instead of heap ByteBuffers.
+ * Set only by {@link BlockOutputStreamWriteBenchmark} to compare allocation strategies
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-15592 use heap allocation in ChunkBuffer [ozone]
Posted by "yandrey321 (via GitHub)" <gi...@apache.org>.
yandrey321 commented on code in PR #10536:
URL: https://github.com/apache/ozone/pull/10536#discussion_r3443720583
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/ChunkBuffer.java:
##########
@@ -17,18 +17,30 @@
package org.apache.hadoop.ozone.common;
+import com.google.common.annotations.VisibleForTesting;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.channels.GatheringByteChannel;
import java.util.List;
import java.util.Objects;
+import java.util.concurrent.atomic.AtomicBoolean;
import org.apache.hadoop.hdds.utils.db.CodecBuffer;
import org.apache.ratis.thirdparty.com.google.protobuf.ByteString;
import org.apache.ratis.util.UncheckedAutoCloseable;
/** Buffer for a block chunk. */
public interface ChunkBuffer extends ChunkBufferToByteString, UncheckedAutoCloseable {
+ /**
+ * When true, {@link #allocate} uses direct ByteBuffers instead of heap ByteBuffers.
+ * Set only by {@link BlockOutputStreamWriteBenchmark} to compare allocation strategies
+ * within a single benchmark run; must never be modified in production code.
+ */
+ @VisibleForTesting
+ // CHECKSTYLE:OFF VisibilityModifier
+ AtomicBoolean ALLOCATE_DIRECT = new AtomicBoolean(false);
+ // CHECKSTYLE:ON VisibilityModifier
Review Comment:
this is a switch that is used in the benchmark to find out the diff between heap and direct allocations
--
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-15592 use heap allocation in ChunkBuffer [ozone]
Posted by "yandrey321 (via GitHub)" <gi...@apache.org>.
yandrey321 commented on PR #10536:
URL: https://github.com/apache/ozone/pull/10536#issuecomment-4938249397
> > Setup: the client write path with mocked container RPCs, ...
>
> @yandrey321 , Were you using Netty/gRPC or just a Mock?
the mock is for network communication, so no Netty, but protobuf serialization is not mocked. I saw the similar async profiler call stacks for freon testing in a sense of overhead of NioByteString to protobuf copy
--
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-15592 use heap allocation in ChunkBuffer [ozone]
Posted by "yandrey321 (via GitHub)" <gi...@apache.org>.
yandrey321 commented on PR #10536:
URL: https://github.com/apache/ozone/pull/10536#issuecomment-4928161923
> @yandrey321 , it seems that the benchmark is a mock but does not involve real network IO. If it is it the case, the improvement probably would become negative when network IO is involved.
The fix aims to decrease CPU consumption on the client side and improve theoretical end-to-end throughput. In case of slow disks the improvement would be less, but faster disks and less latency on the network side would produce better improvement.
Its a small low risk incremental fix.
--
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-15592 use heap allocation in ChunkBuffer [ozone]
Posted by "yandrey321 (via GitHub)" <gi...@apache.org>.
yandrey321 commented on code in PR #10536:
URL: https://github.com/apache/ozone/pull/10536#discussion_r3443697805
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/ChunkBuffer.java:
##########
@@ -17,18 +17,30 @@
package org.apache.hadoop.ozone.common;
+import com.google.common.annotations.VisibleForTesting;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.channels.GatheringByteChannel;
import java.util.List;
import java.util.Objects;
+import java.util.concurrent.atomic.AtomicBoolean;
import org.apache.hadoop.hdds.utils.db.CodecBuffer;
import org.apache.ratis.thirdparty.com.google.protobuf.ByteString;
import org.apache.ratis.util.UncheckedAutoCloseable;
/** Buffer for a block chunk. */
public interface ChunkBuffer extends ChunkBufferToByteString, UncheckedAutoCloseable {
+ /**
+ * When true, {@link #allocate} uses direct ByteBuffers instead of heap ByteBuffers.
+ * Set only by {@link BlockOutputStreamWriteBenchmark} to compare allocation strategies
+ * within a single benchmark run; must never be modified in production code.
+ */
+ @VisibleForTesting
+ // CHECKSTYLE:OFF VisibilityModifier
+ AtomicBoolean ALLOCATE_DIRECT = new AtomicBoolean(false);
+ // CHECKSTYLE:ON VisibilityModifier
Review Comment:
I wanted something lightweight for a benchmark that proves performance improvements.
--
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-15592 use heap allocation in ChunkBuffer [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #10536:
URL: https://github.com/apache/ozone/pull/10536#discussion_r3443692310
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/ChunkBuffer.java:
##########
@@ -17,18 +17,30 @@
package org.apache.hadoop.ozone.common;
+import com.google.common.annotations.VisibleForTesting;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.channels.GatheringByteChannel;
import java.util.List;
import java.util.Objects;
+import java.util.concurrent.atomic.AtomicBoolean;
import org.apache.hadoop.hdds.utils.db.CodecBuffer;
import org.apache.ratis.thirdparty.com.google.protobuf.ByteString;
import org.apache.ratis.util.UncheckedAutoCloseable;
/** Buffer for a block chunk. */
public interface ChunkBuffer extends ChunkBufferToByteString, UncheckedAutoCloseable {
+ /**
+ * When true, {@link #allocate} uses direct ByteBuffers instead of heap ByteBuffers.
+ * Set only by {@link BlockOutputStreamWriteBenchmark} to compare allocation strategies
+ * within a single benchmark run; must never be modified in production code.
+ */
+ @VisibleForTesting
+ // CHECKSTYLE:OFF VisibilityModifier
+ AtomicBoolean ALLOCATE_DIRECT = new AtomicBoolean(false);
+ // CHECKSTYLE:ON VisibilityModifier
Review Comment:
Please:
- avoid checkstyle suppression
- do not add `@VisibleForTesting`
--
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-15592 use heap allocation in ChunkBuffer [ozone]
Posted by "yandrey321 (via GitHub)" <gi...@apache.org>.
yandrey321 commented on PR #10536:
URL: https://github.com/apache/ozone/pull/10536#issuecomment-4928128628
> byte-by-byte NioByteString
Yes, in case of direct memory allocation we re getting NioByteString and JVM use slower memory copy function comparing that the one available for BoundedByteString which is used for heap allocated memory. Benchmark proofs better throughput for heap allocated buffers.
--
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-15592 use heap allocation in ChunkBuffer [ozone]
Posted by "ss77892 (via GitHub)" <gi...@apache.org>.
ss77892 commented on PR #10536:
URL: https://github.com/apache/ozone/pull/10536#issuecomment-4938079636
WARP_REGION="us-east-1"
WARP_ACCESS_KEY="minioadmin"
WARP_SECRET_KEY="minioadmin"
WARP_OBJECTS=3000
WARP_OBJ_SIZES="100mib"
WARP_CONCURRENT=10
WARP_DURATION="420s"
warp --no-color put --access-key "${WARP_ACCESS_KEY}" --secret-key "${WARP_SECRET_KEY}" --lookup path --host "${WARP_HOST}" --region "${WARP_REGION}" --obj.size "${WARP_OBJ_SIZES}" --concurrent "${WARP_CONCURRENT}" --duration "${WARP_DURATION}"
--
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-15592 use heap allocation in ChunkBuffer [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #10536:
URL: https://github.com/apache/ozone/pull/10536#discussion_r3443682839
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/ChunkBuffer.java:
##########
@@ -17,18 +17,30 @@
package org.apache.hadoop.ozone.common;
+import com.google.common.annotations.VisibleForTesting;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.channels.GatheringByteChannel;
import java.util.List;
import java.util.Objects;
+import java.util.concurrent.atomic.AtomicBoolean;
import org.apache.hadoop.hdds.utils.db.CodecBuffer;
import org.apache.ratis.thirdparty.com.google.protobuf.ByteString;
import org.apache.ratis.util.UncheckedAutoCloseable;
/** Buffer for a block chunk. */
public interface ChunkBuffer extends ChunkBufferToByteString, UncheckedAutoCloseable {
+ /**
+ * When true, {@link #allocate} uses direct ByteBuffers instead of heap ByteBuffers.
+ * Set only by {@link BlockOutputStreamWriteBenchmark} to compare allocation strategies
Review Comment:
```
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/ChunkBuffer.java:36: warning - Tag @link: reference not found: BlockOutputStreamWriteBenchmark
```
https://github.com/yandrey321/ozone/actions/runs/27780341047/job/82205691733#step:13:2213
--
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-15592 use heap allocation in ChunkBuffer [ozone]
Posted by "errose28 (via GitHub)" <gi...@apache.org>.
errose28 commented on code in PR #10536:
URL: https://github.com/apache/ozone/pull/10536#discussion_r3454314249
##########
hadoop-hdds/client/dev-support/run-block-output-stream-write-benchmark.sh:
##########
Review Comment:
In this PR the benchmark is a shell script but in #10546 it is a java class. We should be consistent with how we create benchmarks and where we put them.
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/ChunkBuffer.java:
##########
@@ -17,18 +17,30 @@
package org.apache.hadoop.ozone.common;
+import com.google.common.annotations.VisibleForTesting;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.channels.GatheringByteChannel;
import java.util.List;
import java.util.Objects;
+import java.util.concurrent.atomic.AtomicBoolean;
import org.apache.hadoop.hdds.utils.db.CodecBuffer;
import org.apache.ratis.thirdparty.com.google.protobuf.ByteString;
import org.apache.ratis.util.UncheckedAutoCloseable;
/** Buffer for a block chunk. */
public interface ChunkBuffer extends ChunkBufferToByteString, UncheckedAutoCloseable {
+ /**
+ * When true, {@link #allocate} uses direct ByteBuffers instead of heap ByteBuffers.
+ * Set only by {@link BlockOutputStreamWriteBenchmark} to compare allocation strategies
+ * within a single benchmark run; must never be modified in production code.
+ */
+ @VisibleForTesting
+ // CHECKSTYLE:OFF VisibilityModifier
+ AtomicBoolean ALLOCATE_DIRECT = new AtomicBoolean(false);
+ // CHECKSTYLE:ON VisibilityModifier
Review Comment:
Why should it be configurable at all? The purpose of the benchmarks is to produce evidence so we can make a conclusive decision.
--
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-15592 use heap allocation in ChunkBuffer [ozone]
Posted by "rnblough (via GitHub)" <gi...@apache.org>.
rnblough commented on code in PR #10536:
URL: https://github.com/apache/ozone/pull/10536#discussion_r3461899206
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/ChunkBuffer.java:
##########
@@ -17,18 +17,30 @@
package org.apache.hadoop.ozone.common;
+import com.google.common.annotations.VisibleForTesting;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.channels.GatheringByteChannel;
import java.util.List;
import java.util.Objects;
+import java.util.concurrent.atomic.AtomicBoolean;
import org.apache.hadoop.hdds.utils.db.CodecBuffer;
import org.apache.ratis.thirdparty.com.google.protobuf.ByteString;
import org.apache.ratis.util.UncheckedAutoCloseable;
/** Buffer for a block chunk. */
public interface ChunkBuffer extends ChunkBufferToByteString, UncheckedAutoCloseable {
+ /**
+ * When true, {@link #allocate} uses direct ByteBuffers instead of heap ByteBuffers.
+ * Set only by {@link BlockOutputStreamWriteBenchmark} to compare allocation strategies
+ * within a single benchmark run; must never be modified in production code.
+ */
+ @VisibleForTesting
+ // CHECKSTYLE:OFF VisibilityModifier
+ AtomicBoolean ALLOCATE_DIRECT = new AtomicBoolean(false);
+ // CHECKSTYLE:ON VisibilityModifier
Review Comment:
I endorse the idea of an ozone-dev style public repo for POC, benchmarks, & profiling; it would be particularly great if we could keep reference to the benchmark and profiling results themselves for later development. But if the second repo is also Apache, does that let us off the GPL hook?
--
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-15592 use heap allocation in ChunkBuffer [ozone]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on PR #10536:
URL: https://github.com/apache/ozone/pull/10536#issuecomment-4938215987
> Setup: the client write path with mocked container RPCs, ...
Were you using Netty/gRPC or just a Mock?
--
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-15592 use heap allocation in ChunkBuffer [ozone]
Posted by "yandrey321 (via GitHub)" <gi...@apache.org>.
yandrey321 commented on PR #10536:
URL: https://github.com/apache/ozone/pull/10536#issuecomment-4937321161
> > ... NioByteString and JVM use slower memory copy function ...
>
> > ... gRPC/Netty wire buffer ...
>
> For NioByteString (i.e. direct buffer), there is no buffer copying to gRPC/Netty wire buffer.
Just checked async-profiler to check, and the data shows a copy does happen for NioByteString — in fact two copies, versus one for the heap-backed BoundedByteString.
Setup: the client write path with mocked container RPCs, where the request is serialized via request.writeTo(...) into a pooled direct Netty ByteBuf — the same OutputStreamEncoder + framing gRPC's MessageFramer uses. Same benchmark run twice, only flipping ChunkBuffer allocation (allocateDirect → NioByteString vs allocateHeap → BoundedByteString). CPU event, 1 MB writes, 2 threads.
Direct / NioByteString.writeTo — 15.7% of total CPU:
```
CodedOutputStream$OutputStreamEncoder.writeTo
└─ NioByteString.writeTo
├─ slice → slice → <init>×4 → jbyte_disjoint_arraycopy # copy #1: off-heap → temp heap byte[]
└─ writeLazy → ByteBufOutputStream.write
→ AbstractByteBuf.setBytes → Unsafe.copyMemory # copy #2: heap → direct wire ByteBuf
```
Heap / BoundedByteString(LiteralByteString).writeTo — 0.7% of total CPU:
```
CodedOutputStream$OutputStreamEncoder.writeTo
└─ LiteralByteString.writeTo → writeLazy → ByteBufOutputStream.write
→ AbstractByteBuf.setBytes → Unsafe.copyMemory # single copy: backing byte[] → wire ByteBuf
```
Both paths terminate in setBytes → Unsafe.copyMemory into the Netty wire buffer, so neither is zero-copy here. The difference is that OutputStreamEncoder cannot bulk-transfer an array-less (direct) buffer, so for NioByteString it first stages the off-heap bytes into a temporary heap buffer (slice/arraycopy) and only then copies into the pooled direct ByteBuf. That extra staging copy is why the NioByteString serialization path costs ~22× more CPU (15.7% vs 0.7% of total), which lines up with the ~11% end-to-end throughput difference I measured (10,409 vs 11,637 MB/s).
So the direct-buffer path isn't avoiding the wire-buffer copy — it's adding one.
--
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-15592 use heap allocation in ChunkBuffer [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #10536:
URL: https://github.com/apache/ozone/pull/10536#discussion_r3445713538
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/ChunkBuffer.java:
##########
@@ -17,18 +17,30 @@
package org.apache.hadoop.ozone.common;
+import com.google.common.annotations.VisibleForTesting;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.channels.GatheringByteChannel;
import java.util.List;
import java.util.Objects;
+import java.util.concurrent.atomic.AtomicBoolean;
import org.apache.hadoop.hdds.utils.db.CodecBuffer;
import org.apache.ratis.thirdparty.com.google.protobuf.ByteString;
import org.apache.ratis.util.UncheckedAutoCloseable;
/** Buffer for a block chunk. */
public interface ChunkBuffer extends ChunkBufferToByteString, UncheckedAutoCloseable {
+ /**
+ * When true, {@link #allocate} uses direct ByteBuffers instead of heap ByteBuffers.
+ * Set only by {@link BlockOutputStreamWriteBenchmark} to compare allocation strategies
+ * within a single benchmark run; must never be modified in production code.
+ */
+ @VisibleForTesting
+ // CHECKSTYLE:OFF VisibilityModifier
+ AtomicBoolean ALLOCATE_DIRECT = new AtomicBoolean(false);
+ // CHECKSTYLE:ON VisibilityModifier
Review Comment:
Making it configurable in prod is not a big change: https://github.com/adoroszlai/ozone/commit/79cbbf6ca0528d0944d2da8a98a69436644ca8e3
--
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-15592 use heap allocation in ChunkBuffer [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #10536:
URL: https://github.com/apache/ozone/pull/10536#discussion_r3461983409
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/ChunkBuffer.java:
##########
@@ -17,18 +17,30 @@
package org.apache.hadoop.ozone.common;
+import com.google.common.annotations.VisibleForTesting;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.channels.GatheringByteChannel;
import java.util.List;
import java.util.Objects;
+import java.util.concurrent.atomic.AtomicBoolean;
import org.apache.hadoop.hdds.utils.db.CodecBuffer;
import org.apache.ratis.thirdparty.com.google.protobuf.ByteString;
import org.apache.ratis.util.UncheckedAutoCloseable;
/** Buffer for a block chunk. */
public interface ChunkBuffer extends ChunkBufferToByteString, UncheckedAutoCloseable {
+ /**
+ * When true, {@link #allocate} uses direct ByteBuffers instead of heap ByteBuffers.
+ * Set only by {@link BlockOutputStreamWriteBenchmark} to compare allocation strategies
+ * within a single benchmark run; must never be modified in production code.
+ */
+ @VisibleForTesting
+ // CHECKSTYLE:OFF VisibilityModifier
+ AtomicBoolean ALLOCATE_DIRECT = new AtomicBoolean(false);
+ // CHECKSTYLE:ON VisibilityModifier
Review Comment:
> But if the second repo is also Apache, does that let us off the GPL hook?
Yes, since we would not be distributing (releasing) code from that repo.
https://www.apache.org/legal/resolved.html
--
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-15592 use heap allocation in ChunkBuffer [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #10536:
URL: https://github.com/apache/ozone/pull/10536#discussion_r3454883413
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/ChunkBuffer.java:
##########
@@ -17,18 +17,30 @@
package org.apache.hadoop.ozone.common;
+import com.google.common.annotations.VisibleForTesting;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.channels.GatheringByteChannel;
import java.util.List;
import java.util.Objects;
+import java.util.concurrent.atomic.AtomicBoolean;
import org.apache.hadoop.hdds.utils.db.CodecBuffer;
import org.apache.ratis.thirdparty.com.google.protobuf.ByteString;
import org.apache.ratis.util.UncheckedAutoCloseable;
/** Buffer for a block chunk. */
public interface ChunkBuffer extends ChunkBufferToByteString, UncheckedAutoCloseable {
+ /**
+ * When true, {@link #allocate} uses direct ByteBuffers instead of heap ByteBuffers.
+ * Set only by {@link BlockOutputStreamWriteBenchmark} to compare allocation strategies
+ * within a single benchmark run; must never be modified in production code.
+ */
+ @VisibleForTesting
+ // CHECKSTYLE:OFF VisibilityModifier
+ AtomicBoolean ALLOCATE_DIRECT = new AtomicBoolean(false);
+ // CHECKSTYLE:ON VisibilityModifier
Review Comment:
The benchmark needs to test various implementations. Adding the benchmark code to the `master` branch in the main Ozone repo requires that those implementations also exist there. Choosing between the implementations using an internal toggle that even requires `CHECKSTYLE:OFF`-style hacks is ugly.
Results may depend on hardware/software environment. Providing this simple toggle configuration does not add much complexity, but enables users to test in their own environment if needed.
I would prefer adding performance tests in a separate repo:
- various POC implementations can be freely added without affecting production code
- JMH can be used without license problems (HDDS-6202)
- can test different versions of Ozone
- performance tests would not be confused with functional tests (see `TestUnhealthyContainersDerbyPerformance`)
--
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-15592 use heap allocation in ChunkBuffer [ozone]
Posted by "yandrey321 (via GitHub)" <gi...@apache.org>.
yandrey321 commented on PR #10536:
URL: https://github.com/apache/ozone/pull/10536#issuecomment-4936394608
> Performance tip from Google:
>
> > If your application creates and discards thousands of buffers per second, use heap buffers. If your architecture relies on steady, high-throughput network frameworks (like [Netty](https://stackoverflow.com/questions/38638411/netty-4-1-iobuffer-v-s-directbuffer-v-s-heapbuffer)) or file channels, use a reusable pool of direct buffers to get the best of both worlds.
@szetszwo the benchmark clearly shows that in Ozone context heap allocation provides better throughput.
--
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-15592 use heap allocation in ChunkBuffer [ozone]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on PR #10536:
URL: https://github.com/apache/ozone/pull/10536#issuecomment-4938421153
> the mock is for network communication, so no Netty, but protobuf serialization is not mocked. I saw the similar async profiler call stacks for freon testing in a sense of overhead of NioByteString to protobuf copy
Please try with netty. Testing with a mock cannot see the real thing.
As @ss77892 mentioned, there is a significant performance degradation when using heap buffers.
--
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