You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by br...@apache.org on 2020/02/07 01:55:59 UTC

[cassandra] branch trunk updated: Lower the amount of garbage ChecksummingTransformerTest generates by reusing memory

This is an automated email from the ASF dual-hosted git repository.

brandonwilliams pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/cassandra.git


The following commit(s) were added to refs/heads/trunk by this push:
     new ea3a3c4  Lower the amount of garbage ChecksummingTransformerTest generates by reusing memory
ea3a3c4 is described below

commit ea3a3c47f992bd4812415ee6762875d1e3f43ebc
Author: David Capwell <dc...@gmail.com>
AuthorDate: Thu Feb 6 11:37:06 2020 -0800

    Lower the amount of garbage ChecksummingTransformerTest generates by reusing memory
    
    Patch by David Capwell, reviewed by brandonwilliams and Jordan West for
    CASSANDRA-15555
---
 .../checksum/ChecksummingTransformerTest.java      | 51 +++++++++++++--------
 .../transport/frame/checksum/ReusableBuffer.java   | 52 ++++++++++++++++++++++
 2 files changed, 85 insertions(+), 18 deletions(-)

diff --git a/test/unit/org/apache/cassandra/transport/frame/checksum/ChecksummingTransformerTest.java b/test/unit/org/apache/cassandra/transport/frame/checksum/ChecksummingTransformerTest.java
index b905d4b..d678044 100644
--- a/test/unit/org/apache/cassandra/transport/frame/checksum/ChecksummingTransformerTest.java
+++ b/test/unit/org/apache/cassandra/transport/frame/checksum/ChecksummingTransformerTest.java
@@ -37,9 +37,11 @@ import org.apache.cassandra.transport.frame.compress.SnappyCompressor;
 import org.apache.cassandra.utils.ChecksumType;
 import org.apache.cassandra.utils.Pair;
 import org.quicktheories.core.Gen;
+import org.quicktheories.impl.Constraint;
 
 import static org.quicktheories.QuickTheory.qt;
