[GitHub] [hbase] YutSean commented on a change in pull request #3964: HBASE-26604 Replace new allocation with ThreadLocal in CellBlockBuilder to reduce GC
YutSean commented on a change in pull request #3964:
URL: https://github.com/apache/hbase/pull/3964#discussion_r772866871
##
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/CellBlockBuilder.java
##
@@ -294,14 +296,15 @@ private ByteBuffer decompress(CompressionCodec
compressor, InputStream cellBlock
}
Decompressor poolDecompressor = CodecPool.getDecompressor(compressor);
CompressionInputStream cis = compressor.createInputStream(cellBlockStream,
poolDecompressor);
-ByteBufferOutputStream bbos;
try {
// TODO: This is ugly. The buffer will be resized on us if we guess
wrong.
- // TODO: Reuse buffers.
- bbos = new ByteBufferOutputStream(osInitialSize);
- IOUtils.copy(cis, bbos);
- bbos.close();
- return bbos.getByteBuffer();
+ if (this.decompressBuff.get() == null) {
+this.decompressBuff.set(new ByteBufferOutputStream(osInitialSize));
+ }
+ ByteBufferOutputStream localBbos = this.decompressBuff.get();
+ localBbos.clear();
+ IOUtils.copy(cis, localBbos);
+ return localBbos.getByteBuffer();
Review comment:
It seems that there is no better solution except returning a copy of the
underlying bytebuffer. But copy is not a reuse of buffers. Currently, the
threadlocal is not a good idea here... (this should be reconsidered). May I
close this PR and the related ticket ? @Apache9
--
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-unsubscr...@hbase.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [hbase] YutSean commented on a change in pull request #3964: HBASE-26604 Replace new allocation with ThreadLocal in CellBlockBuilder to reduce GC
YutSean commented on a change in pull request #3964:
URL: https://github.com/apache/hbase/pull/3964#discussion_r772866871
##
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/CellBlockBuilder.java
##
@@ -294,14 +296,15 @@ private ByteBuffer decompress(CompressionCodec
compressor, InputStream cellBlock
}
Decompressor poolDecompressor = CodecPool.getDecompressor(compressor);
CompressionInputStream cis = compressor.createInputStream(cellBlockStream,
poolDecompressor);
-ByteBufferOutputStream bbos;
try {
// TODO: This is ugly. The buffer will be resized on us if we guess
wrong.
- // TODO: Reuse buffers.
- bbos = new ByteBufferOutputStream(osInitialSize);
- IOUtils.copy(cis, bbos);
- bbos.close();
- return bbos.getByteBuffer();
+ if (this.decompressBuff.get() == null) {
+this.decompressBuff.set(new ByteBufferOutputStream(osInitialSize));
+ }
+ ByteBufferOutputStream localBbos = this.decompressBuff.get();
+ localBbos.clear();
+ IOUtils.copy(cis, localBbos);
+ return localBbos.getByteBuffer();
Review comment:
It seems that there is no better solution than returning a copy of the
underlying bytebuffer. But copy is not a reuse of buffers. Currently, the
threadlocal is not a good idea here... (this should be reconsidered). May I
close this PR and the related ticket ? @Apache9
--
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-unsubscr...@hbase.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [hbase] YutSean commented on a change in pull request #3964: HBASE-26604 Replace new allocation with ThreadLocal in CellBlockBuilder to reduce GC
YutSean commented on a change in pull request #3964:
URL: https://github.com/apache/hbase/pull/3964#discussion_r772854823
##
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/CellBlockBuilder.java
##
@@ -294,14 +296,15 @@ private ByteBuffer decompress(CompressionCodec
compressor, InputStream cellBlock
}
Decompressor poolDecompressor = CodecPool.getDecompressor(compressor);
CompressionInputStream cis = compressor.createInputStream(cellBlockStream,
poolDecompressor);
-ByteBufferOutputStream bbos;
try {
// TODO: This is ugly. The buffer will be resized on us if we guess
wrong.
- // TODO: Reuse buffers.
- bbos = new ByteBufferOutputStream(osInitialSize);
- IOUtils.copy(cis, bbos);
- bbos.close();
- return bbos.getByteBuffer();
+ if (this.decompressBuff.get() == null) {
+this.decompressBuff.set(new ByteBufferOutputStream(osInitialSize));
+ }
+ ByteBufferOutputStream localBbos = this.decompressBuff.get();
+ localBbos.clear();
+ IOUtils.copy(cis, localBbos);
+ return localBbos.getByteBuffer();
Review comment:
Ah.That is really a problem. Let me think how to deal with this
here...
--
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-unsubscr...@hbase.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
