From 06758bf630a2dd6ab0e65b048b0b6b8c500570bc Mon Sep 17 00:00:00 2001 From: anoopsamjohn Date: Thu, 6 Oct 2016 00:27:34 +0530 Subject: [PATCH] HBASE-16759 Avoid ByteString.copyFrom usage wherever possible. --- .../client/replication/ReplicationSerDeHelper.java | 5 +++-- .../apache/hadoop/hbase/ipc/CoprocessorRpcUtils.java | 7 ++++--- .../hadoop/hbase/shaded/protobuf/RequestConverter.java | 4 ++-- .../org/apache/hadoop/hbase/procedure2/Procedure.java | 4 ++-- .../apache/hadoop/hbase/master/MasterRpcServices.java | 2 +- .../org/apache/hadoop/hbase/master/ServerManager.java | 3 ++- .../org/apache/hadoop/hbase/regionserver/HRegion.java | 5 ++--- .../hadoop/hbase/regionserver/RSRpcServices.java | 10 ++-------- .../hadoop/hbase/regionserver/wal/WALCellCodec.java | 4 ++++ .../main/java/org/apache/hadoop/hbase/wal/WALKey.java | 9 +++++---- 10 files changed, 27 insertions(+), 26 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationSerDeHelper.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationSerDeHelper.java index 25900218bea..6ac441738f8 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationSerDeHelper.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationSerDeHelper.java @@ -20,6 +20,7 @@ package org.apache.hadoop.hbase.client.replication; import org.apache.commons.lang.StringUtils; import org.apache.hadoop.hbase.shaded.com.google.protobuf.ByteString; +import org.apache.hadoop.hbase.shaded.com.google.protobuf.UnsafeByteOperations; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.hadoop.hbase.TableName; @@ -300,8 +301,8 @@ public final class ReplicationSerDeHelper { for (Map.Entry entry : peerConfig.getPeerData().entrySet()) { builder.addData(HBaseProtos.BytesBytesPair.newBuilder() - .setFirst(ByteString.copyFrom(entry.getKey())) - .setSecond(ByteString.copyFrom(entry.getValue())) + .setFirst(UnsafeByteOperations.unsafeWrap(entry.getKey())) + .setSecond(UnsafeByteOperations.unsafeWrap(entry.getValue())) .build()); } diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/CoprocessorRpcUtils.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/CoprocessorRpcUtils.java index 9dfa3346187..dc5f122447f 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/CoprocessorRpcUtils.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/CoprocessorRpcUtils.java @@ -30,6 +30,7 @@ import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.hbase.exceptions.UnknownProtocolException; import org.apache.hadoop.hbase.protobuf.generated.ClientProtos; +import org.apache.hadoop.hbase.shaded.com.google.protobuf.UnsafeByteOperations; import org.apache.hadoop.hbase.shaded.protobuf.RequestConverter; import org.apache.hadoop.hbase.shaded.protobuf.generated.ClientProtos.CoprocessorServiceCall; import org.apache.hadoop.hbase.shaded.protobuf.generated.ClientProtos.CoprocessorServiceRequest; @@ -97,13 +98,13 @@ public final class CoprocessorRpcUtils { private static CoprocessorServiceCall getCoprocessorServiceCall( final Descriptors.MethodDescriptor method, final Message request, final byte [] row) { return CoprocessorServiceCall.newBuilder() - .setRow(org.apache.hadoop.hbase.shaded.com.google.protobuf.ByteString.copyFrom(row)) + .setRow(org.apache.hadoop.hbase.shaded.com.google.protobuf.UnsafeByteOperations.unsafeWrap(row)) .setServiceName(CoprocessorRpcUtils.getServiceName(method.getService())) .setMethodName(method.getName()) // TODO!!!!! Come back here after!!!!! This is a double copy of the request if I read // it right copying from non-shaded to shaded version!!!!!! FIXXXXX!!!!! - .setRequest(org.apache.hadoop.hbase.shaded.com.google.protobuf.ByteString. - copyFrom(request.toByteArray())).build(); + .setRequest(org.apache.hadoop.hbase.shaded.com.google.protobuf.UnsafeByteOperations. + unsafeWrap(request.toByteArray())).build(); } public static MethodDescriptor getMethodDescriptor(final String methodName, diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/RequestConverter.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/RequestConverter.java index 3f91ee0e855..4c8a21ce0cd 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/RequestConverter.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/RequestConverter.java @@ -615,7 +615,7 @@ public final class RequestConverter { RegionCoprocessorServiceExec exec = (RegionCoprocessorServiceExec) row; // DUMB COPY!!! FIX!!! Done to copy from c.g.p.ByteString to shaded ByteString. org.apache.hadoop.hbase.shaded.com.google.protobuf.ByteString value = - org.apache.hadoop.hbase.shaded.com.google.protobuf.ByteString.copyFrom( + org.apache.hadoop.hbase.shaded.com.google.protobuf.UnsafeByteOperations.unsafeWrap( exec.getRequest().toByteArray()); regionActionBuilder.addAction(actionBuilder.setServiceCall( ClientProtos.CoprocessorServiceCall.newBuilder() @@ -698,7 +698,7 @@ public final class RequestConverter { RegionCoprocessorServiceExec exec = (RegionCoprocessorServiceExec) row; // DUMB COPY!!! FIX!!! Done to copy from c.g.p.ByteString to shaded ByteString. org.apache.hadoop.hbase.shaded.com.google.protobuf.ByteString value = - org.apache.hadoop.hbase.shaded.com.google.protobuf.ByteString.copyFrom( + org.apache.hadoop.hbase.shaded.com.google.protobuf.UnsafeByteOperations.unsafeWrap( exec.getRequest().toByteArray()); builder.addAction(actionBuilder.setServiceCall( ClientProtos.CoprocessorServiceCall.newBuilder() diff --git a/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/Procedure.java b/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/Procedure.java index 84328a7b1ff..dd10a913e12 100644 --- a/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/Procedure.java +++ b/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/Procedure.java @@ -28,7 +28,6 @@ import java.util.List; import java.util.Map; import org.apache.hadoop.hbase.HConstants; -import org.apache.hadoop.hbase.ProcedureInfo; import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.hbase.classification.InterfaceStability; import org.apache.hadoop.hbase.exceptions.TimeoutIOException; @@ -36,6 +35,7 @@ import org.apache.hadoop.hbase.procedure2.util.StringUtils; import org.apache.hadoop.hbase.shaded.protobuf.generated.ProcedureProtos; import org.apache.hadoop.hbase.shaded.protobuf.generated.ProcedureProtos.ProcedureState; import org.apache.hadoop.hbase.shaded.com.google.protobuf.ByteString; +import org.apache.hadoop.hbase.shaded.com.google.protobuf.UnsafeByteOperations; import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; import org.apache.hadoop.hbase.util.NonceKey; @@ -815,7 +815,7 @@ public abstract class Procedure implements Comparable { byte[] result = proc.getResult(); if (result != null) { - builder.setResult(ByteString.copyFrom(result)); + builder.setResult(UnsafeByteOperations.unsafeWrap(result)); } ByteString.Output stateStream = ByteString.newOutput(); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java index 11788fa8da2..24ce2a41225 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java @@ -652,7 +652,7 @@ public class MasterRpcServices extends RSRpcServices ExecProcedureResponse.Builder builder = ExecProcedureResponse.newBuilder(); // set return data if available if (data != null) { - builder.setReturnData(ByteString.copyFrom(data)); + builder.setReturnData(UnsafeByteOperations.unsafeWrap(data)); } return builder.build(); } catch (IOException e) { diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java index c917aa8a431..8dbf6970e84 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java @@ -82,6 +82,7 @@ import com.google.common.annotations.VisibleForTesting; import org.apache.hadoop.hbase.shaded.com.google.protobuf.ByteString; import org.apache.hadoop.hbase.shaded.com.google.protobuf.ServiceException; +import org.apache.hadoop.hbase.shaded.com.google.protobuf.UnsafeByteOperations; /** * The ServerManager class manages info about region servers. @@ -475,7 +476,7 @@ public class ServerManager { if (storeFlushedSequenceId != null) { for (Map.Entry entry : storeFlushedSequenceId.entrySet()) { builder.addStoreSequenceId(StoreSequenceId.newBuilder() - .setFamilyName(ByteString.copyFrom(entry.getKey())) + .setFamilyName(UnsafeByteOperations.unsafeWrap(entry.getKey())) .setSequenceId(entry.getValue().longValue()).build()); } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java index 76466d74475..757ddab779f 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java @@ -141,7 +141,6 @@ import org.apache.hadoop.hbase.regionserver.wal.ReplayHLogKey; import org.apache.hadoop.hbase.regionserver.wal.WALEdit; import org.apache.hadoop.hbase.regionserver.wal.WALUtil; import org.apache.hadoop.hbase.security.User; -import org.apache.hadoop.hbase.shaded.com.google.protobuf.ByteString; import org.apache.hadoop.hbase.shaded.com.google.protobuf.TextFormat; import org.apache.hadoop.hbase.shaded.com.google.protobuf.UnsafeByteOperations; import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil; @@ -1741,8 +1740,8 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi // give us a sequence id that is for sure flushed. We want edit replay to start after this // sequence id in this region. If NO_SEQNUM, use the regions maximum flush id. long csid = (earliest == HConstants.NO_SEQNUM)? lastFlushOpSeqIdLocal: earliest - 1; - regionLoadBldr.addStoreCompleteSequenceId(StoreSequenceId. - newBuilder().setFamilyName(ByteString.copyFrom(familyName)).setSequenceId(csid).build()); + regionLoadBldr.addStoreCompleteSequenceId(StoreSequenceId.newBuilder() + .setFamilyName(UnsafeByteOperations.unsafeWrap(familyName)).setSequenceId(csid).build()); } return regionLoadBldr.setCompleteSequenceId(getMaxFlushedSeqId()); } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java index 6974ea03040..a7eb6062ada 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java @@ -80,7 +80,6 @@ import org.apache.hadoop.hbase.exceptions.OutOfOrderScannerNextException; import org.apache.hadoop.hbase.exceptions.ScannerResetException; import org.apache.hadoop.hbase.filter.ByteArrayComparable; import org.apache.hadoop.hbase.filter.CompareFilter.CompareOp; -import org.apache.hadoop.hbase.ipc.CoprocessorRpcUtils; import org.apache.hadoop.hbase.ipc.HBaseRPCErrorHandler; import org.apache.hadoop.hbase.ipc.HBaseRpcController; import org.apache.hadoop.hbase.ipc.PriorityFunction; @@ -193,13 +192,12 @@ import org.apache.hadoop.hbase.wal.WALSplitter; import org.apache.hadoop.hbase.zookeeper.ZKSplitLog; import org.apache.zookeeper.KeeperException; -import com.google.common.annotations.VisibleForTesting; - import org.apache.hadoop.hbase.shaded.com.google.protobuf.ByteString; import org.apache.hadoop.hbase.shaded.com.google.protobuf.Message; import org.apache.hadoop.hbase.shaded.com.google.protobuf.RpcController; import org.apache.hadoop.hbase.shaded.com.google.protobuf.ServiceException; import org.apache.hadoop.hbase.shaded.com.google.protobuf.TextFormat; +import org.apache.hadoop.hbase.shaded.com.google.protobuf.UnsafeByteOperations; /** * Implements the regionserver RPC services. @@ -362,10 +360,6 @@ public class RSRpcServices implements HBaseRPCErrorHandler, private void incNextCallSeq() { nextCallSeq.incrementAndGet(); } - - private void rollbackNextCallSeq() { - nextCallSeq.decrementAndGet(); - } } /** @@ -789,7 +783,7 @@ public class RSRpcServices implements HBaseRPCErrorHandler, serviceResultBuilder.getValueBuilder() .setName(result.getClass().getName()) // TODO: Copy!!! - .setValue(ByteString.copyFrom(result.toByteArray())))); + .setValue(UnsafeByteOperations.unsafeWrap(result.toByteArray())))); } catch (IOException ioe) { rpcServer.getMetrics().exception(ioe); resultOrExceptionBuilder.setException(ResponseConverter.buildException(ioe)); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALCellCodec.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALCellCodec.java index 5878559d9a1..52dfae091e5 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALCellCodec.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALCellCodec.java @@ -140,12 +140,16 @@ public class WALCellCodec implements Codec { // an array of dictionaries. static class BaosAndCompressor extends ByteArrayOutputStream implements ByteStringCompressor { public ByteString toByteString() { + // We need this copy to create the ByteString as the byte[] 'buf' is not immutable. We reuse + // them. return ByteString.copyFrom(this.buf, 0, this.count); } @Override public ByteString compress(byte[] data, Dictionary dict) throws IOException { writeCompressed(data, dict); + // We need this copy to create the ByteString as the byte[] 'buf' is not immutable. We reuse + // them. ByteString result = ByteString.copyFrom(this.buf, 0, this.count); reset(); // Only resets the count - we reuse the byte array. return result; diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/wal/WALKey.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/wal/WALKey.java index 3558cd0b63b..0e35cbebb60 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/wal/WALKey.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/wal/WALKey.java @@ -46,12 +46,12 @@ import org.apache.hadoop.hbase.regionserver.SequenceId; // imports for things that haven't moved from regionserver.wal yet. import org.apache.hadoop.hbase.regionserver.wal.CompressionContext; import org.apache.hadoop.hbase.regionserver.wal.WALCellCodec; -import org.apache.hadoop.hbase.util.ByteStringer; import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; import com.google.common.annotations.VisibleForTesting; import org.apache.hadoop.hbase.shaded.com.google.protobuf.ByteString; +import org.apache.hadoop.hbase.shaded.com.google.protobuf.UnsafeByteOperations; /** * A Key for an entry in the WAL. @@ -650,8 +650,8 @@ public class WALKey implements SequenceId, Comparable { WALCellCodec.ByteStringCompressor compressor) throws IOException { WALProtos.WALKey.Builder builder = WALProtos.WALKey.newBuilder(); if (compressionContext == null) { - builder.setEncodedRegionName(ByteString.copyFrom(this.encodedRegionName)); - builder.setTableName(ByteString.copyFrom(this.tablename.getName())); + builder.setEncodedRegionName(UnsafeByteOperations.unsafeWrap(this.encodedRegionName)); + builder.setTableName(UnsafeByteOperations.unsafeWrap(this.tablename.getName())); } else { builder.setEncodedRegionName(compressor.compress(this.encodedRegionName, compressionContext.regionDict)); @@ -677,7 +677,8 @@ public class WALKey implements SequenceId, Comparable { } if (replicationScope != null) { for (Map.Entry e : replicationScope.entrySet()) { - ByteString family = (compressionContext == null) ? ByteString.copyFrom(e.getKey()) + ByteString family = (compressionContext == null) + ? UnsafeByteOperations.unsafeWrap(e.getKey()) : compressor.compress(e.getKey(), compressionContext.familyDict); builder.addScopes(FamilyScope.newBuilder() .setFamily(family).setScopeType(ScopeType.valueOf(e.getValue())));