From 147ecb26b9e0de8714b4e63d646a4470c64d5014 Mon Sep 17 00:00:00 2001 From: Lee Rhodes Date: Wed, 6 Mar 2024 09:19:39 -0800 Subject: [PATCH 1/2] copying changes from KLL doubles to KLL floats. --- .../org/apache/datasketches/kll/KllFloatsHelper.java | 9 ++++----- .../org/apache/datasketches/kll/KllFloatsSketch.java | 1 + .../java/org/apache/datasketches/kll/KllItemsHelper.java | 6 +++--- .../java/org/apache/datasketches/kll/KllItemsSketch.java | 1 + 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/apache/datasketches/kll/KllFloatsHelper.java b/src/main/java/org/apache/datasketches/kll/KllFloatsHelper.java index 9e2c50e84..53f1e35f3 100644 --- a/src/main/java/org/apache/datasketches/kll/KllFloatsHelper.java +++ b/src/main/java/org/apache/datasketches/kll/KllFloatsHelper.java @@ -123,8 +123,7 @@ private static void compressWhileUpdatingSketch(final KllFloatsSketch fltSk) { } //assumes readOnly = false and UPDATABLE, called from KllFloatsSketch::merge - static void mergeFloatImpl(final KllFloatsSketch mySketch, - final KllFloatsSketch otherFltSk) { + static void mergeFloatImpl(final KllFloatsSketch mySketch, final KllFloatsSketch otherFltSk) { if (otherFltSk.isEmpty()) { return; } //capture my key mutable fields before doing any merging @@ -132,7 +131,7 @@ static void mergeFloatImpl(final KllFloatsSketch mySketch, final float myMin = myEmpty ? Float.NaN : mySketch.getMinItem(); final float myMax = myEmpty ? Float.NaN : mySketch.getMaxItem(); final int myMinK = mySketch.getMinK(); - final long finalN = mySketch.getN() + otherFltSk.getN(); + final long finalN = Math.addExact(mySketch.getN(), otherFltSk.getN()); //buffers that are referenced multiple times final int otherNumLevels = otherFltSk.getNumLevels(); @@ -398,7 +397,7 @@ private static int[] generalFloatsCompress( curLevel++; // start out at level 0 // If we are at the current top level, add an empty level above it for convenience, - // but do not increment numLevels until later + // but do not actually increment numLevels until later if (curLevel == (numLevels - 1)) { inLevels[curLevel + 2] = inLevels[curLevel + 1]; } @@ -465,7 +464,7 @@ private static int[] generalFloatsCompress( return new int[] {numLevels, targetItemCount, currentItemCount}; } - private static void populateFloatWorkArrays( + private static void populateFloatWorkArrays( //workBuf and workLevels are modified final float[] workbuf, final int[] worklevels, final int provisionalNumLevels, final int myCurNumLevels, final int[] myCurLevelsArr, final float[] myCurFloatItemsArr, final int otherNumLevels, final int[] otherLevelsArr, final float[] otherFloatItemsArr) { diff --git a/src/main/java/org/apache/datasketches/kll/KllFloatsSketch.java b/src/main/java/org/apache/datasketches/kll/KllFloatsSketch.java index a3a16813e..2b0c15ef0 100644 --- a/src/main/java/org/apache/datasketches/kll/KllFloatsSketch.java +++ b/src/main/java/org/apache/datasketches/kll/KllFloatsSketch.java @@ -276,6 +276,7 @@ public QuantilesFloatsSketchIterator iterator() { @Override public final void merge(final KllSketch other) { if (readOnly || sketchStructure != UPDATABLE) { throw new SketchesArgumentException(TGT_IS_READ_ONLY_MSG); } + if (this == other) { throw new SketchesArgumentException(SELF_MERGE_MSG); } final KllFloatsSketch othFltSk = (KllFloatsSketch)other; if (othFltSk.isEmpty()) { return; } KllFloatsHelper.mergeFloatImpl(this, othFltSk); diff --git a/src/main/java/org/apache/datasketches/kll/KllItemsHelper.java b/src/main/java/org/apache/datasketches/kll/KllItemsHelper.java index 31c6897d2..f575b6eb2 100644 --- a/src/main/java/org/apache/datasketches/kll/KllItemsHelper.java +++ b/src/main/java/org/apache/datasketches/kll/KllItemsHelper.java @@ -133,7 +133,7 @@ static void mergeItemImpl(final KllItemsSketch mySketch, final Object myMin = myEmpty ? null : mySketch.getMinItem(); final Object myMax = myEmpty ? null : mySketch.getMaxItem(); final int myMinK = mySketch.getMinK(); - final long finalN = mySketch.getN() + otherItmSk.getN(); + final long finalN = Math.addExact(mySketch.getN(), otherItmSk.getN()); //buffers that are referenced multiple times final int otherNumLevels = otherItmSk.getNumLevels(); @@ -399,7 +399,7 @@ private static int[] generalItemsCompress( curLevel++; // start out at level 0 // If we are at the current top level, add an empty level above it for convenience, - // but do not increment numLevels until later + // but do not actually increment numLevels until later if (curLevel == (numLevels - 1)) { inLevels[curLevel + 2] = inLevels[curLevel + 1]; } @@ -466,7 +466,7 @@ private static int[] generalItemsCompress( return new int[] {numLevels, targetItemCount, currentItemCount}; } - private static void populateItemWorkArrays( + private static void populateItemWorkArrays( //workBuf and workLevels are modified final Object[] workbuf, final int[] worklevels, final int provisionalNumLevels, final int myCurNumLevels, final int[] myCurLevelsArr, final Object[] myCurItemsArr, final int otherNumLevels, final int[] otherLevelsArr, final Object[] otherItemsArr, diff --git a/src/main/java/org/apache/datasketches/kll/KllItemsSketch.java b/src/main/java/org/apache/datasketches/kll/KllItemsSketch.java index 9bfc19d96..0461e4f8c 100644 --- a/src/main/java/org/apache/datasketches/kll/KllItemsSketch.java +++ b/src/main/java/org/apache/datasketches/kll/KllItemsSketch.java @@ -251,6 +251,7 @@ public QuantilesGenericSketchIterator iterator() { @Override public final void merge(final KllSketch other) { if (readOnly || sketchStructure != UPDATABLE) { throw new SketchesArgumentException(TGT_IS_READ_ONLY_MSG); } + if (this == other) { throw new SketchesArgumentException(SELF_MERGE_MSG); } final KllItemsSketch othItmSk = (KllItemsSketch)other; if (othItmSk.isEmpty()) { return; } KllItemsHelper.mergeItemImpl(this, othItmSk, comparator); From 05924fb10a8115c35dfaeee502913a06b445a3dc Mon Sep 17 00:00:00 2001 From: Lee Rhodes Date: Wed, 6 Mar 2024 09:23:43 -0800 Subject: [PATCH 2/2] Restore comment. --- .../java/org/apache/datasketches/kll/KllHeapFloatsSketch.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/apache/datasketches/kll/KllHeapFloatsSketch.java b/src/main/java/org/apache/datasketches/kll/KllHeapFloatsSketch.java index afc20a8a8..cc39cce74 100644 --- a/src/main/java/org/apache/datasketches/kll/KllHeapFloatsSketch.java +++ b/src/main/java/org/apache/datasketches/kll/KllHeapFloatsSketch.java @@ -288,7 +288,7 @@ void incNumLevels() { @Override void setNumLevels(final int numLevels) { - //the heap sketch computes num levels from the array itself, so this is not used on-heap + //the heap sketch computes num levels from the array itself, so this is not used on-heap } @Override