From 2bf5e46a33af9724f4db0376a70a217b5071fc6f Mon Sep 17 00:00:00 2001
From: Michael Stack <stack@apache.org>
Date: Wed, 11 Jul 2018 22:40:04 -0400
Subject: [PATCH] HBASE-20875 MemStoreLABImp::copyIntoCell uses 7% CPU when
 writing

Make the #copyCellInto method smaller so it inlines; we do it by
checking for the common type early and then taking a code path
that presumes ByteBufferExtendedCell -- avoids checks.
---
 .../hbase/regionserver/MemStoreLABImpl.java   | 69 ++++++++++++++++++-
 1 file changed, 66 insertions(+), 3 deletions(-)

diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreLABImpl.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreLABImpl.java
index ce775f8ff60..6f1ee92a15a 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreLABImpl.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreLABImpl.java
@@ -29,6 +29,7 @@ import java.util.concurrent.atomic.AtomicReference;
 import java.util.concurrent.locks.ReentrantLock;
 
 import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.ByteBufferExtendedCell;
 import org.apache.hadoop.hbase.Cell;
 import org.apache.hadoop.hbase.ExtendedCell;
 import org.apache.hadoop.hbase.KeyValueUtil;
@@ -108,7 +109,10 @@ public class MemStoreLABImpl implements MemStoreLAB {
 
   @Override
   public Cell copyCellInto(Cell cell) {
-    return copyCellInto(cell, maxAlloc);
+    // See head of copyBBECellInto for how it differs from copyCellInto
+    return (cell instanceof ByteBufferExtendedCell)?
+        copyBBECellInto((ByteBufferExtendedCell)cell, maxAlloc):
+        copyCellInto(cell, maxAlloc);
   }
 
   /**
@@ -133,6 +137,47 @@ public class MemStoreLABImpl implements MemStoreLAB {
     }
   }
 
+  /**
+   * Mostly a duplicate of {@link #copyCellInto(Cell, int)}} done for perf sake. It presumes
+   * ByteBufferExtendedCell instead of Cell so we deal with a specific type rather than the
+   * super generic Cell. Removes instanceof checks. Shrinkage is enough to make this inline where
+   * before it was too big. Uses less CPU. See HBASE-20875 for evidence.
+   * @see #copyCellInto(Cell, int)
+   */
+  private Cell copyBBECellInto(ByteBufferExtendedCell cell, int maxAlloc) {
+    int size = cell.getSerializedSize();
+    Preconditions.checkArgument(size >= 0, "negative size");
+    // Callers should satisfy large allocations from JVM heap so limit fragmentation.
+    if (size > maxAlloc) {
+      return null;
+    }
+    Chunk c = null;
+    int allocOffset = 0;
+    while (true) {
+      // Try to get the chunk
+      c = getOrMakeChunk();
+      // We may get null because the some other thread succeeded in getting the lock
+      // and so the current thread has to try again to make its chunk or grab the chunk
+      // that the other thread created
+      // Try to allocate from this chunk
+      if (c != null) {
+        allocOffset = c.alloc(size);
+        if (allocOffset != -1) {
+          // We succeeded - this is the common case - small alloc
+          // from a big buffer
+          break;
+        }
+        // not enough space!
+        // try to retire this chunk
+        tryRetireChunk(c);
+      }
+    }
+    return copyBBECToChunkCell(cell, c.getData(), allocOffset, size);
+  }
+
+  /**
+   * @see #copyBBECellInto(ByteBufferExtendedCell, int)
+   */
   private Cell copyCellInto(Cell cell, int maxAlloc) {
     int size = Segment.getCellLength(cell);
     Preconditions.checkArgument(size >= 0, "negative size");
@@ -168,6 +213,7 @@ public class MemStoreLABImpl implements MemStoreLAB {
   /**
    * Clone the passed cell by copying its data into the passed buf and create a cell with a chunkid
    * out of it
+   * @see #copyBBECToChunkCell(ByteBufferExtendedCell, ByteBuffer, int, int)
    */
   private static Cell copyToChunkCell(Cell cell, ByteBuffer buf, int offset, int len) {
     int tagsLen = cell.getTagsLength();
@@ -179,6 +225,23 @@ public class MemStoreLABImpl implements MemStoreLAB {
       // serialization format only.
       KeyValueUtil.appendTo(cell, buf, offset, true);
     }
+    return createChunkCell(buf, offset, len, tagsLen, cell.getSequenceId());
+  }
+
+  /**
+   * Clone the passed cell by copying its data into the passed buf and create a cell with a chunkid
+   * out of it
+   * @see #copyToChunkCell(Cell, ByteBuffer, int, int)
+   */
+  private static Cell copyBBECToChunkCell(ByteBufferExtendedCell cell, ByteBuffer buf, int offset,
+      int len) {
+    int tagsLen = cell.getTagsLength();
+    cell.write(buf, offset);
+    return createChunkCell(buf, offset, len, tagsLen, cell.getSequenceId());
+  }
+
+  private static Cell createChunkCell(ByteBuffer buf, int offset, int len, int tagsLen,
+      long sequenceId) {
     // TODO : write the seqid here. For writing seqId we should create a new cell type so
     // that seqId is not used as the state
     if (tagsLen == 0) {
@@ -186,9 +249,9 @@ public class MemStoreLABImpl implements MemStoreLAB {
       // which directly return tagsLen as 0. So we avoid parsing many length components in
       // reading the tagLength stored in the backing buffer. The Memstore addition of every Cell
       // call getTagsLength().
-      return new NoTagByteBufferChunkKeyValue(buf, offset, len, cell.getSequenceId());
+      return new NoTagByteBufferChunkKeyValue(buf, offset, len, sequenceId);
     } else {
-      return new ByteBufferChunkKeyValue(buf, offset, len, cell.getSequenceId());
+      return new ByteBufferChunkKeyValue(buf, offset, len, sequenceId);
     }
   }