Skip to content

Commit

Permalink
Merge pull request #393 from apache/disallowUpdatableHeapify
Browse files Browse the repository at this point in the history
Made the public heapify method throw if the given memory is in updatable
  • Loading branch information
leerho authored Apr 20, 2022
2 parents 70d7356 + 8e5b91d commit 7fe957e
Show file tree
Hide file tree
Showing 12 changed files with 59 additions and 51 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@

import static java.lang.Math.max;
import static java.lang.Math.min;
import static org.apache.datasketches.kll.KllPreambleUtil.getMemoryUpdatableFormatFlag;
import static org.apache.datasketches.kll.KllSketch.Error.MUST_NOT_BE_UPDATABLE_FORMAT;
import static org.apache.datasketches.kll.KllSketch.Error.MUST_NOT_CALL;
import static org.apache.datasketches.kll.KllSketch.Error.SRC_MUST_BE_DOUBLE;
import static org.apache.datasketches.kll.KllSketch.Error.TGT_IS_READ_ONLY;
import static org.apache.datasketches.kll.KllSketch.Error.kllSketchThrow;

Expand Down Expand Up @@ -58,9 +59,8 @@ public static int getMaxSerializedSizeBytes(final int k, final long n, final boo
*/
public static KllDoublesSketch heapify(final Memory srcMem) {
Objects.requireNonNull(srcMem, "Parameter 'srcMem' must not be null");
final KllMemoryValidate memVal = new KllMemoryValidate(srcMem);
if (!memVal.doublesSketch) { Error.kllSketchThrow(SRC_MUST_BE_DOUBLE); }
return new KllHeapDoublesSketch(srcMem, memVal);
if (getMemoryUpdatableFormatFlag(srcMem)) { Error.kllSketchThrow(MUST_NOT_BE_UPDATABLE_FORMAT); }
return KllHeapDoublesSketch.heapifyImpl(srcMem);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@

import static java.lang.Math.max;
import static java.lang.Math.min;
import static org.apache.datasketches.kll.KllPreambleUtil.getMemoryUpdatableFormatFlag;
import static org.apache.datasketches.kll.KllSketch.Error.MUST_NOT_BE_UPDATABLE_FORMAT;
import static org.apache.datasketches.kll.KllSketch.Error.MUST_NOT_CALL;
import static org.apache.datasketches.kll.KllSketch.Error.SRC_MUST_BE_FLOAT;
import static org.apache.datasketches.kll.KllSketch.Error.TGT_IS_READ_ONLY;
import static org.apache.datasketches.kll.KllSketch.Error.kllSketchThrow;

Expand Down Expand Up @@ -58,9 +59,8 @@ public static int getMaxSerializedSizeBytes(final int k, final long n, final boo
*/
public static KllFloatsSketch heapify(final Memory srcMem) {
Objects.requireNonNull(srcMem, "Parameter 'srcMem' must not be null");
final KllMemoryValidate memVal = new KllMemoryValidate(srcMem);
if (memVal.doublesSketch) { Error.kllSketchThrow(SRC_MUST_BE_FLOAT); }
return new KllHeapFloatsSketch(srcMem, memVal);
if (getMemoryUpdatableFormatFlag(srcMem)) { Error.kllSketchThrow(MUST_NOT_BE_UPDATABLE_FORMAT); }
return KllHeapFloatsSketch.heapifyImpl(srcMem);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,11 @@
import static org.apache.datasketches.kll.KllPreambleUtil.DATA_START_ADR_SINGLE_ITEM;
import static org.apache.datasketches.kll.KllSketch.Error.MUST_NOT_CALL;
import static org.apache.datasketches.kll.KllSketch.Error.NOT_SINGLE_ITEM;
import static org.apache.datasketches.kll.KllSketch.Error.SRC_MUST_BE_DOUBLE;
import static org.apache.datasketches.kll.KllSketch.Error.kllSketchThrow;

import java.util.Objects;

import org.apache.datasketches.memory.Memory;

/**
Expand Down Expand Up @@ -74,9 +77,9 @@ final class KllHeapDoublesSketch extends KllDoublesSketch {
/**
* Heapify constructor.
* @param srcMem Memory object that contains data serialized by this sketch.
* @param memVal the MemoryCheck object
* @param memVal the MemoryVaidate object
*/
KllHeapDoublesSketch(final Memory srcMem, final KllMemoryValidate memVal) {
private KllHeapDoublesSketch(final Memory srcMem, final KllMemoryValidate memVal) {
super(null, null );
k_ = memVal.k;
m_ = memVal.m;
Expand Down Expand Up @@ -117,6 +120,13 @@ else if (memVal.singleItem && !updatableMemFormat) {
}
}

static KllHeapDoublesSketch heapifyImpl(final Memory srcMem) {
Objects.requireNonNull(srcMem, "Parameter 'srcMem' must not be null");
final KllMemoryValidate memVal = new KllMemoryValidate(srcMem);
if (!memVal.doublesSketch) { Error.kllSketchThrow(SRC_MUST_BE_DOUBLE); }
return new KllHeapDoublesSketch(srcMem, memVal);
}

@Override
public int getK() { return k_; }

Expand Down
14 changes: 12 additions & 2 deletions src/main/java/org/apache/datasketches/kll/KllHeapFloatsSketch.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,11 @@
import static org.apache.datasketches.kll.KllPreambleUtil.DATA_START_ADR_SINGLE_ITEM;
import static org.apache.datasketches.kll.KllSketch.Error.MUST_NOT_CALL;
import static org.apache.datasketches.kll.KllSketch.Error.NOT_SINGLE_ITEM;
import static org.apache.datasketches.kll.KllSketch.Error.SRC_MUST_BE_FLOAT;
import static org.apache.datasketches.kll.KllSketch.Error.kllSketchThrow;

import java.util.Objects;

import org.apache.datasketches.memory.Memory;

/**
Expand Down Expand Up @@ -74,9 +77,9 @@ final class KllHeapFloatsSketch extends KllFloatsSketch {
/**
* Heapify constructor.
* @param srcMem Memory object that contains data serialized by this sketch.
* @param memVal the MemoryCheck object
* @param memVal the MemoryValidate object
*/
KllHeapFloatsSketch(final Memory srcMem, final KllMemoryValidate memVal) {
private KllHeapFloatsSketch(final Memory srcMem, final KllMemoryValidate memVal) {
super(null, null);
k_ = memVal.k;
m_ = memVal.m;
Expand Down Expand Up @@ -117,6 +120,13 @@ else if (memVal.singleItem && !updatableMemFormat) {
}
}

static KllHeapFloatsSketch heapifyImpl(final Memory srcMem) {
Objects.requireNonNull(srcMem, "Parameter 'srcMem' must not be null");
final KllMemoryValidate memVal = new KllMemoryValidate(srcMem);
if (memVal.doublesSketch) { Error.kllSketchThrow(SRC_MUST_BE_FLOAT); }
return new KllHeapFloatsSketch(srcMem, memVal);
}

@Override
public int getK() { return k_; }

Expand Down
5 changes: 3 additions & 2 deletions src/main/java/org/apache/datasketches/kll/KllSketch.java
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@
*
* @author Lee Rhodes, Kevin Lang
*/
abstract class KllSketch {
public abstract class KllSketch {

/**
* Used to define the variable type of the current instance of this class.
Expand All @@ -81,7 +81,8 @@ enum Error {
MUST_NOT_CALL("This is an artifact of inheritance and should never be called."),
SINGLE_ITEM_IMPROPER_CALL("Improper method use for single-item sketch"),
MRS_MUST_NOT_BE_NULL("MemoryRequestServer cannot be null."),
NOT_SINGLE_ITEM("Sketch is not single item.");
NOT_SINGLE_ITEM("Sketch is not single item."),
MUST_NOT_BE_UPDATABLE_FORMAT("Given Memory object must not be in updatableFormat.");

private String msg;

Expand Down
17 changes: 2 additions & 15 deletions src/main/java/org/apache/datasketches/kll/package-info.java
Original file line number Diff line number Diff line change
Expand Up @@ -148,21 +148,8 @@
* <li>Then <i>v<sub>lo</sub> &le; v &le; v<sub>hi</sub></i>, with 99% confidence.</li>
* </ul>
*
* <p>The current implementations of the KLL sketch in the DataSketches Java library component include:</p>
*
* <ul>
* <li><b>KllFloatsSketch</b>: This operates on the Java heap and uses the java <i>float</i> primitive for the
* smallest possible size. It can be serialized to a compact, immutable format or to an updatable format, which can
* be modified off-heap. </li>
* <li><b>KllDoublesSketch</b>: This operates on the Java heap and uses the java <i>double</i> primitive for a much
* larger range of numeric values, and is larger as a result. It can be serialized to a compact, immutable format or
* to an updatable format, which can be modified off-heap.</li>
* <li><b>KllFloatsSketchIterator</b>: Iterates over the retained values and weights of the KllFloatsSketch.</li>
* <li><b>KllDoublesSketchIterator</b>: : Iterates over the retained values and weights of the KllDoublesSketch.</li>
* </ul>
*
* <p>Please visit our website: <a href="https://datasketches.apache.org">DataSketches Home Page</a> for more
* information.</p>
* <p>Please visit our website: <a href="https://datasketches.apache.org">DataSketches Home Page</a> and
* the Javadocs for more information.</p>
*
* @author Kevin Lang
* @author Alexander Saydakov
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -550,7 +550,7 @@ public void checkGetWritableMemory() {
assertFalse(sketch.isFloatsSketch());

final WritableMemory wmem = sketch.getWritableMemory();
final KllDoublesSketch sk = KllDoublesSketch.heapify(wmem);
final KllDoublesSketch sk = KllHeapDoublesSketch.heapifyImpl(wmem);
assertEquals(sk.getK(), 200);
assertEquals(sk.getN(), 200);
assertFalse(sk.isEmpty());
Expand Down Expand Up @@ -584,7 +584,7 @@ public void checkHeapify() {
WritableMemory dstMem = WritableMemory.allocate(6000);
KllDoublesSketch sk = KllDoublesSketch.newDirectInstance(20, dstMem, memReqSvr);
for (int i = 1; i <= 100; i++) { sk.update(i); }
KllDoublesSketch sk2 = KllDoublesSketch.heapify(dstMem);
KllDoublesSketch sk2 = KllHeapDoublesSketch.heapifyImpl(dstMem);
assertEquals(sk2.getMinValue(), 1.0);
assertEquals(sk2.getMaxValue(), 100.0);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -550,7 +550,7 @@ public void checkGetWritableMemory() {
assertFalse(sketch.isDoublesSketch());

final WritableMemory wmem = sketch.getWritableMemory();
final KllFloatsSketch sk = KllFloatsSketch.heapify(wmem);
final KllFloatsSketch sk = KllHeapFloatsSketch.heapifyImpl(wmem);
assertEquals(sk.getK(), 200);
assertEquals(sk.getN(), 200);
assertFalse(sk.isEmpty());
Expand Down Expand Up @@ -584,7 +584,7 @@ public void checkHeapify() {
WritableMemory dstMem = WritableMemory.allocate(6000);
KllFloatsSketch sk = KllFloatsSketch.newDirectInstance(20, dstMem, memReqSvr);
for (int i = 1; i <= 100; i++) { sk.update(i); }
KllFloatsSketch sk2 = KllFloatsSketch.heapify(dstMem);
KllFloatsSketch sk2 = KllHeapFloatsSketch.heapifyImpl(dstMem);
assertEquals(sk2.getMinValue(), 1.0);
assertEquals(sk2.getMaxValue(), 100.0);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ public void checkSketchInitializeDoubleHeapifyUpdatableMem() {
//println(sk2.toString(true, true));
compBytes = KllHelper.toUpdatableByteArrayImpl(sk2);
wmem = WritableMemory.writableWrap(compBytes);
sk = KllDoublesSketch.heapify(wmem);
sk = KllHeapDoublesSketch.heapifyImpl(wmem);
println(sk.toString(true, true));
assertEquals(sk.getK(), k);
assertEquals(sk.getN(), k + 1);
Expand All @@ -266,7 +266,7 @@ public void checkSketchInitializeDoubleHeapifyUpdatableMem() {
compBytes = KllHelper.toUpdatableByteArrayImpl(sk2);
wmem = WritableMemory.writableWrap(compBytes);
//println(KllPreambleUtil.toString(wmem));
sk = KllDoublesSketch.heapify(wmem);
sk = KllHeapDoublesSketch.heapifyImpl(wmem);
assertEquals(sk.getK(), k);
assertEquals(sk.getN(), 0);
assertEquals(sk.getNumRetained(), 0);
Expand All @@ -287,7 +287,7 @@ public void checkSketchInitializeDoubleHeapifyUpdatableMem() {
compBytes = KllHelper.toUpdatableByteArrayImpl(sk2);
wmem = WritableMemory.writableWrap(compBytes);
//println(KllPreambleUtil.memoryToString(wmem, true));
sk = KllDoublesSketch.heapify(wmem);
sk = KllHeapDoublesSketch.heapifyImpl(wmem);
assertEquals(sk.getK(), k);
assertEquals(sk.getN(), 1);
assertEquals(sk.getNumRetained(), 1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ public void checkSketchInitializeFloatHeapifyUpdatableMem() {
compBytes = KllHelper.toUpdatableByteArrayImpl(sk2);
wmem = WritableMemory.writableWrap(compBytes);
//println(KllPreambleUtil.toString(wmem));
sk = KllFloatsSketch.heapify(wmem);
sk = KllHeapFloatsSketch.heapifyImpl(wmem);
assertEquals(sk.getK(), k);
assertEquals(sk.getN(), k + 1);
assertEquals(sk.getNumRetained(), 11);
Expand All @@ -266,7 +266,7 @@ public void checkSketchInitializeFloatHeapifyUpdatableMem() {
compBytes = KllHelper.toUpdatableByteArrayImpl(sk2);
wmem = WritableMemory.writableWrap(compBytes);
//println(KllPreambleUtil.toString(wmem));
sk = KllFloatsSketch.heapify(wmem);
sk = KllHeapFloatsSketch.heapifyImpl(wmem);
assertEquals(sk.getK(), k);
assertEquals(sk.getN(), 0);
assertEquals(sk.getNumRetained(), 0);
Expand All @@ -287,7 +287,7 @@ public void checkSketchInitializeFloatHeapifyUpdatableMem() {
compBytes = KllHelper.toUpdatableByteArrayImpl(sk2);
wmem = WritableMemory.writableWrap(compBytes);
//println(KllPreambleUtil.toString(wmem));
sk = KllFloatsSketch.heapify(wmem);
sk = KllHeapFloatsSketch.heapifyImpl(wmem);
assertEquals(sk.getK(), k);
assertEquals(sk.getN(), 1);
assertEquals(sk.getNumRetained(), 1);
Expand Down
12 changes: 6 additions & 6 deletions src/test/java/org/apache/datasketches/kll/MiscDoublesTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ public void checkSketchInitializeDoubleHeapifyUpdatableMem() {
compBytes = KllHelper.toUpdatableByteArrayImpl(sk2);
wmem = WritableMemory.writableWrap(compBytes);
println(KllPreambleUtil.toString(wmem, true));
sk = KllDoublesSketch.heapify(wmem);
sk = KllHeapDoublesSketch.heapifyImpl(wmem);
assertEquals(sk.getK(), k);
assertEquals(sk.getN(), k + 1);
assertEquals(sk.getNumRetained(), 11);
Expand All @@ -325,7 +325,7 @@ public void checkSketchInitializeDoubleHeapifyUpdatableMem() {
compBytes = KllHelper.toUpdatableByteArrayImpl(sk2);
wmem = WritableMemory.writableWrap(compBytes);
println(KllPreambleUtil.toString(wmem, true));
sk = KllDoublesSketch.heapify(wmem);
sk = KllHeapDoublesSketch.heapifyImpl(wmem);
assertEquals(sk.getK(), k);
assertEquals(sk.getN(), 0);
assertEquals(sk.getNumRetained(), 0);
Expand All @@ -346,7 +346,7 @@ public void checkSketchInitializeDoubleHeapifyUpdatableMem() {
compBytes = KllHelper.toUpdatableByteArrayImpl(sk2);
wmem = WritableMemory.writableWrap(compBytes);
println(KllPreambleUtil.toString(wmem, true));
sk = KllDoublesSketch.heapify(wmem);
sk = KllHeapDoublesSketch.heapifyImpl(wmem);
assertEquals(sk.getK(), k);
assertEquals(sk.getN(), 1);
assertEquals(sk.getNumRetained(), 1);
Expand Down Expand Up @@ -437,7 +437,7 @@ public void checkMemoryToStringDoubleUpdatable() {
s = KllPreambleUtil.toString(wmem, true);
println("step 1: sketch to byte[]/memory & analyze memory");
println(s);
sk2 = KllDoublesSketch.heapify(wmem);
sk2 = KllHeapDoublesSketch.heapifyImpl(wmem);
upBytes2 = KllHelper.toUpdatableByteArrayImpl(sk2);
wmem = WritableMemory.writableWrap(upBytes2);
s = KllPreambleUtil.toString(wmem, true);
Expand All @@ -455,7 +455,7 @@ public void checkMemoryToStringDoubleUpdatable() {
s = KllPreambleUtil.toString(wmem, true);
println("step 1: sketch to byte[]/memory & analyze memory");
println(s);
sk2 = KllDoublesSketch.heapify(wmem);
sk2 = KllHeapDoublesSketch.heapifyImpl(wmem);
upBytes2 = KllHelper.toUpdatableByteArrayImpl(sk2);
wmem = WritableMemory.writableWrap(upBytes2);
s = KllPreambleUtil.toString(wmem, true);
Expand All @@ -471,7 +471,7 @@ public void checkMemoryToStringDoubleUpdatable() {
s = KllPreambleUtil.toString(wmem, true);
println("step 1: sketch to byte[]/memory & analyze memory");
println(s);
sk2 = KllDoublesSketch.heapify(wmem);
sk2 = KllHeapDoublesSketch.heapifyImpl(wmem);
upBytes2 = KllHelper.toUpdatableByteArrayImpl(sk2);
wmem = WritableMemory.writableWrap(upBytes2);
s = KllPreambleUtil.toString(wmem, true);
Expand Down
12 changes: 6 additions & 6 deletions src/test/java/org/apache/datasketches/kll/MiscFloatsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ public void checkSketchInitializeFloatHeapifyUpdatableMem() {
compBytes = KllHelper.toUpdatableByteArrayImpl(sk2);
wmem = WritableMemory.writableWrap(compBytes);
println(KllPreambleUtil.toString(wmem, true));
sk = KllFloatsSketch.heapify(wmem);
sk = KllHeapFloatsSketch.heapifyImpl(wmem);
assertEquals(sk.getK(), k);
assertEquals(sk.getN(), k + 1);
assertEquals(sk.getNumRetained(), 11);
Expand All @@ -325,7 +325,7 @@ public void checkSketchInitializeFloatHeapifyUpdatableMem() {
compBytes = KllHelper.toUpdatableByteArrayImpl(sk2);
wmem = WritableMemory.writableWrap(compBytes);
println(KllPreambleUtil.toString(wmem, true));
sk = KllFloatsSketch.heapify(wmem);
sk = KllHeapFloatsSketch.heapifyImpl(wmem);
assertEquals(sk.getK(), k);
assertEquals(sk.getN(), 0);
assertEquals(sk.getNumRetained(), 0);
Expand All @@ -346,7 +346,7 @@ public void checkSketchInitializeFloatHeapifyUpdatableMem() {
compBytes = KllHelper.toUpdatableByteArrayImpl(sk2);
wmem = WritableMemory.writableWrap(compBytes);
println(KllPreambleUtil.toString(wmem, true));
sk = KllFloatsSketch.heapify(wmem);
sk = KllHeapFloatsSketch.heapifyImpl(wmem);
assertEquals(sk.getK(), k);
assertEquals(sk.getN(), 1);
assertEquals(sk.getNumRetained(), 1);
Expand Down Expand Up @@ -437,7 +437,7 @@ public void checkMemoryToStringFloatUpdatable() {
s = KllPreambleUtil.toString(wmem, true);
println("step 1: sketch to byte[]/memory & analyze memory");
println(s);
sk2 = KllFloatsSketch.heapify(wmem);
sk2 = KllHeapFloatsSketch.heapifyImpl(wmem);
upBytes2 = KllHelper.toUpdatableByteArrayImpl(sk2);
wmem = WritableMemory.writableWrap(upBytes2);
s = KllPreambleUtil.toString(wmem, true);
Expand All @@ -455,7 +455,7 @@ public void checkMemoryToStringFloatUpdatable() {
s = KllPreambleUtil.toString(wmem, true);
println("step 1: sketch to byte[]/memory & analyze memory");
println(s);
sk2 = KllFloatsSketch.heapify(wmem);
sk2 = KllHeapFloatsSketch.heapifyImpl(wmem);
upBytes2 = KllHelper.toUpdatableByteArrayImpl(sk2);
wmem = WritableMemory.writableWrap(upBytes2);
s = KllPreambleUtil.toString(wmem, true);
Expand All @@ -471,7 +471,7 @@ public void checkMemoryToStringFloatUpdatable() {
s = KllPreambleUtil.toString(wmem, true);
println("step 1: sketch to byte[]/memory & analyze memory");
println(s);
sk2 = KllFloatsSketch.heapify(wmem);
sk2 = KllHeapFloatsSketch.heapifyImpl(wmem);
upBytes2 = KllHelper.toUpdatableByteArrayImpl(sk2);
wmem = WritableMemory.writableWrap(upBytes2);
s = KllPreambleUtil.toString(wmem, true);
Expand Down

0 comments on commit 7fe957e

Please sign in to comment.