From 99531f22f32354317cc3dff9e5d48730270aafa7 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Mon, 30 Aug 2021 16:51:11 +0200 Subject: Avoid copying data just to compress them when it is not necessary. --- .../core/rpc/SlimeClusterStateBundleCodec.java | 2 +- .../yahoo/messagebus/network/rpc/RPCSendV2.java | 6 ++-- .../main/java/com/yahoo/compress/Compressor.java | 25 ++++++++----- .../main/java/com/yahoo/slime/BinaryDecoder.java | 10 +++--- .../main/java/com/yahoo/slime/BinaryEncoder.java | 20 ++++++----- .../main/java/com/yahoo/slime/BinaryFormat.java | 15 ++++++-- .../main/java/com/yahoo/slime/BufferedInput.java | 24 ++++++------- .../main/java/com/yahoo/slime/BufferedOutput.java | 17 +++++---- .../java/com/yahoo/slime/BinaryFormatTestCase.java | 42 +++++++++++++--------- 9 files changed, 97 insertions(+), 64 deletions(-) diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/rpc/SlimeClusterStateBundleCodec.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/rpc/SlimeClusterStateBundleCodec.java index 1a3184955d5..65dc4de07bf 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/rpc/SlimeClusterStateBundleCodec.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/rpc/SlimeClusterStateBundleCodec.java @@ -48,7 +48,7 @@ public class SlimeClusterStateBundleCodec implements ClusterStateBundleCodec, En } byte[] serialized = BinaryFormat.encode(slime); - Compressor.Compression compression = compressor.compress(serialized); + Compressor.Compression compression = BinaryFormat.encode_and_compress(slime, compressor); return EncodedClusterStateBundle.fromCompressionBuffer(compression); } diff --git a/messagebus/src/main/java/com/yahoo/messagebus/network/rpc/RPCSendV2.java b/messagebus/src/main/java/com/yahoo/messagebus/network/rpc/RPCSendV2.java index 6ec3ea5ec7d..dbc6af42295 100644 --- a/messagebus/src/main/java/com/yahoo/messagebus/network/rpc/RPCSendV2.java +++ b/messagebus/src/main/java/com/yahoo/messagebus/network/rpc/RPCSendV2.java @@ -100,8 +100,7 @@ public class RPCSendV2 extends RPCSend { root.setLong(TRACELEVEL_F, traceLevel); root.setData(BLOB_F, payload); - byte[] serializedSlime = BinaryFormat.encode(slime); - Compressor.Compression compressionResult = compressor.compress(serializedSlime); + Compressor.Compression compressionResult = BinaryFormat.encode_and_compress(slime, compressor); v.add(new Int8Value(compressionResult.type().getCode())); v.add(new Int32Value(compressionResult.uncompressedSize())); @@ -200,8 +199,7 @@ public class RPCSendV2 extends RPCSend { } } - byte[] serializedSlime = BinaryFormat.encode(slime); - Compressor.Compression compressionResult = compressor.compress(serializedSlime); + Compressor.Compression compressionResult = BinaryFormat.encode_and_compress(slime, compressor); ret.add(new Int8Value(compressionResult.type().getCode())); ret.add(new Int32Value(compressionResult.uncompressedSize())); diff --git a/vespajlib/src/main/java/com/yahoo/compress/Compressor.java b/vespajlib/src/main/java/com/yahoo/compress/Compressor.java index 98228292cc7..1b20f1dcaf7 100644 --- a/vespajlib/src/main/java/com/yahoo/compress/Compressor.java +++ b/vespajlib/src/main/java/com/yahoo/compress/Compressor.java @@ -87,31 +87,38 @@ public class Compressor { public Compression compress(CompressionType requestedCompression, byte[] data, int offset, int len) { switch (requestedCompression) { case NONE: - if ((offset != 0) || (len != data.length)) { - data = Arrays.copyOfRange(data, offset, offset + len); - } - return new Compression(CompressionType.NONE, data.length, data); + return compact(CompressionType.NONE, data, offset, len); case LZ4: if (len < compressMinSizeBytes) return new Compression(CompressionType.INCOMPRESSIBLE, len, data); byte[] compressedData = getCompressor().compress(data, offset, len); - if (compressedData.length + 8 >= len * compressionThresholdFactor) - return new Compression(CompressionType.INCOMPRESSIBLE, len, data); + if (compressedData.length + 8 >= len * compressionThresholdFactor) { + return compact(CompressionType.INCOMPRESSIBLE, data, offset, len); + } return new Compression(CompressionType.LZ4, len, compressedData); case ZSTD: - if (len < compressMinSizeBytes) return new Compression(CompressionType.INCOMPRESSIBLE, len, data); + if (len < compressMinSizeBytes) { + return compact(CompressionType.INCOMPRESSIBLE, data, offset, len); + } byte[] compressed = zstdCompressor.compress(data, offset, len); return new Compression(CompressionType.ZSTD, len, compressed); default: throw new IllegalArgumentException(requestedCompression + " is not supported"); } } + + private Compression compact(CompressionType type, byte[] data, int offset, int len) { + if ((offset != 0) || (len != data.length)) { + data = Arrays.copyOfRange(data, offset, offset + len); + } + return new Compression(type, len, data); + } private LZ4Compressor getCompressor() { return level < 7 ? factory.fastCompressor() : factory.highCompressor(); } /** Compresses some data using the requested compression type */ - public Compression compress(CompressionType requestedCompression, byte[] data) { return compress(requestedCompression, data, Optional.empty()); } + public Compression compress(CompressionType requestedCompression, byte[] data) { return compress(requestedCompression, data, 0, data.length); } /** Compresses some data using the compression type of this compressor */ - public Compression compress(byte[] data, int uncompressedSize) { return compress(type, data, Optional.of(uncompressedSize)); } + public Compression compress(byte[] data, int uncompressedSize) { return compress(type, data, 0, uncompressedSize); } /** Compresses some data using the compression type of this compressor */ public Compression compress(byte[] data) { return compress(type, data, Optional.empty()); } diff --git a/vespajlib/src/main/java/com/yahoo/slime/BinaryDecoder.java b/vespajlib/src/main/java/com/yahoo/slime/BinaryDecoder.java index ecee5e9504e..13e08484d7c 100644 --- a/vespajlib/src/main/java/com/yahoo/slime/BinaryDecoder.java +++ b/vespajlib/src/main/java/com/yahoo/slime/BinaryDecoder.java @@ -1,7 +1,11 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.slime; -import static com.yahoo.slime.BinaryFormat.*; + +import static com.yahoo.slime.BinaryFormat.decode_double; +import static com.yahoo.slime.BinaryFormat.decode_meta; +import static com.yahoo.slime.BinaryFormat.decode_type; +import static com.yahoo.slime.BinaryFormat.decode_zigzag; final class BinaryDecoder { BufferedInput in; @@ -135,9 +139,7 @@ final class BinaryDecoder { void decodeValue(Inserter inserter) { byte b = in.getByte(); - Cursor cursor = decodeValue(inserter, - decode_type(b), - decode_meta(b)); + Cursor cursor = decodeValue(inserter, decode_type(b), decode_meta(b)); if (!cursor.valid()) { in.fail("failed to decode value"); } diff --git a/vespajlib/src/main/java/com/yahoo/slime/BinaryEncoder.java b/vespajlib/src/main/java/com/yahoo/slime/BinaryEncoder.java index 4d9fdbba6d4..8f654b46032 100644 --- a/vespajlib/src/main/java/com/yahoo/slime/BinaryEncoder.java +++ b/vespajlib/src/main/java/com/yahoo/slime/BinaryEncoder.java @@ -1,28 +1,30 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.slime; -import static com.yahoo.slime.BinaryFormat.*; +import static com.yahoo.slime.BinaryFormat.encode_double; +import static com.yahoo.slime.BinaryFormat.encode_type_and_meta; +import static com.yahoo.slime.BinaryFormat.encode_zigzag; final class BinaryEncoder implements ArrayTraverser, ObjectSymbolTraverser { - BufferedOutput out; + private final BufferedOutput out; - public BinaryEncoder(int capacity) { - out = new BufferedOutput(capacity); + BinaryEncoder() { + this(new BufferedOutput()); } - - public BinaryEncoder() { - out = new BufferedOutput(); + BinaryEncoder(BufferedOutput output) { + out = output; } - public byte[] encode(Slime slime) { + BufferedOutput encode(Slime slime) { out.reset(); encodeSymbolTable(slime); encodeValue(slime.get()); - return out.toArray(); + return out; } + void encode_cmpr_long(long value) { byte next = (byte)(value & 0x7f); value >>>= 7; // unsigned shift diff --git a/vespajlib/src/main/java/com/yahoo/slime/BinaryFormat.java b/vespajlib/src/main/java/com/yahoo/slime/BinaryFormat.java index c212500eca7..b6cde18f824 100644 --- a/vespajlib/src/main/java/com/yahoo/slime/BinaryFormat.java +++ b/vespajlib/src/main/java/com/yahoo/slime/BinaryFormat.java @@ -1,6 +1,8 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.slime; +import com.yahoo.compress.Compressor; + /** * Class for serializing Slime data into binary format, or deserializing * the binary format into a Slime object. @@ -40,8 +42,17 @@ public class BinaryFormat { * @return a new byte array with just the encoded slime. **/ public static byte[] encode(Slime slime) { - BinaryEncoder encoder = new BinaryEncoder(); - return encoder.encode(slime); + return new BinaryEncoder().encode(slime).toArray(); + } + + /** + * Take a Slime object and serialize it into binary format, and compresses it. + * @param slime the object which is to be serialized. + * @param compressor the compressor to use. + * @return a new byte array with just the encoded and compressed slime. + **/ + public static Compressor.Compression encode_and_compress(Slime slime, Compressor compressor) { + return new BinaryEncoder().encode(slime).compress(compressor); } /** diff --git a/vespajlib/src/main/java/com/yahoo/slime/BufferedInput.java b/vespajlib/src/main/java/com/yahoo/slime/BufferedInput.java index 8a647269697..1e67aac3cb9 100644 --- a/vespajlib/src/main/java/com/yahoo/slime/BufferedInput.java +++ b/vespajlib/src/main/java/com/yahoo/slime/BufferedInput.java @@ -19,17 +19,17 @@ final class BufferedInput { position = end; } - public BufferedInput(byte[] bytes) { + BufferedInput(byte[] bytes) { this(bytes, 0, bytes.length); } - public BufferedInput(byte[] bytes, int offset, int length) { + BufferedInput(byte[] bytes, int offset, int length) { this.source = bytes; this.start = offset; position = offset; this.end = offset + length; } - public final byte getByte() { + byte getByte() { if (position == end) { fail("underflow"); return 0; @@ -37,31 +37,31 @@ final class BufferedInput { return source[position++]; } - public boolean failed() { + boolean failed() { return failReason != null; } - public boolean eof() { + boolean eof() { return this.position == this.end; } - public String getErrorMessage() { + String getErrorMessage() { return failReason; } - public int getConsumedSize() { + int getConsumedSize() { return failed() ? 0 : position - start; } - public byte[] getOffending() { + byte[] getOffending() { byte[] ret = new byte[failPos-start]; System.arraycopy(source, start, ret, 0, failPos-start); return ret; } - public final byte [] getBacking() { return source; } - public final int getPosition() { return position; } - public final void skip(int size) { + byte [] getBacking() { return source; } + int getPosition() { return position; } + void skip(int size) { if (position + size > end) { fail("underflow"); } else { @@ -69,7 +69,7 @@ final class BufferedInput { } } - public final byte[] getBytes(int size) { + byte[] getBytes(int size) { if (position + size > end) { fail("underflow"); return new byte[0]; diff --git a/vespajlib/src/main/java/com/yahoo/slime/BufferedOutput.java b/vespajlib/src/main/java/com/yahoo/slime/BufferedOutput.java index 20ed0fc8018..61a00185e76 100644 --- a/vespajlib/src/main/java/com/yahoo/slime/BufferedOutput.java +++ b/vespajlib/src/main/java/com/yahoo/slime/BufferedOutput.java @@ -1,22 +1,24 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.slime; -final class BufferedOutput { +import com.yahoo.compress.Compressor; + +class BufferedOutput { private byte[] buf; private int capacity; private int pos; - public BufferedOutput(int cap) { + BufferedOutput(int cap) { capacity = (cap < 64) ? 64 : cap; buf = new byte[capacity]; } - public BufferedOutput() { + BufferedOutput() { this(4096); } - public void reset() { + void reset() { pos = 0; } @@ -31,7 +33,7 @@ final class BufferedOutput { } } - public int position() { return pos; } + int position() { return pos; } void put(byte b) { reserve(1); @@ -49,9 +51,12 @@ final class BufferedOutput { } } - public byte[] toArray() { + byte[] toArray() { byte[] ret = new byte[pos]; System.arraycopy(buf, 0, ret, 0, pos); return ret; } + Compressor.Compression compress(Compressor compressor) { + return compressor.compress(buf, pos); + } } diff --git a/vespajlib/src/test/java/com/yahoo/slime/BinaryFormatTestCase.java b/vespajlib/src/test/java/com/yahoo/slime/BinaryFormatTestCase.java index 7cf4bddaa01..cd67e0653dd 100644 --- a/vespajlib/src/test/java/com/yahoo/slime/BinaryFormatTestCase.java +++ b/vespajlib/src/test/java/com/yahoo/slime/BinaryFormatTestCase.java @@ -1,35 +1,42 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.slime; +import com.yahoo.compress.CompressionType; +import com.yahoo.compress.Compressor; import org.junit.Test; -import static org.junit.Assert.assertTrue; +import static com.yahoo.slime.BinaryFormat.decode_double; +import static com.yahoo.slime.BinaryFormat.decode_meta; +import static com.yahoo.slime.BinaryFormat.decode_type; +import static com.yahoo.slime.BinaryFormat.decode_zigzag; +import static com.yahoo.slime.BinaryFormat.encode_double; +import static com.yahoo.slime.BinaryFormat.encode_type_and_meta; +import static com.yahoo.slime.BinaryFormat.encode_zigzag; +import static org.hamcrest.CoreMatchers.is; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThat; -import static org.hamcrest.CoreMatchers.*; -import static com.yahoo.slime.BinaryFormat.*; public class BinaryFormatTestCase { static final int TYPE_LIMIT = 8; static final int META_LIMIT = 32; - static final int MAX_CMPR_SIZE = 10; static final int MAX_NUM_SIZE = 8; - static final byte enc_t_and_sz(Type t, int size) { + static byte enc_t_and_sz(Type t, int size) { assert size <= 30; return encode_type_and_meta(t.ID, size + 1); } - static final byte enc_t_and_m(Type t, int meta) { + static byte enc_t_and_m(Type t, int meta) { assert meta <= 31; return encode_type_and_meta(t.ID, meta); } void verify_cmpr_long(long value, byte[] expect) { - BinaryEncoder bof = new BinaryEncoder(); + BufferedOutput output = new BufferedOutput(); + BinaryEncoder bof = new BinaryEncoder(output); bof.encode_cmpr_long(value); - byte[] actual = bof.out.toArray(); + byte[] actual = output.toArray(); assertThat(actual, is(expect)); BinaryDecoder bif = new BinaryDecoder(); @@ -41,6 +48,10 @@ public class BinaryFormatTestCase { // was verifyBasic void verifyEncoding(Slime slime, byte[] expect) { assertThat(BinaryFormat.encode(slime), is(expect)); + Compressor compressor = new Compressor(CompressionType.LZ4, 3, 2, 0); + Compressor.Compression result = BinaryFormat.encode_and_compress(slime, compressor); + byte [] decompressed = compressor.decompress(result); + assertThat(decompressed, is(expect)); verifyMultiEncode(expect); } @@ -223,13 +234,11 @@ public class BinaryFormatTestCase { expect.put(encode_type_and_meta((int)type, (int)(size +1))); } else { expect.put(type); - BinaryEncoder encoder = new BinaryEncoder(); - encoder.out = expect; + BinaryEncoder encoder = new BinaryEncoder(expect); encoder.encode_cmpr_long(size); } { - BinaryEncoder encoder = new BinaryEncoder(); - encoder.out = actual; + BinaryEncoder encoder = new BinaryEncoder(actual); encoder.write_type_and_size(type, size); } assertThat(actual.toArray(), is(expect.toArray())); @@ -273,14 +282,14 @@ public class BinaryFormatTestCase { byte[] expect = expbuf.toArray(); // test output: - BinaryEncoder bof = new BinaryEncoder(); - bof.out = new BufferedOutput(); + BufferedOutput output = new BufferedOutput(); + BinaryEncoder bof = new BinaryEncoder(output); if (hi != 0) { bof.write_type_and_bytes_be(type, bits); } else { bof.write_type_and_bytes_le(type, bits); } - byte[] actual = bof.out.toArray(); + byte[] actual = output.toArray(); assertThat(actual, is(expect)); // test input: @@ -530,9 +539,8 @@ public class BinaryFormatTestCase { 2, enc_t_and_sz(Type.DATA, 4), // f 'd', 'a', 't', 'a' }; - Slime slime = new Slime(); BinaryDecoder decoder = new BinaryDecoder(); - slime = decoder.decode(data); + Slime slime = decoder.decode(data); int consumed = decoder.in.getConsumedSize(); assertThat(consumed, is(data.length)); Cursor c = slime.get(); -- cgit v1.2.3 From d343d6e40b20945f3cc8ced8d9da6015f959d4a4 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Mon, 30 Aug 2021 19:43:18 +0200 Subject: Update clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/rpc/SlimeClusterStateBundleCodec.java Remove unused Co-authored-by: Harald Musum --- .../vespa/clustercontroller/core/rpc/SlimeClusterStateBundleCodec.java | 1 - 1 file changed, 1 deletion(-) diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/rpc/SlimeClusterStateBundleCodec.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/rpc/SlimeClusterStateBundleCodec.java index 65dc4de07bf..be0109b892d 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/rpc/SlimeClusterStateBundleCodec.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/rpc/SlimeClusterStateBundleCodec.java @@ -47,7 +47,6 @@ public class SlimeClusterStateBundleCodec implements ClusterStateBundleCodec, En feedBlock.setString("description", stateBundle.getFeedBlock().get().getDescription()); } - byte[] serialized = BinaryFormat.encode(slime); Compressor.Compression compression = BinaryFormat.encode_and_compress(slime, compressor); return EncodedClusterStateBundle.fromCompressionBuffer(compression); } -- cgit v1.2.3 From 1cfca4dd53342f139866de60c570e5767ba5153f Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Mon, 30 Aug 2021 19:45:01 +0200 Subject: Use explicit import. --- .../clustercontroller/core/rpc/SlimeClusterStateBundleCodec.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/rpc/SlimeClusterStateBundleCodec.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/rpc/SlimeClusterStateBundleCodec.java index be0109b892d..5799406ef4b 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/rpc/SlimeClusterStateBundleCodec.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/rpc/SlimeClusterStateBundleCodec.java @@ -3,7 +3,11 @@ package com.yahoo.vespa.clustercontroller.core.rpc; import com.yahoo.compress.CompressionType; import com.yahoo.compress.Compressor; -import com.yahoo.slime.*; +import com.yahoo.slime.BinaryFormat; +import com.yahoo.slime.Cursor; +import com.yahoo.slime.Inspector; +import com.yahoo.slime.ObjectTraverser; +import com.yahoo.slime.Slime; import com.yahoo.vdslib.state.ClusterState; import com.yahoo.vespa.clustercontroller.core.AnnotatedClusterState; import com.yahoo.vespa.clustercontroller.core.ClusterStateBundle; -- cgit v1.2.3