-import static org.quicktheories.generators.SourceDSL.*;
+import static org.quicktheories.generators.SourceDSL.arbitrary;
+import static org.quicktheories.generators.SourceDSL.integers;
 
 public class ChecksummingTransformerTest
 {
@@ -57,7 +59,7 @@ public class ChecksummingTransformerTest
     @Test
     public void roundTripSafetyProperty()
     {
-        qt().withExamples(35)
+        qt()
             .forAll(inputs(),
                     compressors(),
                     checksumTypes(),
@@ -68,7 +70,7 @@ public class ChecksummingTransformerTest
     @Test
     public void roundTripZeroLengthInput()
     {
-        qt().withExamples(20)
+        qt()
             .forAll(zeroLengthInputs(),
                     compressors(),
                     checksumTypes(),
@@ -79,7 +81,7 @@ public class ChecksummingTransformerTest
     @Test
     public void corruptionCausesFailure()
     {
-        qt().withExamples(35)
+        qt()
             .forAll(inputWithCorruptablePosition(),
                     integers().between(0, Byte.MAX_VALUE).map(Integer::byteValue),
                     compressors(),
@@ -87,13 +89,13 @@ public class ChecksummingTransformerTest
             .checkAssert(this::roundTripWithCorruption);
     }
 
-    private void roundTripWithCorruption(Pair<String, Integer> inputAndCorruptablePosition,
+    private void roundTripWithCorruption(Pair<ReusableBuffer, Integer> inputAndCorruptablePosition,
                                          byte corruptionValue,
                                          Compressor compressor,
                                          ChecksumType checksum)
     {
-        String input = inputAndCorruptablePosition.left;
-        ByteBuf expectedBuf = Unpooled.wrappedBuffer(input.getBytes());
+        ReusableBuffer input = inputAndCorruptablePosition.left;
+        ByteBuf expectedBuf = input.toByteBuf();
         int byteToCorrupt = inputAndCorruptablePosition.right;
         ChecksummingTransformer transformer = new ChecksummingTransformer(checksum, DEFAULT_BLOCK_SIZE, compressor);
         ByteBuf outbound = transformer.transformOutbound(expectedBuf);
@@ -173,11 +175,10 @@ public class ChecksummingTransformerTest
         Assert.assertEquals(expectedBuf, inbound);
     }
 
-    private void roundTrip(String input, Compressor compressor, ChecksumType checksum, int blockSize)
+    private void roundTrip(ReusableBuffer input, Compressor compressor, ChecksumType checksum, int blockSize)
     {
         ChecksummingTransformer transformer = new ChecksummingTransformer(checksum, blockSize, compressor);
-        byte[] expectedBytes = input.getBytes();
-        ByteBuf expectedBuf = Unpooled.wrappedBuffer(expectedBytes);
+        ByteBuf expectedBuf = input.toByteBuf();
 
         ByteBuf outbound = transformer.transformOutbound(expectedBuf);
         ByteBuf inbound = transformer.transformInbound(outbound, FLAGS);
@@ -187,23 +188,38 @@ public class ChecksummingTransformerTest
         Assert.assertEquals(expectedBuf, inbound);
     }
 
-    private Gen<Pair<String, Integer>> inputWithCorruptablePosition()
+    private Gen<Pair<ReusableBuffer, Integer>> inputWithCorruptablePosition()
     {
         // we only generate corruption for byte 2 onward. This is to skip introducing corruption in the number
         // of chunks (which isn't checksummed
-        return inputs().flatMap(s -> integers().between(2, s.length() + 2).map(i -> Pair.create(s, i)));
+        return inputs().flatMap(s -> integers().between(2, s.length + 2).map(i -> Pair.create(s, i)));
     }
 
-    private Gen<String> inputs()
+    private static Gen<ReusableBuffer> inputs()
     {
-        Gen<String> randomStrings = strings().basicMultilingualPlaneAlphabet().ofLengthBetween(0, MAX_INPUT_SIZE);
-        Gen<String> highlyCompressable = strings().betweenCodePoints('c', 'e').ofLengthBetween(1, MAX_INPUT_SIZE);
+        Gen<ReusableBuffer> randomStrings = inputs(0, MAX_INPUT_SIZE, 0, (1 << 8) - 1);
+        Gen<ReusableBuffer> highlyCompressable = inputs(1, MAX_INPUT_SIZE, 'c', 'e');
         return randomStrings.mix(highlyCompressable, 50);
     }
 
-    private Gen<String> zeroLengthInputs()
+    private static Gen<ReusableBuffer> inputs(int minSize, int maxSize, int smallestByte, int largestByte)
     {
-        return strings().ascii().ofLength(0);
+        ReusableBuffer buffer = new ReusableBuffer(new byte[maxSize]);
+        Constraint byteGen = Constraint.between(smallestByte, largestByte);
+        Constraint lengthGen = Constraint.between(minSize, maxSize);
+        Gen<ReusableBuffer> gen = td -> {
+            int size = (int) td.next(lengthGen);
+            buffer.length = size;
+            for (int i = 0; i < size; i++)
+                buffer.bytes[i] = (byte) td.next(byteGen);
+            return buffer;
+        };
+        return gen;
+    }
+
+    private Gen<ReusableBuffer> zeroLengthInputs()
+    {
+        return arbitrary().constant(new ReusableBuffer(new byte[0]));
     }
 
     private Gen<Compressor> compressors()
@@ -220,5 +236,4 @@ public class ChecksummingTransformerTest
     {
         return arbitrary().constant(DEFAULT_BLOCK_SIZE);
     }
-
 }
diff --git a/test/unit/org/apache/cassandra/transport/frame/checksum/ReusableBuffer.java b/test/unit/org/apache/cassandra/transport/frame/checksum/ReusableBuffer.java
new file mode 100644
index 0000000..c0e0abe
--- /dev/null
+++ b/test/unit/org/apache/cassandra/transport/frame/checksum/ReusableBuffer.java
@@ -0,0 +1,52 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.transport.frame.checksum;
+
+import java.nio.ByteBuffer;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.Unpooled;
+import org.apache.cassandra.utils.ByteBufferUtil;
+
+/**
+ * Wrapper around byte[] with length which is expected to be reused with different data.  It is expected that the bytes
+ * are directly modified and the length gets updated to reflect; this is done to avoid producing unneeded garbage.
+ *
+ * This class is not thread safe.
+ */
+public final class ReusableBuffer
+{
+    public final byte[] bytes;
+    public int length;
+
+    public ReusableBuffer(byte[] bytes)
+    {
+        this.bytes = bytes;
+        this.length = bytes.length;
+    }
+
+    public ByteBuf toByteBuf() {
+        return Unpooled.wrappedBuffer(bytes, 0, length);
+    }
+
+    public String toString()
+    {
+        return ByteBufferUtil.bytesToHex(ByteBuffer.wrap(bytes, 0, length));
+    }
+}


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org