Skip to content

Commit a59f199

Browse files
committed
minor tweaks
1 parent 7b33321 commit a59f199

2 files changed

Lines changed: 15 additions & 24 deletions

File tree

src/main/java/org/apache/datasketches/theta/CompactOperations.java

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -196,17 +196,14 @@ static CompactSketch segmentToCompact(
196196
}
197197
}
198198

199-
private static final void checkFamilyAndFlags(
199+
private static void checkFamilyAndFlags(
200200
final int srcFamId,
201201
final boolean srcCompactFlag,
202202
final boolean srcReadOnlyFlag) {
203203
final Family srcFamily = Family.idToFamily(srcFamId);
204204
if (srcCompactFlag) {
205205
if ((srcFamily == Family.COMPACT) && srcReadOnlyFlag) { return; }
206-
} else {
207-
if (srcFamily == Family.ALPHA) { return; }
208-
if (srcFamily == Family.QUICKSELECT) { return; }
209-
}
206+
} else if ((srcFamily == Family.ALPHA) || (srcFamily == Family.QUICKSELECT)) { return; }
210207
throw new SketchesArgumentException(
211208
"Possible Corruption: Family does not match flags: Family: "
212209
+ srcFamily.toString()
@@ -217,7 +214,7 @@ private static final void checkFamilyAndFlags(
217214
//All arguments must be valid and correct including flags.
218215
// Used as helper to create byte arrays as well as loading MemorySegment for direct compact sketches
219216
//Input must be writable, return can be Read Only
220-
static final MemorySegment loadCompactMemorySegment(
217+
static MemorySegment loadCompactMemorySegment(
221218
final long[] compactHashArr,
222219
final short seedHash,
223220
final int curCount,
@@ -236,12 +233,9 @@ static final MemorySegment loadCompactMemorySegment(
236233
}
237234
final byte famID = (byte) Family.COMPACT.getID();
238235

239-
//Caution: The following loads directly into a MemorySegment without creating a heap byte[] first,
240-
// which would act as a pre-clearing, initialization mechanism. So it is important to make sure
241-
// that all fields are initialized, even those that are not used by the CompactSketch.
242-
// Otherwise, uninitialized fields could be filled with off-heap garbage, which could cause
243-
// other problems downstream if those fields are not filtered out first.
244-
// As written below, all fields are initialized avoiding an extra copy.
236+
//It is important to make sure that all byte fields are initialized, even those that are not used by the CompactSketch.
237+
// Otherwise, uninitialized fields could could cause other problems downstream.
238+
// As written below, all fields are initialized.
245239

246240
//The first 8 bytes (pre0)
247241
insertPreLongs(dstWSeg, preLongs); //RF not used = 0
@@ -265,10 +259,9 @@ static final MemorySegment loadCompactMemorySegment(
265259
insertThetaLong(dstWSeg, thetaLong);
266260
}
267261
if (curCount > 0) { //theta could be < 1.0.
268-
//dstWSeg.putLongArray(preLongs << 3, compactHashArr, 0, curCount);
269262
MemorySegment.copy(compactHashArr, 0, dstWSeg, JAVA_LONG_UNALIGNED, preLongs << 3, curCount);
270263
}
271-
return dstWSeg; //if prelongs == 3 & curCount == 0, theta could be < 1.0. This can be RO
264+
return dstWSeg; //if prelongs == 3 & curCount == 0, theta could be < 1.0. This can be read-only
272265
}
273266

274267
/**
@@ -283,7 +276,7 @@ static final MemorySegment loadCompactMemorySegment(
283276
* @param dstOrdered true if output array must be sorted
284277
* @return the compacted array.
285278
*/
286-
static final long[] compactCache(final long[] srcCache, final int curCount,
279+
static long[] compactCache(final long[] srcCache, final int curCount,
287280
final long thetaLong, final boolean dstOrdered) {
288281
if (curCount == 0) {
289282
return new long[0];
@@ -341,7 +334,7 @@ static final long[] compactCache(final long[] srcCache, final int curCount,
341334
* @param thetaLong the given thetaLong
342335
* @return thetaLong
343336
*/
344-
static final long correctThetaOnCompact(final boolean empty, final int curCount,
337+
static long correctThetaOnCompact(final boolean empty, final int curCount,
345338
final long thetaLong) { //handles #4 above
346339
return (empty && (curCount == 0)) ? Long.MAX_VALUE : thetaLong;
347340
}
@@ -353,7 +346,7 @@ static final long correctThetaOnCompact(final boolean empty, final int curCount,
353346
* @param empty the given empty state
354347
* @param curCount the given current count
355348
*/ //This handles #2 and #6 above
356-
static final void checkIllegalCurCountAndEmpty(final boolean empty, final int curCount) {
349+
static void checkIllegalCurCountAndEmpty(final boolean empty, final int curCount) {
357350
if (empty && (curCount != 0)) { //this handles #2 and #6 above
358351
throw new SketchesStateException("Illegal State: Empty=true and Current Count != 0.");
359352
}
@@ -368,7 +361,7 @@ static final void checkIllegalCurCountAndEmpty(final boolean empty, final int cu
368361
* @param thetaLong the current thetaLong
369362
* @return the number of preamble longs
370363
*/
371-
static final int computeCompactPreLongs(final boolean empty, final int curCount,
364+
static int computeCompactPreLongs(final boolean empty, final int curCount,
372365
final long thetaLong) {
373366
return (thetaLong < Long.MAX_VALUE) ? 3 : empty ? 1 : (curCount > 1) ? 2 : 1;
374367
}
@@ -380,7 +373,7 @@ static final int computeCompactPreLongs(final boolean empty, final int curCount,
380373
* @param thetaLong the given thetaLong
381374
* @return true if notEmpty, curCount = 1 and theta = 1.0;
382375
*/
383-
static final boolean isSingleItem(final boolean empty, final int curCount,
376+
static boolean isSingleItem(final boolean empty, final int curCount,
384377
final long thetaLong) {
385378
return !empty && (curCount == 1) && (thetaLong == Long.MAX_VALUE);
386379
}

src/main/java/org/apache/datasketches/theta/HeapCompactSketch.java

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ final class HeapCompactSketch extends CompactSketch {
7676

7777
@Override
7878
public CompactSketch compact(final boolean dstOrdered, final MemorySegment dstSeg) {
79-
if (dstSeg == null && (dstOrdered == false || this.ordered_ == dstOrdered)) { return this; }
79+
if ((dstSeg == null) && (!dstOrdered || (ordered_ == dstOrdered))) { return this; }
8080
return componentsToCompact(getThetaLong(), getRetainedEntries(true), getSeedHash(), isEmpty(),
8181
true, ordered_, dstOrdered, dstSeg, getCache().clone());
8282
}
@@ -142,11 +142,9 @@ public byte[] toByteArray() {
142142
final int emptyBit = isEmpty() ? EMPTY_FLAG_MASK : 0;
143143
final int orderedBit = ordered_ ? ORDERED_FLAG_MASK : 0;
144144
final int singleItemBit = singleItem_ ? SINGLEITEM_FLAG_MASK : 0;
145-
final byte flags = (byte) (emptyBit | READ_ONLY_FLAG_MASK | COMPACT_FLAG_MASK
146-
| orderedBit | singleItemBit);
145+
final byte flags = (byte) (emptyBit | READ_ONLY_FLAG_MASK | COMPACT_FLAG_MASK | orderedBit | singleItemBit);
147146
final int preLongs = getCompactPreambleLongs();
148-
loadCompactMemorySegment(getCache(), getSeedHash(), getRetainedEntries(true), getThetaLong(),
149-
dstSeg, flags, preLongs);
147+
loadCompactMemorySegment(getCache(), getSeedHash(), getRetainedEntries(true), getThetaLong(), dstSeg, flags, preLongs);
150148
return byteArray;
151149
}
152150

0 commit comments

Comments
 (0)