From 21be0698fbeb261081284eaf509d98a8b76605c6 Mon Sep 17 00:00:00 2001 From: lrhodes Date: Wed, 18 Apr 2018 09:28:06 -0700 Subject: [PATCH 1/3] Fix SerDe Issue 165 --- .../com/yahoo/sketches/quantiles/DoublesUnionImpl.java | 4 ++-- .../yahoo/sketches/quantiles/DoublesUnionImplTest.java | 9 +++++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/yahoo/sketches/quantiles/DoublesUnionImpl.java b/src/main/java/com/yahoo/sketches/quantiles/DoublesUnionImpl.java index feef1fe8a..29303b103 100644 --- a/src/main/java/com/yahoo/sketches/quantiles/DoublesUnionImpl.java +++ b/src/main/java/com/yahoo/sketches/quantiles/DoublesUnionImpl.java @@ -77,9 +77,9 @@ static DoublesUnionImpl heapifyInstance(final DoublesSketch sketch) { * @return a DoublesUnion object */ static DoublesUnionImpl heapifyInstance(final Memory srcMem) { - final long n = srcMem.getLong(PreambleUtil.N_LONG); + final int preLongs = srcMem.getByte(PreambleUtil.PREAMBLE_LONGS_BYTE) & 0xFF; final int k = srcMem.getShort(PreambleUtil.K_SHORT) & 0xFFFF; - final HeapUpdateDoublesSketch sketch = (n == 0) + final HeapUpdateDoublesSketch sketch = (preLongs == 1) ? HeapUpdateDoublesSketch.newInstance(k) : HeapUpdateDoublesSketch.heapifyInstance(srcMem); final DoublesUnionImpl union = new DoublesUnionImpl(k); diff --git a/src/test/java/com/yahoo/sketches/quantiles/DoublesUnionImplTest.java b/src/test/java/com/yahoo/sketches/quantiles/DoublesUnionImplTest.java index 69bb49812..a0b8e2c7e 100644 --- a/src/test/java/com/yahoo/sketches/quantiles/DoublesUnionImplTest.java +++ b/src/test/java/com/yahoo/sketches/quantiles/DoublesUnionImplTest.java @@ -693,6 +693,15 @@ public void isSameResourceDirect() { Assert.assertFalse(union.isSameResource(mem2)); } + @SuppressWarnings("unused") + @Test + public void checkSerDeIssue165() { + DoublesUnion union = DoublesUnion.builder().build(); + byte[] byteArr = union.toByteArray(); + Memory mem = Memory.wrap(byteArr); + DoublesUnion union2 = DoublesUnionBuilder.heapify(mem); + } + @Test public void printlnTest() { println("PRINTING: " + this.getClass().getName()); From 0ba1c2bfd1fc1ecc06d5732c96cc4ac39aa83521 Mon Sep 17 00:00:00 2001 From: lrhodes Date: Wed, 18 Apr 2018 13:31:02 -0700 Subject: [PATCH 2/3] Update unit test with more checks. --- .../com/yahoo/sketches/quantiles/DoublesUnionImplTest.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/test/java/com/yahoo/sketches/quantiles/DoublesUnionImplTest.java b/src/test/java/com/yahoo/sketches/quantiles/DoublesUnionImplTest.java index a0b8e2c7e..86871a9a1 100644 --- a/src/test/java/com/yahoo/sketches/quantiles/DoublesUnionImplTest.java +++ b/src/test/java/com/yahoo/sketches/quantiles/DoublesUnionImplTest.java @@ -695,11 +695,13 @@ public void isSameResourceDirect() { @SuppressWarnings("unused") @Test - public void checkSerDeIssue165() { + public void emptyUnionSerDeIssue195() { DoublesUnion union = DoublesUnion.builder().build(); byte[] byteArr = union.toByteArray(); Memory mem = Memory.wrap(byteArr); DoublesUnion union2 = DoublesUnionBuilder.heapify(mem); + Assert.assertEquals(mem.getCapacity(), 8L); + Assert.assertTrue(union2.isEmpty()); } @Test From 276d6735af611427d641012cb6f2910cd71dc60e Mon Sep 17 00:00:00 2001 From: lrhodes Date: Wed, 18 Apr 2018 13:39:46 -0700 Subject: [PATCH 3/3] Make random tests deterministic. --- .../com/yahoo/sketches/tuple/FilterTest.java | 31 ++++++++++++------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/src/test/java/com/yahoo/sketches/tuple/FilterTest.java b/src/test/java/com/yahoo/sketches/tuple/FilterTest.java index 9860d020f..5f936d48d 100644 --- a/src/test/java/com/yahoo/sketches/tuple/FilterTest.java +++ b/src/test/java/com/yahoo/sketches/tuple/FilterTest.java @@ -1,13 +1,14 @@ package com.yahoo.sketches.tuple; +import java.util.Random; + import org.testng.Assert; import org.testng.annotations.Test; -import java.util.Random; - public class FilterTest { - private final int numberOfElements = 100; + private static final int numberOfElements = 100; + private static final Random random = new Random(1);//deterministic for this class @Test public void emptySketch() { @@ -39,7 +40,8 @@ public void nullSketch() { @Test public void filledSketchShouldBehaveTheSame() { - UpdatableSketch sketch = new UpdatableSketchBuilder<>(new DoubleSummaryFactory()).build(); + UpdatableSketch sketch = + new UpdatableSketchBuilder<>(new DoubleSummaryFactory()).build(); fillSketch(sketch, numberOfElements, 0.0); @@ -56,7 +58,8 @@ public void filledSketchShouldBehaveTheSame() { @Test public void filledSketchShouldFilterOutElements() { - UpdatableSketch sketch = new UpdatableSketchBuilder<>(new DoubleSummaryFactory()).build(); + UpdatableSketch sketch = + new UpdatableSketchBuilder<>(new DoubleSummaryFactory()).build(); fillSketch(sketch, numberOfElements, 0.0); fillSketch(sketch, 2 * numberOfElements, 1.0); @@ -74,7 +77,8 @@ public void filledSketchShouldFilterOutElements() { @Test public void filteringInEstimationMode() { - UpdatableSketch sketch = new UpdatableSketchBuilder<>(new DoubleSummaryFactory()).build(); + UpdatableSketch sketch = + new UpdatableSketchBuilder<>(new DoubleSummaryFactory()).build(); int n = 10000; fillSketch(sketch, n, 0.0); @@ -93,7 +97,9 @@ public void filteringInEstimationMode() { @Test public void nonEmptySketchWithNoEntries() { - UpdatableSketch sketch = new UpdatableSketchBuilder<>(new DoubleSummaryFactory()).setSamplingProbability(0.0001f).build(); + UpdatableSketch sketch = + new UpdatableSketchBuilder<>( + new DoubleSummaryFactory()).setSamplingProbability(0.0001f).build(); sketch.update(0, 0.0); Assert.assertFalse(sketch.isEmpty()); @@ -110,11 +116,12 @@ public void nonEmptySketchWithNoEntries() { Assert.assertEquals(filteredSketch.getUpperBound(1), sketch.getUpperBound(1)); } - private static void fillSketch(UpdatableSketch sketch, int numberOfElements, Double sketchValue) { - Random random = new Random(); + private static void fillSketch(UpdatableSketch sketch, + int numberOfElements, Double sketchValue) { + - for (int cont = 0; cont < numberOfElements; cont++) { - sketch.update(random.nextLong(), sketchValue); - } + for (int cont = 0; cont < numberOfElements; cont++) { + sketch.update(random.nextLong(), sketchValue); + } } } \ No newline at end of file