diff --git a/src/main/java/com/yahoo/sketches/cpc/CompressedState.java b/src/main/java/com/yahoo/sketches/cpc/CompressedState.java index 766704efa..28c771bd8 100644 --- a/src/main/java/com/yahoo/sketches/cpc/CompressedState.java +++ b/src/main/java/com/yahoo/sketches/cpc/CompressedState.java @@ -42,13 +42,12 @@ */ final class CompressedState { private static final String LS = System.getProperty("line.separator"); + private boolean csvIsValid = false; + private boolean windowIsValid = false; final int lgK; final short seedHash; int fiCol = 0; boolean mergeFlag = false; //compliment of HIP Flag - boolean csvIsValid = false; - boolean windowIsValid = false; - long numCoupons = 0; double kxp; @@ -60,7 +59,7 @@ final class CompressedState { int[] cwStream = null; //may be longer than required int cwLengthInts = 0; - int cpcRequiredBytes = 0; + //int cpcRequiredBytes = 0; private CompressedState(final int lgK, final short seedHash) { this.lgK = lgK; diff --git a/src/main/java/com/yahoo/sketches/theta/CompactSketch.java b/src/main/java/com/yahoo/sketches/theta/CompactSketch.java index 5036e3ce0..bc9b5b1b3 100644 --- a/src/main/java/com/yahoo/sketches/theta/CompactSketch.java +++ b/src/main/java/com/yahoo/sketches/theta/CompactSketch.java @@ -39,6 +39,14 @@ public abstract class CompactSketch extends Sketch { //Sketch + @Override + public CompactSketch compact() { return this; } + + @Override + public CompactSketch compact(final boolean dstOrdered, final WritableMemory dstMem) { + return this; + } + @Override public Family getFamily() { return Family.COMPACT; @@ -112,6 +120,7 @@ static final long[] compactCachePart(final long[] srcCache, final int lgArrLongs return cacheOut; } + //compactCache and dstMem must be valid static final Memory loadCompactMemory(final long[] compactCache, final short seedHash, final int curCount, final long thetaLong, final WritableMemory dstMem, final byte flags, final int preLongs) { @@ -144,7 +153,7 @@ static final Memory loadCompactMemory(final long[] compactCache, final short see if (preLongs > 2) { insertThetaLong(dstMem, thetaLong); } - if ((compactCache != null) && (curCount > 0)) { + if (curCount > 0) { dstMem.putLongArray(preLongs << 3, compactCache, 0, curCount); } return dstMem; diff --git a/src/main/java/com/yahoo/sketches/theta/ConcurrentDirectQuickSelectSketch.java b/src/main/java/com/yahoo/sketches/theta/ConcurrentDirectQuickSelectSketch.java index 66b41d438..fc104e214 100644 --- a/src/main/java/com/yahoo/sketches/theta/ConcurrentDirectQuickSelectSketch.java +++ b/src/main/java/com/yahoo/sketches/theta/ConcurrentDirectQuickSelectSketch.java @@ -241,7 +241,8 @@ private void advanceEpoch() { ConcurrentPropagationService.resetExecutorService(Thread.currentThread().getId()); //noinspection NonAtomicOperationOnVolatileField // this increment of a volatile field is done within the scope of the propagation - // synchronization and hence is done by a single thread + // synchronization and hence is done by a single thread. + // Ignore a FindBugs warning epoch_++; endPropagation(null, true); initBgPropagationService(); diff --git a/src/main/java/com/yahoo/sketches/theta/ConcurrentHeapQuickSelectSketch.java b/src/main/java/com/yahoo/sketches/theta/ConcurrentHeapQuickSelectSketch.java index 780ba542e..220e93760 100644 --- a/src/main/java/com/yahoo/sketches/theta/ConcurrentHeapQuickSelectSketch.java +++ b/src/main/java/com/yahoo/sketches/theta/ConcurrentHeapQuickSelectSketch.java @@ -239,6 +239,7 @@ private void advanceEpoch() { //noinspection NonAtomicOperationOnVolatileField // this increment of a volatile field is done within the scope of the propagation // synchronization and hence is done by a single thread + // Ignore a FindBugs warning epoch_++; endPropagation(null, true); initBgPropagationService(); diff --git a/src/main/java/com/yahoo/sketches/theta/Sketch.java b/src/main/java/com/yahoo/sketches/theta/Sketch.java index 948a71401..e03b6bb33 100644 --- a/src/main/java/com/yahoo/sketches/theta/Sketch.java +++ b/src/main/java/com/yahoo/sketches/theta/Sketch.java @@ -20,6 +20,7 @@ import static com.yahoo.sketches.theta.PreambleUtil.SER_VER_BYTE; import com.yahoo.memory.Memory; +import com.yahoo.memory.WritableMemory; import com.yahoo.sketches.BinomialBoundsN; import com.yahoo.sketches.Family; import com.yahoo.sketches.SketchesArgumentException; @@ -142,6 +143,38 @@ else if (serVer == 2) { //Sketch interface, defined here with Javadocs + /** + * Converts this sketch to an ordered CompactSketch on the Java heap. + * + *

If this sketch is already in compact form this operation returns this. + * + * @return this sketch as an ordered CompactSketch on the Java heap. + */ + public abstract CompactSketch compact(); + + /** + * Convert this sketch to a CompactSketch in the chosen form. + * + *

If this sketch is already in compact form this operation returns this. + * + *

Otherwise, this compacting process converts the hash table form of an UpdateSketch to + * a simple list of the valid hash values from the hash table. Any hash values equal to or + * greater than theta will be discarded. The number of valid values remaining in the + * Compact Sketch depends on a number of factors, but may be larger or smaller than + * Nominal Entries (or k). It will never exceed 2k. If it is critical + * to always limit the size to no more than k, then rebuild() should be called + * on the UpdateSketch prior to this. + * + * @param dstOrdered + * See Destination Ordered + * + * @param dstMem + * See Destination Memory. + * + * @return this sketch as a CompactSketch in the chosen form + */ + public abstract CompactSketch compact(final boolean dstOrdered, final WritableMemory dstMem); + /** * Gets the number of hash values less than the given theta. * @param theta the given theta as a double between zero and one. diff --git a/src/main/java/com/yahoo/sketches/theta/UnionImpl.java b/src/main/java/com/yahoo/sketches/theta/UnionImpl.java index 13674bc53..939936745 100644 --- a/src/main/java/com/yahoo/sketches/theta/UnionImpl.java +++ b/src/main/java/com/yahoo/sketches/theta/UnionImpl.java @@ -37,6 +37,15 @@ * @author Kevin Lang */ final class UnionImpl extends Union { + /** + * Although the gadget object is initially an UpdateSketch, in the context of a Union it is used + * as a specialized buffer that happens to leverage much of the machinery of an UpdateSketch. + * However, in this context some of the key invariants of the sketch algorithm are intentionally + * violated as an optimization. As a result this object can not be considered as an UpdateSketch + * and should never be exported as an UpdateSketch. It's internal state is not necessarily + * finalized and may contain garbage. Also its internal concept of "nominal entries" or "k" can + * be meaningless. It is private for very good reasons. + */ private final UpdateSketch gadget_; private final short seedHash_; //eliminates having to compute the seedHash on every update. private long unionThetaLong_; //when on-heap, this is the only copy diff --git a/src/main/java/com/yahoo/sketches/theta/UpdateSketch.java b/src/main/java/com/yahoo/sketches/theta/UpdateSketch.java index e1b943ced..ef8a8f7c3 100644 --- a/src/main/java/com/yahoo/sketches/theta/UpdateSketch.java +++ b/src/main/java/com/yahoo/sketches/theta/UpdateSketch.java @@ -115,50 +115,11 @@ public static UpdateSketch heapify(final Memory srcMem, final long seed) { //Sketch interface @Override - public boolean isCompact() { - return false; + public CompactSketch compact() { + return compact(true, null); } @Override - public boolean isOrdered() { - return false; - } - - //UpdateSketch interface - - /** - * Returns a new builder - * - * @return a new builder - */ - public static final UpdateSketchBuilder builder() { - return new UpdateSketchBuilder(); - } - - /** - * Resets this sketch back to a virgin empty state. - */ - public abstract void reset(); - - /** - * Convert this UpdateSketch to a CompactSketch in the chosen form. - * - *

This compacting process converts the hash table form of an UpdateSketch to - * a simple list of the valid hash values from the hash table. Any hash values equal to or - * greater than theta will be discarded. The number of valid values remaining in the - * Compact Sketch depends on a number of factors, but may be larger or smaller than - * Nominal Entries (or k). It will never exceed 2k. If it is critical - * to always limit the size to no more than k, then rebuild() should be called - * on the UpdateSketch prior to this. - * - * @param dstOrdered - * See Destination Ordered - * - * @param dstMem - * See Destination Memory. - * - * @return this sketch as a CompactSketch in the chosen form - */ public CompactSketch compact(final boolean dstOrdered, final WritableMemory dstMem) { CompactSketch sketchOut = null; final int sw = (dstOrdered ? 2 : 0) | ((dstMem != null) ? 1 : 0); @@ -184,14 +145,32 @@ public CompactSketch compact(final boolean dstOrdered, final WritableMemory dstM return sketchOut; } + @Override + public boolean isCompact() { + return false; + } + + @Override + public boolean isOrdered() { + return false; + } + + //UpdateSketch interface + /** - * Converts this UpdateSketch to an ordered CompactSketch on the Java heap. - * @return this sketch as an ordered CompactSketch on the Java heap. + * Returns a new builder + * + * @return a new builder */ - public CompactSketch compact() { - return compact(true, null); + public static final UpdateSketchBuilder builder() { + return new UpdateSketchBuilder(); } + /** + * Resets this sketch back to a virgin empty state. + */ + public abstract void reset(); + /** * Rebuilds the hash table to remove dirty values or to reduce the size * to nominal entries. diff --git a/src/test/java/com/yahoo/sketches/theta/CompactSketchTest.java b/src/test/java/com/yahoo/sketches/theta/CompactSketchTest.java index 1ea5d0c89..f137d89a1 100644 --- a/src/test/java/com/yahoo/sketches/theta/CompactSketchTest.java +++ b/src/test/java/com/yahoo/sketches/theta/CompactSketchTest.java @@ -241,6 +241,8 @@ public void checkDirectCompactSingleItemSketch() { sk.update(1); csk = sk.compact(true, WritableMemory.allocate(16)); assertEquals(csk.getCurrentBytes(true), 16); + assertTrue(csk == csk.compact()); + assertTrue(csk == csk.compact(true, null)); } @Test