You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by nk...@apache.org on 2013/11/07 00:19:56 UTC

svn commit: r1539492 - in /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf: ProtobufUtil.java RequestConverter.java

Author: nkeywal
Date: Wed Nov  6 23:19:56 2013
New Revision: 1539492

URL: http://svn.apache.org/r1539492
Log:
HBASE-9885 Avoid some Result creation in protobuf conversions - REVERT to check the cause of precommit flakiness

Modified:
    hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
    hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java

Modified: hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
URL: http://svn.apache.org/viewvc/hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java?rev=1539492&r1=1539491&r2=1539492&view=diff
==============================================================================
--- hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java (original)
+++ hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java Wed Nov  6 23:19:56 2013
@@ -154,35 +154,6 @@ public final class ProtobufUtil {
   private final static Map<String, Class<?>>
     PRIMITIVES = new HashMap<String, Class<?>>();
 
-
-  /**
-   * Many results are simple: no cell, exists true or false. To save on object creations,
-   *  we reuse them across calls.
-   */
-  private final static Cell[] EMPTY_CELL_ARRAY = new Cell[]{};
-  private final static Result EMPTY_RESULT = Result.create(EMPTY_CELL_ARRAY);
-  private final static Result EMPTY_RESULT_EXISTS_TRUE = Result.create(null, true);
-  private final static Result EMPTY_RESULT_EXISTS_FALSE = Result.create(null, false);
-
-  private final static ClientProtos.Result EMPTY_RESULT_PB;
-  private final static ClientProtos.Result EMPTY_RESULT_PB_EXISTS_TRUE;
-  private final static ClientProtos.Result EMPTY_RESULT_PB_EXISTS_FALSE;
-
-  static {
-    ClientProtos.Result.Builder builder = ClientProtos.Result.newBuilder();
-
-    builder.setExists(true);
-    EMPTY_RESULT_PB_EXISTS_TRUE =  builder.build();
-
-    builder.clear();
-    builder.setExists(false);
-    EMPTY_RESULT_PB_EXISTS_FALSE =  builder.build();
-
-    builder.clear();
-    builder.setAssociatedCellCount(0);
-    EMPTY_RESULT_PB =  builder.build();
-  }
-
   /**
    * Dynamic class loader to load filter/comparators
    */
@@ -1112,20 +1083,16 @@ public final class ProtobufUtil {
    * @return the converted protocol buffer Result
    */
   public static ClientProtos.Result toResult(final Result result) {
-    if (result.getExists() != null) {
-      return toResult(result.getExists());
-    }
-
-    Cell[] cells = result.rawCells();
-    if (cells == null || cells.length == 0) {
-      return EMPTY_RESULT_PB;
-    }
-
     ClientProtos.Result.Builder builder = ClientProtos.Result.newBuilder();
-    for (Cell c : cells) {
-      builder.addCell(toCell(c));
+    Cell [] cells = result.rawCells();
+    if (cells != null) {
+      for (Cell c : cells) {
+        builder.addCell(toCell(c));
+      }
+    }
+    if (result.getExists() != null){
+      builder.setExists(result.getExists());
     }
-
     return builder.build();
   }
 
@@ -1136,7 +1103,9 @@ public final class ProtobufUtil {
    * @return the converted protocol buffer Result
    */
   public static ClientProtos.Result toResult(final boolean existence) {
-    return existence ? EMPTY_RESULT_PB_EXISTS_TRUE : EMPTY_RESULT_PB_EXISTS_FALSE;
+    ClientProtos.Result.Builder builder = ClientProtos.Result.newBuilder();
+    builder.setExists(existence);
+    return builder.build();
   }
 
   /**
@@ -1147,19 +1116,11 @@ public final class ProtobufUtil {
    * @return the converted protocol buffer Result
    */
   public static ClientProtos.Result toResultNoData(final Result result) {
+    ClientProtos.Result.Builder builder = ClientProtos.Result.newBuilder();
+    builder.setAssociatedCellCount(result.size());
     if (result.getExists() != null){
-      return result.getExists() ? EMPTY_RESULT_PB_EXISTS_TRUE : EMPTY_RESULT_PB_EXISTS_FALSE;
+      builder.setExists(result.getExists());
     }
-
-    int size = result.size();
-
-    if (size == 0){
-      return EMPTY_RESULT_PB;
-    }
-
-    ClientProtos.Result.Builder builder = ClientProtos.Result.newBuilder();
-    builder.setAssociatedCellCount(size);
-
     return builder.build();
   }
 
@@ -1171,14 +1132,10 @@ public final class ProtobufUtil {
    */
   public static Result toResult(final ClientProtos.Result proto) {
     if (proto.hasExists()) {
-      return proto.getExists() ? EMPTY_RESULT_EXISTS_TRUE : EMPTY_RESULT_EXISTS_FALSE;
+      return Result.create(null, proto.getExists());
     }
 
     List<CellProtos.Cell> values = proto.getCellList();
-    if (values.isEmpty()){
-      return EMPTY_RESULT;
-    }
-
     List<Cell> cells = new ArrayList<Cell>(values.size());
     for (CellProtos.Cell c : values) {
       cells.add(toCell(c));
@@ -1197,7 +1154,7 @@ public final class ProtobufUtil {
   public static Result toResult(final ClientProtos.Result proto, final CellScanner scanner)
   throws IOException {
     if (proto.hasExists()){
-      return proto.getExists() ? EMPTY_RESULT_EXISTS_TRUE : EMPTY_RESULT_EXISTS_FALSE;
+      return Result.create(null, proto.getExists());
     }
 
     // TODO: Unit test that has some Cells in scanner and some in the proto.
@@ -1211,20 +1168,13 @@ public final class ProtobufUtil {
       }
     }
     List<CellProtos.Cell> values = proto.getCellList();
-    if (cells == null) {
-      if (values.isEmpty()) {
-        return EMPTY_RESULT;
-      } else {
-        cells = new ArrayList<Cell>(values.size());
-        for (CellProtos.Cell c : values) {
-          cells.add(toCell(c));
-        }
-      }
+    if (cells == null) cells = new ArrayList<Cell>(values.size());
+    for (CellProtos.Cell c: values) {
+      cells.add(toCell(c));
     }
     return Result.create(cells, null);
   }
 
-
   /**
    * Convert a ByteArrayComparable to a protocol buffer Comparator
    *

Modified: hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java
URL: http://svn.apache.org/viewvc/hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java?rev=1539492&r1=1539491&r2=1539492&view=diff
==============================================================================
--- hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java (original)
+++ hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java Wed Nov  6 23:19:56 2013
@@ -488,6 +488,7 @@ public final class RequestConverter {
   throws IOException {
     RegionAction.Builder builder = getRegionActionBuilderWithRegion(regionName);
     ClientProtos.Action.Builder actionBuilder = ClientProtos.Action.newBuilder();
+    MutationProto.Builder mutationBuilder = ClientProtos.MutationProto.newBuilder();
     for (Action<R> action: actions) {
       Row row = action.getAction();
       actionBuilder.clear();