From c470c8953d4927043b6383fad8e792289c634c09 Mon Sep 17 00:00:00 2001 From: cnauroth Date: Wed, 16 Dec 2015 10:45:32 -0800 Subject: [PATCH] HDFS-9557. Reduce object allocation in PB conversion. Contributed by Daryn Sharp. --- .../hdfs/protocolPB/PBHelperClient.java | 29 ++++++++++--------- hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt | 3 ++ .../hadoop/hdfs/protocolPB/PBHelper.java | 2 +- .../hadoop/hdfs/protocolPB/TestPBHelper.java | 7 +++++ 4 files changed, 26 insertions(+), 15 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelperClient.java b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelperClient.java index 33bdc5c9666..ac960e8f1f5 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelperClient.java +++ b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelperClient.java @@ -194,7 +194,8 @@ private PBHelperClient() { } public static ByteString getByteString(byte[] bytes) { - return ByteString.copyFrom(bytes); + // return singleton to reduce object allocation + return (bytes.length == 0) ? ByteString.EMPTY : ByteString.copyFrom(bytes); } public static ShmId convert(ShortCircuitShmIdProto shmId) { @@ -221,8 +222,8 @@ public static ExtendedBlockProto convert(final ExtendedBlock b) { public static TokenProto convert(Token tok) { return TokenProto.newBuilder(). - setIdentifier(ByteString.copyFrom(tok.getIdentifier())). - setPassword(ByteString.copyFrom(tok.getPassword())). + setIdentifier(getByteString(tok.getIdentifier())). + setPassword(getByteString(tok.getPassword())). setKind(tok.getKind().toString()). setService(tok.getService().toString()).build(); } @@ -451,16 +452,16 @@ public static HdfsProtos.CipherOptionProto convert(CipherOption option) { builder.setSuite(convert(option.getCipherSuite())); } if (option.getInKey() != null) { - builder.setInKey(ByteString.copyFrom(option.getInKey())); + builder.setInKey(getByteString(option.getInKey())); } if (option.getInIv() != null) { - builder.setInIv(ByteString.copyFrom(option.getInIv())); + builder.setInIv(getByteString(option.getInIv())); } if (option.getOutKey() != null) { - builder.setOutKey(ByteString.copyFrom(option.getOutKey())); + builder.setOutKey(getByteString(option.getOutKey())); } if (option.getOutIv() != null) { - builder.setOutIv(ByteString.copyFrom(option.getOutIv())); + builder.setOutIv(getByteString(option.getOutIv())); } return builder.build(); } @@ -1738,8 +1739,8 @@ public static DataEncryptionKeyProto convert(DataEncryptionKey bet) { DataEncryptionKeyProto.Builder b = DataEncryptionKeyProto.newBuilder() .setKeyId(bet.keyId) .setBlockPoolId(bet.blockPoolId) - .setNonce(ByteString.copyFrom(bet.nonce)) - .setEncryptionKey(ByteString.copyFrom(bet.encryptionKey)) + .setNonce(getByteString(bet.nonce)) + .setEncryptionKey(getByteString(bet.encryptionKey)) .setExpiryDate(bet.expiryDate); if (bet.encryptionAlgorithm != null) { b.setEncryptionAlgorithm(bet.encryptionAlgorithm); @@ -1816,10 +1817,10 @@ public static HdfsFileStatusProto convert(HdfsFileStatus fs) { setGroup(fs.getGroup()). setFileId(fs.getFileId()). setChildrenNum(fs.getChildrenNum()). - setPath(ByteString.copyFrom(fs.getLocalNameInBytes())). + setPath(getByteString(fs.getLocalNameInBytes())). setStoragePolicy(fs.getStoragePolicy()); if (fs.isSymlink()) { - builder.setSymlink(ByteString.copyFrom(fs.getSymlinkInBytes())); + builder.setSymlink(getByteString(fs.getSymlinkInBytes())); } if (fs.getFileEncryptionInfo() != null) { builder.setFileEncryptionInfo(convert(fs.getFileEncryptionInfo())); @@ -1846,7 +1847,7 @@ public static SnapshottableDirectoryStatusProto convert( int snapshotNumber = status.getSnapshotNumber(); int snapshotQuota = status.getSnapshotQuota(); byte[] parentFullPath = status.getParentFullPath(); - ByteString parentFullPathBytes = ByteString.copyFrom( + ByteString parentFullPathBytes = getByteString( parentFullPath == null ? DFSUtilClient.EMPTY_BYTES : parentFullPath); HdfsFileStatusProto fs = convert(status.getDirStatus()); SnapshottableDirectoryStatusProto.Builder builder = @@ -2061,7 +2062,7 @@ public static SnapshotDiffReportEntryProto convert(DiffReportEntry entry) { if (entry == null) { return null; } - ByteString sourcePath = ByteString.copyFrom(entry.getSourcePath() == null ? + ByteString sourcePath = getByteString(entry.getSourcePath() == null ? DFSUtilClient.EMPTY_BYTES : entry.getSourcePath()); String modification = entry.getType().getLabel(); SnapshotDiffReportEntryProto.Builder builder = SnapshotDiffReportEntryProto @@ -2069,7 +2070,7 @@ public static SnapshotDiffReportEntryProto convert(DiffReportEntry entry) { .setModificationLabel(modification); if (entry.getType() == DiffType.RENAME) { ByteString targetPath = - ByteString.copyFrom(entry.getTargetPath() == null ? + getByteString(entry.getTargetPath() == null ? DFSUtilClient.EMPTY_BYTES : entry.getTargetPath()); builder.setTargetPath(targetPath); } diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt index c11b48ff65c..69fa302eb8d 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -1764,6 +1764,9 @@ Release 2.8.0 - UNRELEASED HDFS-8894. Set SO_KEEPALIVE on DN server sockets. (Kanaka Kumar Avvaru via wang) + HDFS-9557. Reduce object allocation in PB conversion + (Daryn Sharp via cnauroth) + OPTIMIZATIONS HDFS-8026. Trace FSOutputSummer#writeChecksumChunks rather than diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelper.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelper.java index fd8a3869352..9271e331259 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelper.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelper.java @@ -229,7 +229,7 @@ public static BlocksWithLocations convert(BlocksWithLocationsProto blocks) { public static BlockKeyProto convert(BlockKey key) { byte[] encodedKey = key.getEncodedKey(); - ByteString keyBytes = ByteString.copyFrom(encodedKey == null ? + ByteString keyBytes = PBHelperClient.getByteString(encodedKey == null ? DFSUtilClient.EMPTY_BYTES : encodedKey); return BlockKeyProto.newBuilder().setKeyId(key.getKeyId()) .setKeyBytes(keyBytes).setExpiryDate(key.getExpiryDate()).build(); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/protocolPB/TestPBHelper.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/protocolPB/TestPBHelper.java index 4171d5c78c2..9f0e0f18112 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/protocolPB/TestPBHelper.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/protocolPB/TestPBHelper.java @@ -19,6 +19,7 @@ import static org.hamcrest.CoreMatchers.is; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertSame; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; @@ -99,6 +100,7 @@ import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; import com.google.common.collect.Lists; +import com.google.protobuf.ByteString; /** * Tests for {@link PBHelper} @@ -110,6 +112,11 @@ public class TestPBHelper { */ private static final double DELTA = 0.000001; + @Test + public void testGetByteString() { + assertSame(ByteString.EMPTY, PBHelperClient.getByteString(new byte[0])); + } + @Test public void testConvertNamenodeRole() { assertEquals(NamenodeRoleProto.BACKUP,