leerho commented on a change in pull request #324:
URL:
https://github.com/apache/incubator-datasketches-java/pull/324#discussion_r455403972
##########
File path: src/main/java/org/apache/datasketches/theta/HeapAlphaSketch.java
##########
@@ -128,20 +128,19 @@ static HeapAlphaSketch heapifyInstance(final Memory
srcMem, final long seed) {
checkMemIntegrity(srcMem, seed, preambleLongs, lgNomLongs, lgArrLongs);
final float p = extractP(srcMem); //bytes 12-15
- final int lgRF = extractLgResizeFactor(srcMem); //byte 0
- final ResizeFactor myRF = ResizeFactor.getRF(lgRF);
+ final int memlgRF = extractLgResizeFactor(srcMem); //byte 0
+ final ResizeFactor memRF = ResizeFactor.getRF(memlgRF);
final double nomLongs = (1L << lgNomLongs);
final double alpha = nomLongs / (nomLongs + 1.0);
final long split1 = (long) (((p * (alpha + 1.0)) / 2.0) *
LONG_MAX_VALUE_AS_DOUBLE);
- if ((myRF == ResizeFactor.X1)
- && (lgArrLongs != startingSubMultiple(lgNomLongs + 1, myRF,
MIN_LG_ARR_LONGS))) {
- throw new SketchesArgumentException("Possible corruption: ResizeFactor
X1, but provided "
- + "array too small for sketch size");
+ if (isResizeFactorIncorrect(srcMem, lgNomLongs, lgArrLongs)) {
+ throw new SketchesArgumentException("Possible corruption: ResizeFactor "
+ + "inconsistent with lgNomLongs and lgArrLongs.");
Review comment:
The DirectQuickSelectSketch input is a WritableMemory srcMem, and the
operation is a WritableWrap, which allows the sketch to be **operated on**
while in off-heap memory. Here I clearly want to make sure the ResizeFactor is
correct in case I need to continue to grow the sketch off-heap.
In the HeapQuickSelectSketch the operation is heapifyInstance which means I
am putting a sketch that was off-heap onto the Java heap, thus discarding the
srcMem object. Here I only need to make sure I have a working value of
ResizeFactor in case I need to continue growing the sketch on-heap.
In the HeapAlphaSketch I could have used the same strategy as the
HeapQuickSelectSketch, instead of throwing.
This is a very observant catch! Thank you!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
