Skip to content

Commit 3ec34cb

Browse files
leerholeerho
committed
Many minor changes.
Extensive comments are part of the GitHub PR.
1 parent 29a9434 commit 3ec34cb

19 files changed

Lines changed: 107 additions & 149 deletions

‎src/main/java/org/apache/datasketches/quantiles/CompactDoublesSketch.java‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,4 @@ public void update(final double quantile) {
5050
throw new SketchesStateException("Cannot update a compact sketch, which is read-only.");
5151
}
5252

53-
@Override
54-
void setReadOnly() {
55-
//No-op: A Compact Sketch is already read-only.
56-
}
57-
5853
}

‎src/main/java/org/apache/datasketches/quantiles/DirectCompactDoublesSketch.java‎

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -67,11 +67,12 @@
6767
*/
6868
final class DirectCompactDoublesSketch extends CompactDoublesSketch {
6969
private static final int MIN_DIRECT_DOUBLES_SER_VER = 3;
70-
private MemorySegment seg_;
70+
private final MemorySegment seg_;
7171

7272
//**CONSTRUCTORS**********************************************************
73-
private DirectCompactDoublesSketch(final int k) {
73+
private DirectCompactDoublesSketch(final int k, final MemorySegment seg) {
7474
super(k); //Checks k
75+
seg_ = seg.asReadOnly();
7576
}
7677

7778
/**
@@ -128,17 +129,14 @@ static DirectCompactDoublesSketch createFromUpdateSketch(final UpdateDoublesSket
128129
}
129130
}
130131

131-
final DirectCompactDoublesSketch dcds = new DirectCompactDoublesSketch(k);
132-
dcds.seg_ = dstSeg;
133-
134-
return dcds;
132+
return new DirectCompactDoublesSketch(k, dstSeg);
135133
}
136134

137135
/**
138136
* Wrap this sketch around the given compact MemorySegment image of a DoublesSketch.
139137
*
140-
* @param srcSeg the given compact MemorySegment image of a DoublesSketch that may have data,
141-
* @return a sketch that wraps the given srcSeg
138+
* @param srcSeg the given compact MemorySegment image of a DoublesSketch,
139+
* @return a sketch that wraps the given srcSeg.
142140
*/
143141
static DirectCompactDoublesSketch wrapInstance(final MemorySegment srcSeg) {
144142
final long segCap = srcSeg.byteSize();
@@ -161,9 +159,7 @@ static DirectCompactDoublesSketch wrapInstance(final MemorySegment srcSeg) {
161159
checkDirectSegCapacity(k, n, segCap);
162160
DirectUpdateDoublesSketch.checkEmptyAndN(empty, n);
163161

164-
final DirectCompactDoublesSketch dds = new DirectCompactDoublesSketch(k);
165-
dds.seg_ = srcSeg;
166-
return dds;
162+
return new DirectCompactDoublesSketch(k, srcSeg);
167163
}
168164

169165
@Override

‎src/main/java/org/apache/datasketches/quantiles/DirectUpdateDoublesSketch.java‎

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
import static org.apache.datasketches.quantiles.PreambleUtil.insertSerVer;
5353

5454
import java.lang.foreign.MemorySegment;
55+
import java.util.Objects;
5556

5657
import org.apache.datasketches.common.Family;
5758
import org.apache.datasketches.common.MemorySegmentRequest;
@@ -83,13 +84,15 @@ private DirectUpdateDoublesSketch(final int k, final MemorySegment seg, final Me
8384
*
8485
* @param k Parameter that controls space usage of sketch and accuracy of estimates.
8586
* Must be greater than 1 and less than 65536 and a power of 2.
86-
* @param dstSeg the destination MemorySegment that will be initialized to hold the data for this sketch.
87+
* @param dstSeg the non-null destination MemorySegment that will be initialized to hold the data for this sketch.
8788
* It must initially be at least (16 * MIN_K + 32) bytes, where MIN_K defaults to 2. As it grows
8889
* it will request more MemorySegment using the MemorySegmentRequest callback.
90+
* @param mSegReq the MemorySegmentRequest used if the incoming MemorySegment needs to expand.
91+
* Otherwise, it can be null and the default MemorySegmentRequest will be used.
8992
* @return a DirectUpdateDoublesSketch
9093
*/
9194
static DirectUpdateDoublesSketch newInstance(final int k, final MemorySegment dstSeg, final MemorySegmentRequest mSegReq) {
92-
// must be able to hold at least an empty sketch
95+
Objects.requireNonNull(dstSeg, "The MemorySegment dstSeg must not be null");
9396
final long segCap = dstSeg.byteSize();
9497
checkDirectSegCapacity(k, 0, segCap);
9598

@@ -110,20 +113,16 @@ static DirectUpdateDoublesSketch newInstance(final int k, final MemorySegment ds
110113
return new DirectUpdateDoublesSketch(k, dstSeg, mSegReq);
111114
}
112115

113-
static HeapUpdateDoublesSketch heapify(final DirectUpdateDoublesSketch skIn) {
114-
final MemorySegment segIn = skIn.getMemorySegment();
115-
return HeapUpdateDoublesSketch.heapifyInstance(segIn);
116-
}
117-
118116
/**
119117
* Wrap this sketch around the given updatable MemorySegment image of a DoublesSketch.
120118
*
121-
* @param srcSeg the given non-compact MemorySegment image of a DoublesSketch that may have data
122-
* @param mSegReq the MemorySegmentRequest used as a callback for more space if required.
123-
* If it is null, the default MemorySegmentRequest will be used.
119+
* @param srcSeg the given MemorySegment image of an UpdateDoublesSketch and must not be null.
120+
* @param mSegReq the MemorySegmentRequest used if the incoming MemorySegment needs to expand.
121+
* Otherwise, it can be null and the default MemorySegmentRequest will be used.
124122
* @return a sketch that wraps the given srcSeg
125123
*/
126124
static DirectUpdateDoublesSketch wrapInstance(final MemorySegment srcSeg, final MemorySegmentRequest mSegReq) {
125+
Objects.requireNonNull(srcSeg, "The source MemorySegment must not be null");
127126
final long segCap = srcSeg.byteSize();
128127

129128
final int preLongs = extractPreLongs(srcSeg);
@@ -294,14 +293,9 @@ MemorySegment getMemorySegment() {
294293
return seg_;
295294
}
296295

297-
@Override
298-
void setReadOnly() {
299-
seg_ = seg_.asReadOnly();
300-
}
301-
302296
@Override
303297
UpdateDoublesSketch getSketchAndReset() {
304-
final HeapUpdateDoublesSketch skCopy = heapify(this);
298+
final HeapUpdateDoublesSketch skCopy = HeapUpdateDoublesSketch.heapifyInstance(seg_);
305299
reset();
306300
return skCopy;
307301
}

‎src/main/java/org/apache/datasketches/quantiles/DoublesSketch.java‎

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -393,13 +393,14 @@ public static String toString(final MemorySegment seg) {
393393
* @param smallerK the new sketch's K that must be smaller than this K.
394394
* It is required that this.getK() = smallerK * 2^(nonnegative integer).
395395
* @param dstSeg the destination MemorySegment. It must not overlap the MemorySegment of this sketch.
396-
* If null, a heap sketch will be returned, otherwise it will be off-heap.
397-
*
396+
* If null, a heap sketch will be returned, otherwise it will be MemorySegment based.
397+
* @param mSegReq the MemorySegmentRequest used if the incoming MemorySegment needs to expand.
398+
* Otherwise, it can be null and the default MemorySegmentRequest will be used.
398399
* @return the new sketch.
399400
*/
400-
public DoublesSketch downSample(final DoublesSketch srcSketch, final int smallerK,
401-
final MemorySegment dstSeg) {
402-
return downSampleInternal(srcSketch, smallerK, dstSeg);
401+
public DoublesSketch downSample(final DoublesSketch srcSketch, final int smallerK, final MemorySegment dstSeg,
402+
final MemorySegmentRequest mSegReq) {
403+
return downSampleInternal(srcSketch, smallerK, dstSeg, mSegReq);
403404
}
404405

405406
@Override
@@ -500,11 +501,6 @@ public QuantilesDoublesSketchIterator iterator() {
500501
return new DoublesSketchIterator(this, getBitPattern());
501502
}
502503

503-
/**
504-
* {@inheritDoc}
505-
*
506-
* <p>The parameter <i>k</i> will not change.</p>
507-
*/
508504
@Override
509505
public abstract void reset();
510506

@@ -516,11 +512,11 @@ public QuantilesDoublesSketchIterator iterator() {
516512
* specifies a DoublesSketch. This lets us be more specific about the type without changing the
517513
* public API.
518514
*/
519-
UpdateDoublesSketch downSampleInternal(final DoublesSketch srcSketch, final int smallerK,
520-
final MemorySegment dstSeg) {
515+
UpdateDoublesSketch downSampleInternal(final DoublesSketch srcSketch, final int smallerK, final MemorySegment dstSeg,
516+
final MemorySegmentRequest mSegReq) {
521517
final UpdateDoublesSketch newSketch = dstSeg == null
522518
? HeapUpdateDoublesSketch.newInstance(smallerK)
523-
: DirectUpdateDoublesSketch.newInstance(smallerK, dstSeg, null);
519+
: DirectUpdateDoublesSketch.newInstance(smallerK, dstSeg, mSegReq);
524520
if (srcSketch.isEmpty()) { return newSketch; }
525521
DoublesMergeImpl.downSamplingMergeInto(srcSketch, newSketch);
526522
return newSketch;
@@ -564,11 +560,6 @@ UpdateDoublesSketch downSampleInternal(final DoublesSketch srcSketch, final int
564560
*/
565561
abstract MemorySegment getMemorySegment();
566562

567-
/**
568-
* Sets the internal MemorySegment to Read Only.
569-
*/
570-
abstract void setReadOnly();
571-
572563
//************SORTED VIEW****************************
573564

574565
@Override

‎src/main/java/org/apache/datasketches/quantiles/DoublesSketchBuilder.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,8 @@ public UpdateDoublesSketch build(final MemorySegment dstSeg) {
8787
* Returns a UpdateDoublesSketch with the current configuration of this builder
8888
* and the specified backing destination MemorySegment store that can grow.
8989
* @param dstSeg destination MemorySegment for use by the sketch
90-
* @param mSegReq a user developed MemorySegmentRequest or null.
90+
* @param mSegReq the MemorySegmentRequest used if the incoming MemorySegment needs to expand.
91+
* Otherwise, it can be null and the default MemorySegmentRequest will be used.
9192
* @return an UpdateDoublesSketch
9293
*/
9394
public UpdateDoublesSketch build(final MemorySegment dstSeg, final MemorySegmentRequest mSegReq) {

‎src/main/java/org/apache/datasketches/quantiles/DoublesUnion.java‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
import java.lang.foreign.MemorySegment;
2323

24+
import org.apache.datasketches.common.MemorySegmentRequest;
2425
import org.apache.datasketches.common.MemorySegmentStatus;
2526

2627
/**
@@ -62,11 +63,13 @@ public static DoublesUnion heapify(final MemorySegment srcSeg) {
6263
* Returns an updatable Union object that wraps the given MemorySegment that contains an image of a sketch.
6364
* The data structures of the Union remain in the MemorySegment.
6465
*
65-
* @param seg A MemorySegment to be used as the data structure for the sketch and will be modified.
66+
* @param srcSeg A MemorySegment to be used as the data structure for the sketch and will be modified.
67+
* @param mSegReq the MemorySegmentRequest used if the incoming MemorySegment needs to expand.
68+
* Otherwise, it can be null and the default MemorySegmentRequest will be used.
6669
* @return a Union object
6770
*/
68-
public static DoublesUnion writableWrap(final MemorySegment seg) {
69-
return DoublesUnionImpl.wrapInstance(seg);
71+
public static DoublesUnion writableWrap(final MemorySegment srcSeg, final MemorySegmentRequest mSegReq) {
72+
return DoublesUnionImpl.wrapInstance(srcSeg, mSegReq);
7073
}
7174

7275
@Override
@@ -148,9 +151,11 @@ public static DoublesUnion writableWrap(final MemorySegment seg) {
148151
* been changed, which allows further union operations.
149152
*
150153
* @param dstSeg the destination MemorySegment for the result
154+
* @param mSegReq the MemorySegmentRequest used if the incoming MemorySegment needs to expand.
155+
* Otherwise, it can be null and the default MemorySegmentRequest will be used.
151156
* @return the result of this Union operation
152157
*/
153-
public abstract UpdateDoublesSketch getResult(MemorySegment dstSeg);
158+
public abstract UpdateDoublesSketch getResult(MemorySegment dstSeg, MemorySegmentRequest mSegReq);
154159

155160
/**
156161
* Gets the result of this Union as an UpdateDoublesSketch, which enables further update

‎src/main/java/org/apache/datasketches/quantiles/DoublesUnionBuilder.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@
2121

2222
import java.lang.foreign.MemorySegment;
2323

24+
import org.apache.datasketches.common.MemorySegmentRequest;
25+
2426
/**
2527
* For building a new DoublesSketch Union operation.
2628
*
@@ -72,10 +74,12 @@ public DoublesUnion build() {
7274
* Returns a new empty Union object with the current configuration of this Builder
7375
* and the specified backing destination MemorySegment store.
7476
* @param dstSeg the destination MemorySegment
77+
* @param mSegReq the MemorySegmentRequest used if the incoming MemorySegment needs to expand.
78+
* Otherwise, it can be null and the default MemorySegmentRequest will be used.
7579
* @return a Union object
7680
*/
77-
public DoublesUnion build(final MemorySegment dstSeg) {
78-
return DoublesUnionImpl.directInstance(bMaxK, dstSeg);
81+
public DoublesUnion build(final MemorySegment dstSeg, final MemorySegmentRequest mSegReq) {
82+
return DoublesUnionImpl.directInstance(bMaxK, dstSeg, mSegReq);
7983
}
8084

8185
}

‎src/main/java/org/apache/datasketches/quantiles/DoublesUnionImpl.java‎

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import java.lang.foreign.MemorySegment;
2626
import java.util.Objects;
2727

28+
import org.apache.datasketches.common.MemorySegmentRequest;
2829
import org.apache.datasketches.common.SketchesArgumentException;
2930
import org.apache.datasketches.common.SketchesReadOnlyException;
3031

@@ -55,19 +56,21 @@ static DoublesUnionImpl heapInstance(final int maxK) {
5556
}
5657

5758
/**
58-
* Returns a empty DoublesUnion object that refers to the given MemorySegment,
59-
* which will be initialized to the empty state.
59+
* Returns a empty DoublesUnion object that uses the given MemorySegment for its internal sketch gadget
60+
* and will be initialized to the empty state.
6061
*
6162
* @param maxK determines the accuracy and size of the union and is a maximum.
6263
* The effective <i>k</i> can be smaller due to unions with smaller <i>k</i> sketches.
6364
* It is recommended that <i>maxK</i> be a power of 2 to enable unioning of sketches with
6465
* different <i>k</i>.
65-
* @param dstSeg the MemorySegment to be used by the sketch
66+
* @param dstSeg the MemorySegment to be used by the internal sketch and must not be null.
67+
* @param mSegReq the MemorySegmentRequest used if the incoming MemorySegment needs to expand.
68+
* Otherwise, it can be null and the default MemorySegmentRequest will be used.
6669
* @return a DoublesUnion object
6770
*/
68-
static DoublesUnionImpl directInstance(final int maxK, final MemorySegment dstSeg) {
71+
static DoublesUnionImpl directInstance(final int maxK, final MemorySegment dstSeg, final MemorySegmentRequest mSegReq) {
6972
Objects.requireNonNull(dstSeg);
70-
final DirectUpdateDoublesSketch sketch = DirectUpdateDoublesSketch.newInstance(maxK, dstSeg, null);
73+
final DirectUpdateDoublesSketch sketch = DirectUpdateDoublesSketch.newInstance(maxK, dstSeg, mSegReq);
7174
final DoublesUnionImpl union = new DoublesUnionImpl(maxK);
7275
union.maxK_ = maxK;
7376
union.gadget_ = sketch;
@@ -112,12 +115,14 @@ static DoublesUnionImpl heapifyInstance(final MemorySegment srcSeg) {
112115
* image of a updatable DoublesSketch. The data of the Union will remain in the MemorySegment.
113116
*
114117
* @param srcSeg A writable MemorySegment image of a updatable DoublesSketch to be used as data for the union.
118+
* @param mSegReq the MemorySegmentRequest used if the incoming MemorySegment needs to expand.
119+
* Otherwise, it can be null and the default MemorySegmentRequest will be used.
115120
* @return a Union object
116121
*/
117-
static DoublesUnionImpl wrapInstance(final MemorySegment srcSeg) {
122+
static DoublesUnionImpl wrapInstance(final MemorySegment srcSeg, final MemorySegmentRequest mSegReq) {
118123
Objects.requireNonNull(srcSeg);
119124
if (srcSeg.isReadOnly()) { throw new SketchesReadOnlyException("Cannot create a Union with a Read Only MemorySegment."); }
120-
final DirectUpdateDoublesSketch sketch = DirectUpdateDoublesSketch.wrapInstance(srcSeg, null);
125+
final DirectUpdateDoublesSketch sketch = DirectUpdateDoublesSketch.wrapInstance(srcSeg, mSegReq);
121126
final DoublesUnionImpl union = new DoublesUnionImpl(sketch.getK());
122127
union.gadget_ = sketch;
123128
return union;
@@ -159,21 +164,21 @@ public UpdateDoublesSketch getResult() {
159164
if (gadget_ == null) {
160165
return HeapUpdateDoublesSketch.newInstance(maxK_);
161166
}
162-
return DoublesUtil.copyToHeap(gadget_); //can't have any externally owned handles.
167+
return DoublesUtil.copyToHeap(gadget_);
163168
}
164169

165170
@Override
166-
public UpdateDoublesSketch getResult(final MemorySegment dstSeg) {
171+
public UpdateDoublesSketch getResult(final MemorySegment dstSeg, final MemorySegmentRequest mSegReq) {
167172
final long segCapBytes = dstSeg.byteSize();
168173
if (gadget_ == null) {
169174
if (segCapBytes < DoublesSketch.getUpdatableStorageBytes(0, 0)) {
170175
throw new SketchesArgumentException("Insufficient capacity for result: " + segCapBytes);
171176
}
172-
return DirectUpdateDoublesSketch.newInstance(maxK_, dstSeg, null);
177+
return DirectUpdateDoublesSketch.newInstance(maxK_, dstSeg, mSegReq);
173178
}
174179

175180
gadget_.putIntoMemorySegment(dstSeg, false);
176-
return DirectUpdateDoublesSketch.wrapInstance(dstSeg, null);
181+
return DirectUpdateDoublesSketch.wrapInstance(dstSeg, mSegReq);
177182
}
178183

179184
@Override
@@ -268,14 +273,14 @@ static UpdateDoublesSketch updateLogic(final int myMaxK, final UpdateDoublesSket
268273
if (!other.isEstimationMode()) { //other is exact, stream items in
269274
ret = HeapUpdateDoublesSketch.newInstance(myMaxK);
270275
// exact mode, only need copy base buffer
271-
final DoublesSketchAccessor otherAccessor = DoublesSketchAccessor.wrap(other, false); // T/F doesn't matter
276+
final DoublesSketchAccessor otherAccessor = DoublesSketchAccessor.wrap(other, false);
272277
for (int i = 0; i < otherAccessor.numItems(); ++i) {
273278
ret.update(otherAccessor.get(i));
274279
}
275280
}
276281
else { //myQS = null, other is est mode
277282
ret = (myMaxK < other.getK())
278-
? other.downSampleInternal(other, myMaxK, null) //null seg
283+
? other.downSampleInternal(other, myMaxK, null, null) //null seg, null mSegReq
279284
: DoublesUtil.copyToHeap(other); //copy required because caller has handle
280285
}
281286
break;
@@ -286,7 +291,7 @@ static UpdateDoublesSketch updateLogic(final int myMaxK, final UpdateDoublesSket
286291
if (!other.isEstimationMode()) { //other is exact, stream items in
287292
ret = myQS;
288293
// exact mode, only need copy base buffer
289-
final DoublesSketchAccessor otherAccessor = DoublesSketchAccessor.wrap(other, false); // T/F doesn't matter
294+
final DoublesSketchAccessor otherAccessor = DoublesSketchAccessor.wrap(other, false);
290295
for (int i = 0; i < otherAccessor.numItems(); ++i) {
291296
ret.update(otherAccessor.get(i));
292297
}

‎src/main/java/org/apache/datasketches/quantiles/DoublesUtil.java‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -137,12 +137,11 @@ private static String getSummary(final DoublesSketch sk) {
137137
sb.append(LS).append("### Classic Quantiles ").append(thisSimpleName).append(" SUMMARY: ").append(LS);
138138
sb.append(" Empty : ").append(sk.isEmpty()).append(LS);
139139
sb.append(" Segment, Capacity bytes : ").append(sk.hasMemorySegment()).append(", ").append(segCap).append(LS);
140-
sb.append(" Segment, ReadOnly : ").append(sk.hasMemorySegment() ? sk.getMemorySegment().isReadOnly() : false).append(LS);
140+
sb.append(" Segment, ReadOnly : ").append(sk.hasMemorySegment() && sk.getMemorySegment().isReadOnly()).append(LS);
141141
sb.append(" Estimation Mode : ").append(sk.isEstimationMode()).append(LS);
142142
sb.append(" K : ").append(kStr).append(LS);
143143
sb.append(" N : ").append(nStr).append(LS);
144-
sb.append(" Levels (Needed, Total, Valid): ").append(neededLevels + ", " + totalLevels + ", " + validLevels)
145-
.append(LS);
144+
sb.append(" Levels (Needed, Total, Valid): ").append(neededLevels + ", " + totalLevels + ", " + validLevels).append(LS);
146145
sb.append(" Level Bit Pattern : ").append(Long.toBinaryString(bitPattern)).append(LS);
147146
sb.append(" Base Buffer Count : ").append(bbCntStr).append(LS);
148147
sb.append(" Combined Buffer Capacity : ").append(combBufCapStr).append(LS);
@@ -152,10 +151,8 @@ private static String getSummary(final DoublesSketch sk) {
152151
sb.append(" Updatable Storage Bytes : ").append(updtBytesStr).append(LS);
153152
sb.append(" Normalized Rank Error : ").append(epsPctStr).append(LS);
154153
sb.append(" Normalized Rank Error (PMF) : ").append(epsPmfPctStr).append(LS);
155-
sb.append(" Min Item : ")
156-
.append(String.format("%12.6e", minItem)).append(LS);
157-
sb.append(" Max Item : ")
158-
.append(String.format("%12.6e", maxItem)).append(LS);
154+
sb.append(" Min Item : ").append(String.format("%12.6e", minItem)).append(LS);
155+
sb.append(" Max Item : ").append(String.format("%12.6e", maxItem)).append(LS);
159156
sb.append("### END SKETCH SUMMARY").append(LS);
160157
return sb.toString();
161158
}

0 commit comments

Comments
 (0)