[GitHub] [hbase] Apache9 commented on a change in pull request #3964: HBASE-26604 Replace new allocation with ThreadLocal in CellBlockBuilder to reduce GC

2021-12-22 Thread GitBox 


Apache9 commented on a change in pull request #3964:
URL: https://github.com/apache/hbase/pull/3964#discussion_r774314711



##
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:
   OK, should be closed as later or won't 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-unsubscr...@hbase.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [hbase] Apache9 commented on a change in pull request #3964: HBASE-26604 Replace new allocation with ThreadLocal in CellBlockBuilder to reduce GC

2021-12-20 Thread GitBox 


Apache9 commented on a change in pull request #3964:
URL: https://github.com/apache/hbase/pull/3964#discussion_r772849330



##
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:
   This will leak the internal ByteBuffer output? What if the upper layer 
cache this ByteBuffer and pass it to other threads...




-- 
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