From 91890b6d30949f444055e3338ebfa2d1b7184d25 Mon Sep 17 00:00:00 2001 From: Lee Rhodes Date: Thu, 21 Apr 2022 08:43:00 -0700 Subject: [PATCH 1/2] First pass speed optimization of the off-heap Kll sketch. --- .../kll/KllDirectFloatsSketch.java | 8 + .../datasketches/kll/KllDoublesSketch.java | 5 + .../datasketches/kll/KllFloatsHelper.java | 4 +- .../datasketches/kll/KllHeapFloatsSketch.java | 5 + .../apache/datasketches/kll/KllHelper.java | 179 ++++++------------ .../datasketches/kll/KllMemoryValidate.java | 2 +- .../apache/datasketches/kll/KllSketch.java | 2 + .../kll/KllDoublesSketchTest.java | 12 +- .../datasketches/kll/KllSketchTest.java | 3 +- 9 files changed, 92 insertions(+), 128 deletions(-) diff --git a/src/main/java/org/apache/datasketches/kll/KllDirectFloatsSketch.java b/src/main/java/org/apache/datasketches/kll/KllDirectFloatsSketch.java index ec057f860..d6122bf9e 100644 --- a/src/main/java/org/apache/datasketches/kll/KllDirectFloatsSketch.java +++ b/src/main/java/org/apache/datasketches/kll/KllDirectFloatsSketch.java @@ -187,6 +187,14 @@ void setFloatItemsArrayAt(final int index, final float value) { wmem.putFloat(offset, value); } + @Override + void setFloatItemsArraySubrange(final float[] srcArr, final int srcIndex, final int tgtIndex, final int numItems) { + if (readOnly) { kllSketchThrow(TGT_IS_READ_ONLY); } + final int offset = + DATA_START_ADR + getLevelsArray().length * Integer.BYTES + (tgtIndex + 2) * Float.BYTES; + wmem.putFloatArray(offset, srcArr, srcIndex, numItems); + } + @Override void setLevelZeroSorted(final boolean sorted) { if (readOnly) { kllSketchThrow(TGT_IS_READ_ONLY); } diff --git a/src/main/java/org/apache/datasketches/kll/KllDoublesSketch.java b/src/main/java/org/apache/datasketches/kll/KllDoublesSketch.java index 6ef6a172e..f9a35844a 100644 --- a/src/main/java/org/apache/datasketches/kll/KllDoublesSketch.java +++ b/src/main/java/org/apache/datasketches/kll/KllDoublesSketch.java @@ -356,6 +356,11 @@ public void update(final double value) { @Override //Artifact of inheritance void setFloatItemsArrayAt(final int index, final float value) { kllSketchThrow(MUST_NOT_CALL); } + @Override //Artifact of inheritance + void setFloatItemsArraySubrange(final float[] srcArr, final int srcIndex, final int tgtIndex, final int numItems) { + kllSketchThrow(MUST_NOT_CALL); + } + @Override //Artifact of inheritance void setMaxFloatValue(final float value) { kllSketchThrow(MUST_NOT_CALL); } diff --git a/src/main/java/org/apache/datasketches/kll/KllFloatsHelper.java b/src/main/java/org/apache/datasketches/kll/KllFloatsHelper.java index 19260882d..45dda8d0c 100644 --- a/src/main/java/org/apache/datasketches/kll/KllFloatsHelper.java +++ b/src/main/java/org/apache/datasketches/kll/KllFloatsHelper.java @@ -259,7 +259,7 @@ static void mergeSortedFloatArrays( } /** - * Validation Method. This must be modified to test validation + * Validation Method. This must be modified to use the validation test * @param buf the items array * @param start data start * @param length items length @@ -278,7 +278,7 @@ static void randomlyHalveDownFloats(final float[] buf, final int start, final in } /** - * Validation Method. This must be modified to test validation + * Validation Method. This must be modified to use the validation test * @param buf the items array * @param start data start * @param length items length diff --git a/src/main/java/org/apache/datasketches/kll/KllHeapFloatsSketch.java b/src/main/java/org/apache/datasketches/kll/KllHeapFloatsSketch.java index 3499850e6..b671da6fd 100644 --- a/src/main/java/org/apache/datasketches/kll/KllHeapFloatsSketch.java +++ b/src/main/java/org/apache/datasketches/kll/KllHeapFloatsSketch.java @@ -172,6 +172,11 @@ void incNumLevels() { } //not used here @Override void setFloatItemsArrayAt(final int index, final float value) { floatItems_[index] = value; } + @Override + void setFloatItemsArraySubrange(final float[] srcArr, final int srcIndex, final int tgtIndex, final int numItems) { + System.arraycopy(srcArr, srcIndex, floatItems_, tgtIndex, numItems); + } + @Override void setLevelZeroSorted(final boolean sorted) { this.isLevelZeroSorted_ = sorted; } diff --git a/src/main/java/org/apache/datasketches/kll/KllHelper.java b/src/main/java/org/apache/datasketches/kll/KllHelper.java index 38d85e5c5..dfbcdf7a5 100644 --- a/src/main/java/org/apache/datasketches/kll/KllHelper.java +++ b/src/main/java/org/apache/datasketches/kll/KllHelper.java @@ -161,142 +161,85 @@ static void compressWhileUpdatingSketch(final KllSketch mine) { final int adjPop = oddPop ? rawPop - 1 : rawPop; final int halfAdjPop = adjPop / 2; - // level zero might not be sorted, so we must sort it if we wish to compact it - float[] myFloatItemsArr; - double[] myDoubleItemsArr; - if (mine.sketchType == DOUBLES_SKETCH) { - myFloatItemsArr = null; - myDoubleItemsArr = mine.getDoubleItemsArray(); - if (level == 0) { - if (mine.updatableMemFormat) { - myDoubleItemsArr = mine.getDoubleItemsArray(); - Arrays.sort(myDoubleItemsArr, adjBeg, adjBeg + adjPop); - mine.setDoubleItemsArray(myDoubleItemsArr); - } else { - Arrays.sort(mine.getDoubleItemsArray(), adjBeg, adjBeg + adjPop); - } + final double[] myDoubleItemsArr = mine.getDoubleItemsArray(); + if (level == 0) { // level zero might not be sorted, so we must sort it if we wish to compact it + Arrays.sort(myDoubleItemsArr, adjBeg, adjBeg + adjPop); } if (popAbove == 0) { - if (mine.updatableMemFormat) { - myDoubleItemsArr = mine.getDoubleItemsArray(); - KllDoublesHelper.randomlyHalveUpDoubles(myDoubleItemsArr, adjBeg, adjPop, KllSketch.random); - mine.setDoubleItemsArray(myDoubleItemsArr); - } else { - KllDoublesHelper.randomlyHalveUpDoubles(mine.getDoubleItemsArray(), adjBeg, adjPop, KllSketch.random); - } + KllDoublesHelper.randomlyHalveUpDoubles(myDoubleItemsArr, adjBeg, adjPop, KllSketch.random); } else { - if (mine.updatableMemFormat) { - myDoubleItemsArr = mine.getDoubleItemsArray(); - KllDoublesHelper.randomlyHalveDownDoubles(myDoubleItemsArr, adjBeg, adjPop, KllSketch.random); - mine.setDoubleItemsArray(myDoubleItemsArr); - } else { - KllDoublesHelper.randomlyHalveDownDoubles(mine.getDoubleItemsArray(), adjBeg, adjPop, KllSketch.random); - } - if (mine.updatableMemFormat ) { - myDoubleItemsArr = mine.getDoubleItemsArray(); - KllDoublesHelper.mergeSortedDoubleArrays( - myDoubleItemsArr, adjBeg, halfAdjPop, - myDoubleItemsArr, rawEnd, popAbove, - myDoubleItemsArr, adjBeg + halfAdjPop); - mine.setDoubleItemsArray(myDoubleItemsArr); - } else { - myDoubleItemsArr = mine.getDoubleItemsArray(); - KllDoublesHelper.mergeSortedDoubleArrays( - myDoubleItemsArr, adjBeg, halfAdjPop, - myDoubleItemsArr, rawEnd, popAbove, - myDoubleItemsArr, adjBeg + halfAdjPop); - } + KllDoublesHelper.randomlyHalveDownDoubles(myDoubleItemsArr, adjBeg, adjPop, KllSketch.random); + KllDoublesHelper.mergeSortedDoubleArrays( + myDoubleItemsArr, adjBeg, halfAdjPop, + myDoubleItemsArr, rawEnd, popAbove, + myDoubleItemsArr, adjBeg + halfAdjPop); } - } else { //Float sketch - myFloatItemsArr = mine.getFloatItemsArray(); - myDoubleItemsArr = null; - if (level == 0) { - if (mine.updatableMemFormat) { - myFloatItemsArr = mine.getFloatItemsArray(); - Arrays.sort(myFloatItemsArr, adjBeg, adjBeg + adjPop); - mine.setFloatItemsArray(myFloatItemsArr); - } else { - Arrays.sort(mine.getFloatItemsArray(), adjBeg, adjBeg + adjPop); - } + + int newIndex = myLevelsArr[level + 1] - halfAdjPop; // adjust boundaries of the level above + mine.setLevelsArrayAt(level + 1, newIndex); + + if (oddPop) { + mine.setLevelsArrayAt(level, myLevelsArr[level + 1] - 1); // the current level now contains one item + myDoubleItemsArr[myLevelsArr[level]] = myDoubleItemsArr[rawBeg]; // namely this leftover guy + } else { + mine.setLevelsArrayAt(level, myLevelsArr[level + 1]); // the current level is now empty + } + + // verify that we freed up halfAdjPop array slots just below the current level + assert myLevelsArr[level] == rawBeg + halfAdjPop; + + // finally, we need to shift up the data in the levels below + // so that the freed-up space can be used by level zero + if (level > 0) { + final int amount = rawBeg - myLevelsArr[0]; + System.arraycopy(myDoubleItemsArr, myLevelsArr[0], myDoubleItemsArr, myLevelsArr[0] + halfAdjPop, amount); + } + for (int lvl = 0; lvl < level; lvl++) { + newIndex = myLevelsArr[lvl] + halfAdjPop; //adjust boundary + mine.setLevelsArrayAt(lvl, newIndex); + } + mine.setDoubleItemsArray(myDoubleItemsArr); + } + else { //Float sketch + final float[] myFloatItemsArr = mine.getFloatItemsArray(); + if (level == 0) { // level zero might not be sorted, so we must sort it if we wish to compact it + Arrays.sort(myFloatItemsArr, adjBeg, adjBeg + adjPop); } if (popAbove == 0) { - if (mine.updatableMemFormat) { - myFloatItemsArr = mine.getFloatItemsArray(); - KllFloatsHelper.randomlyHalveUpFloats(myFloatItemsArr, adjBeg, adjPop, KllSketch.random); - mine.setFloatItemsArray(myFloatItemsArr); - } else { - KllFloatsHelper.randomlyHalveUpFloats(mine.getFloatItemsArray(), adjBeg, adjPop, KllSketch.random); - } + KllFloatsHelper.randomlyHalveUpFloats(myFloatItemsArr, adjBeg, adjPop, KllSketch.random); } else { - if (mine.updatableMemFormat) { - myFloatItemsArr = mine.getFloatItemsArray(); - KllFloatsHelper.randomlyHalveDownFloats(myFloatItemsArr, adjBeg, adjPop, KllSketch.random); - mine.setFloatItemsArray(myFloatItemsArr); - } else { - KllFloatsHelper.randomlyHalveDownFloats(mine.getFloatItemsArray(), adjBeg, adjPop, KllSketch.random); - } - if (mine.updatableMemFormat ) { - myFloatItemsArr = mine.getFloatItemsArray(); - KllFloatsHelper.mergeSortedFloatArrays( - myFloatItemsArr, adjBeg, halfAdjPop, - myFloatItemsArr, rawEnd, popAbove, - myFloatItemsArr, adjBeg + halfAdjPop); - mine.setFloatItemsArray(myFloatItemsArr); - } else { - myFloatItemsArr = mine.getFloatItemsArray(); - KllFloatsHelper.mergeSortedFloatArrays( - myFloatItemsArr, adjBeg, halfAdjPop, - myFloatItemsArr, rawEnd, popAbove, - myFloatItemsArr, adjBeg + halfAdjPop); - } + KllFloatsHelper.randomlyHalveDownFloats(myFloatItemsArr, adjBeg, adjPop, KllSketch.random); + KllFloatsHelper.mergeSortedFloatArrays( + myFloatItemsArr, adjBeg, halfAdjPop, + myFloatItemsArr, rawEnd, popAbove, + myFloatItemsArr, adjBeg + halfAdjPop); } - } - int newIndex = myLevelsArr[level + 1] - halfAdjPop; // adjust boundaries of the level above - mine.setLevelsArrayAt(level + 1, newIndex); - if (oddPop) { - mine.setLevelsArrayAt(level, myLevelsArr[level + 1] - 1); // the current level now contains one item - if (mine.sketchType == DOUBLES_SKETCH) { - mine.setDoubleItemsArrayAt( - myLevelsArr[level], mine.getDoubleItemsArray()[rawBeg]); // namely this leftover guy + int newIndex = myLevelsArr[level + 1] - halfAdjPop; // adjust boundaries of the level above + mine.setLevelsArrayAt(level + 1, newIndex); + + if (oddPop) { + mine.setLevelsArrayAt(level, myLevelsArr[level + 1] - 1); // the current level now contains one item + myFloatItemsArr[myLevelsArr[level]] = myFloatItemsArr[rawBeg]; // namely this leftover guy } else { - mine.setFloatItemsArrayAt( - myLevelsArr[level], mine.getFloatItemsArray()[rawBeg]); // namely this leftover guy + mine.setLevelsArrayAt(level, myLevelsArr[level + 1]); // the current level is now empty } - } else { - mine.setLevelsArrayAt(level, myLevelsArr[level + 1]); // the current level is now empty - } - - // verify that we freed up halfAdjPop array slots just below the current level - assert myLevelsArr[level] == rawBeg + halfAdjPop; - - // finally, we need to shift up the data in the levels below - // so that the freed-up space can be used by level zero - if (level > 0) { - final int amount = rawBeg - myLevelsArr[0]; - if (mine.sketchType == DOUBLES_SKETCH) { - if (mine.updatableMemFormat) { - myDoubleItemsArr = mine.getDoubleItemsArray(); - System.arraycopy(myDoubleItemsArr, myLevelsArr[0], myDoubleItemsArr, myLevelsArr[0] + halfAdjPop, amount); - mine.setDoubleItemsArray(myDoubleItemsArr); - } else { - System.arraycopy(myDoubleItemsArr, myLevelsArr[0], myDoubleItemsArr, myLevelsArr[0] + halfAdjPop, amount); - } - } else { - if (mine.updatableMemFormat) { - myFloatItemsArr = mine.getFloatItemsArray(); - System.arraycopy(myFloatItemsArr, myLevelsArr[0], myFloatItemsArr, myLevelsArr[0] + halfAdjPop, amount); - mine.setFloatItemsArray(myFloatItemsArr); - } else { - System.arraycopy(myFloatItemsArr, myLevelsArr[0], myFloatItemsArr, myLevelsArr[0] + halfAdjPop, amount); - } + // verify that we freed up halfAdjPop array slots just below the current level + assert myLevelsArr[level] == rawBeg + halfAdjPop; + + // finally, we need to shift up the data in the levels below + // so that the freed-up space can be used by level zero + if (level > 0) { + final int amount = rawBeg - myLevelsArr[0]; + System.arraycopy(myFloatItemsArr, myLevelsArr[0], myFloatItemsArr, myLevelsArr[0] + halfAdjPop, amount); } for (int lvl = 0; lvl < level; lvl++) { newIndex = myLevelsArr[lvl] + halfAdjPop; //adjust boundary mine.setLevelsArrayAt(lvl, newIndex); } + mine.setFloatItemsArray(myFloatItemsArr); } } diff --git a/src/main/java/org/apache/datasketches/kll/KllMemoryValidate.java b/src/main/java/org/apache/datasketches/kll/KllMemoryValidate.java index 1588112a6..9eb25195d 100644 --- a/src/main/java/org/apache/datasketches/kll/KllMemoryValidate.java +++ b/src/main/java/org/apache/datasketches/kll/KllMemoryValidate.java @@ -74,7 +74,7 @@ final class KllMemoryValidate { boolean singleItem; final boolean level0Sorted; final boolean doublesSketch; - final boolean updatableMemFormat; + boolean updatableMemFormat = false; final boolean readOnly; final int k; final int m; diff --git a/src/main/java/org/apache/datasketches/kll/KllSketch.java b/src/main/java/org/apache/datasketches/kll/KllSketch.java index 896cb1b9d..679b10675 100644 --- a/src/main/java/org/apache/datasketches/kll/KllSketch.java +++ b/src/main/java/org/apache/datasketches/kll/KllSketch.java @@ -506,6 +506,8 @@ final boolean isCompactSingleItem() { abstract void setFloatItemsArrayAt(int index, float value); + abstract void setFloatItemsArraySubrange(float[] srcArr, int srcIndex, int tgtIndex, int numItems); + final void setLevelsArray(final int[] levelsArr) { if (readOnly) { kllSketchThrow(TGT_IS_READ_ONLY); } this.levelsArr = levelsArr; diff --git a/src/test/java/org/apache/datasketches/kll/KllDoublesSketchTest.java b/src/test/java/org/apache/datasketches/kll/KllDoublesSketchTest.java index ca534bd5d..5684084ac 100644 --- a/src/test/java/org/apache/datasketches/kll/KllDoublesSketchTest.java +++ b/src/test/java/org/apache/datasketches/kll/KllDoublesSketchTest.java @@ -417,17 +417,17 @@ public void checkReset() { @Test public void coverInheritanceArtifacts() { - float[] dblArr = new float[0]; - float dblV = 1.0f; + float[] fltArr = new float[0]; + float fltV = 1.0f; int idx = 1; KllDoublesSketch sk = KllDoublesSketch.newHeapInstance(20); try { sk.getFloatItemsArray(); fail(); } catch (SketchesArgumentException e) { } try { sk.getMaxFloatValue(); fail(); } catch (SketchesArgumentException e) { } try { sk.getMinFloatValue(); fail(); } catch (SketchesArgumentException e) { } - try { sk.setFloatItemsArray(dblArr); fail(); } catch (SketchesArgumentException e) { } - try { sk.setFloatItemsArrayAt(idx,dblV); fail(); } catch (SketchesArgumentException e) { } - try { sk.setMaxFloatValue(dblV); fail(); } catch (SketchesArgumentException e) { } - try { sk.setMinFloatValue(dblV); fail(); } catch (SketchesArgumentException e) { } + try { sk.setFloatItemsArray(fltArr); fail(); } catch (SketchesArgumentException e) { } + try { sk.setFloatItemsArrayAt(idx,fltV); fail(); } catch (SketchesArgumentException e) { } + try { sk.setMaxFloatValue(fltV); fail(); } catch (SketchesArgumentException e) { } + try { sk.setMinFloatValue(fltV); fail(); } catch (SketchesArgumentException e) { } } @Test diff --git a/src/test/java/org/apache/datasketches/kll/KllSketchTest.java b/src/test/java/org/apache/datasketches/kll/KllSketchTest.java index 1801a7dc9..f99bf9cdb 100644 --- a/src/test/java/org/apache/datasketches/kll/KllSketchTest.java +++ b/src/test/java/org/apache/datasketches/kll/KllSketchTest.java @@ -69,7 +69,8 @@ public void checkWritableWrapCase6And2Doubles() { @Test public void checkWritableWrapFloats() { KllFloatsSketch sk = KllFloatsSketch.newHeapInstance(20); - for (int i = 1; i <= 21; i++) { sk.update(i); } + for (int i = 1; i <= 20; i++) { sk.update(i); } + sk.update(21); WritableMemory wmem = WritableMemory.writableWrap(KllHelper.toUpdatableByteArrayImpl(sk)); KllFloatsSketch sk2 = KllFloatsSketch.writableWrap(wmem, memReqSvr); assertFalse(wmem.isReadOnly()); From de24a45e6ab9533dabf84f32847d725a81b8bb66 Mon Sep 17 00:00:00 2001 From: Lee Rhodes Date: Thu, 21 Apr 2022 11:28:03 -0700 Subject: [PATCH 2/2] Remove unused method --- .../apache/datasketches/kll/KllDirectFloatsSketch.java | 8 -------- .../org/apache/datasketches/kll/KllDoublesSketch.java | 5 ----- .../org/apache/datasketches/kll/KllHeapFloatsSketch.java | 5 ----- src/main/java/org/apache/datasketches/kll/KllSketch.java | 2 -- 4 files changed, 20 deletions(-) diff --git a/src/main/java/org/apache/datasketches/kll/KllDirectFloatsSketch.java b/src/main/java/org/apache/datasketches/kll/KllDirectFloatsSketch.java index d6122bf9e..ec057f860 100644 --- a/src/main/java/org/apache/datasketches/kll/KllDirectFloatsSketch.java +++ b/src/main/java/org/apache/datasketches/kll/KllDirectFloatsSketch.java @@ -187,14 +187,6 @@ void setFloatItemsArrayAt(final int index, final float value) { wmem.putFloat(offset, value); } - @Override - void setFloatItemsArraySubrange(final float[] srcArr, final int srcIndex, final int tgtIndex, final int numItems) { - if (readOnly) { kllSketchThrow(TGT_IS_READ_ONLY); } - final int offset = - DATA_START_ADR + getLevelsArray().length * Integer.BYTES + (tgtIndex + 2) * Float.BYTES; - wmem.putFloatArray(offset, srcArr, srcIndex, numItems); - } - @Override void setLevelZeroSorted(final boolean sorted) { if (readOnly) { kllSketchThrow(TGT_IS_READ_ONLY); } diff --git a/src/main/java/org/apache/datasketches/kll/KllDoublesSketch.java b/src/main/java/org/apache/datasketches/kll/KllDoublesSketch.java index f9a35844a..6ef6a172e 100644 --- a/src/main/java/org/apache/datasketches/kll/KllDoublesSketch.java +++ b/src/main/java/org/apache/datasketches/kll/KllDoublesSketch.java @@ -356,11 +356,6 @@ public void update(final double value) { @Override //Artifact of inheritance void setFloatItemsArrayAt(final int index, final float value) { kllSketchThrow(MUST_NOT_CALL); } - @Override //Artifact of inheritance - void setFloatItemsArraySubrange(final float[] srcArr, final int srcIndex, final int tgtIndex, final int numItems) { - kllSketchThrow(MUST_NOT_CALL); - } - @Override //Artifact of inheritance void setMaxFloatValue(final float value) { kllSketchThrow(MUST_NOT_CALL); } diff --git a/src/main/java/org/apache/datasketches/kll/KllHeapFloatsSketch.java b/src/main/java/org/apache/datasketches/kll/KllHeapFloatsSketch.java index b671da6fd..3499850e6 100644 --- a/src/main/java/org/apache/datasketches/kll/KllHeapFloatsSketch.java +++ b/src/main/java/org/apache/datasketches/kll/KllHeapFloatsSketch.java @@ -172,11 +172,6 @@ void incNumLevels() { } //not used here @Override void setFloatItemsArrayAt(final int index, final float value) { floatItems_[index] = value; } - @Override - void setFloatItemsArraySubrange(final float[] srcArr, final int srcIndex, final int tgtIndex, final int numItems) { - System.arraycopy(srcArr, srcIndex, floatItems_, tgtIndex, numItems); - } - @Override void setLevelZeroSorted(final boolean sorted) { this.isLevelZeroSorted_ = sorted; } diff --git a/src/main/java/org/apache/datasketches/kll/KllSketch.java b/src/main/java/org/apache/datasketches/kll/KllSketch.java index 679b10675..896cb1b9d 100644 --- a/src/main/java/org/apache/datasketches/kll/KllSketch.java +++ b/src/main/java/org/apache/datasketches/kll/KllSketch.java @@ -506,8 +506,6 @@ final boolean isCompactSingleItem() { abstract void setFloatItemsArrayAt(int index, float value); - abstract void setFloatItemsArraySubrange(float[] srcArr, int srcIndex, int tgtIndex, int numItems); - final void setLevelsArray(final int[] levelsArr) { if (readOnly) { kllSketchThrow(TGT_IS_READ_ONLY); } this.levelsArr = levelsArr;