Merge pull request #18906 from vespa-engine/revert-18905-revert-18903-balder/avoid-copying-data

Revert "Revert "Avoid copying data just to compress them when it is not necessary.""

9 files changed, 105 insertions, 67 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..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;
@@ -47,8 +51,7 @@ public class SlimeClusterStateBundleCodec implements ClusterStateBundleCodec, En
             feedBlock.setString("description", stateBundle.getFeedBlock().get().getDescription());
         }
 
-        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..eacb0f2c406 100644
--- a/vespajlib/src/main/java/com/yahoo/compress/Compressor.java
+++ b/vespajlib/src/main/java/com/yahoo/compress/Compressor.java
@@ -87,31 +87,40 @@ 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);
+                if (len < compressMinSizeBytes) {
+                    return compact(CompressionType.INCOMPRESSIBLE, data, offset, len);
+                }
                 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();