Skip to content

Commit 587f2cd

Browse files
committed
I didn't like the implementation of DS.common/MemorySegmentStatus.
So this new one takes advantage of a new FFM method.
1 parent 89fb019 commit 587f2cd

4 files changed

Lines changed: 22 additions & 32 deletions

File tree

src/main/java/org/apache/datasketches/common/MemorySegmentStatus.java

Lines changed: 18 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
package org.apache.datasketches.common;
2121

2222
import java.lang.foreign.MemorySegment;
23+
import java.util.Optional;
2324

2425
/**
2526
* Methods for inquiring the status of a backing MemorySegment.
@@ -40,49 +41,38 @@ public interface MemorySegmentStatus {
4041
boolean isOffHeap();
4142

4243
/**
43-
* Returns true if an internally referenced MemorySegment refers to the same MemorySegment as <i>that</i>.
44-
* They can either have the same off-heap memory location and size, or refer to the same on-heap array object.
44+
* Returns true if an internally referenced MemorySegment has the same backing resource as <i>that</i>,
45+
* or equivalently, if their two memory regions overlap. This applies to both on-heap and off-heap MemorySegments.
4546
*
46-
* <p>If both segment are off-heap, they both must have the same starting address and the same size.</p>
47+
* <p>This returns false if either segment is <i>null</i> or not alive.</p>
4748
*
48-
* <p>For on-heap segments, both segments must be based on or derived from the same array object and neither segment
49-
* can be read-only.</p>
50-
*
51-
* <p>Returns false if either argument is null;</p>
49+
* <p><b>Note:</b> If both segments are on-heap and not read-only, it can be determined if they were derived from
50+
* the same backing memory (array). However, this is not always possible off-heap. Because of this asymmetry, this definition
51+
* of "isSameResource" is confined to the existence of an overlap.</p>
5252
*
5353
* @param that The given MemorySegment.
54-
* @return true if an internally referenced MemorySegment refers to the same MemorySegment as <i>that</i>.
54+
* @return true if an internally referenced MemorySegment has the same backing resource as <i>that</i>.
5555
*/
5656
boolean isSameResource(final MemorySegment that);
5757

5858
/**
59-
* Returns true if the two given MemorySegments refer to the same backing resource,
60-
* which is either an off-heap memory address and size, or the same on-heap array object.
61-
*
62-
* <p>If both segment are off-heap, they both must have the same starting address and the same size.</p>
59+
* Returns true if the two given MemorySegments have to the same backing resource, or equivalently,
60+
* if the two memory regions overlap. This applies to both on-heap and off-heap MemorySegments.
6361
*
64-
* <p>For on-heap segments, both segments must be based on or derived from the same array object and neither segment
65-
* can be read-only.</p>
62+
* <p>This returns false if either segment is <i>null</i> or not alive.</p>
6663
*
67-
* <p>Returns false if either argument is null;</p>
64+
* <p><b>Note:</b> If both segments are on-heap and not read-only, it can be determined if they were derived from
65+
* the same backing memory (array). However, this is not always possible off-heap. Because of this asymmetry, this definition
66+
* of "isSameResource" is confined to the existence of an overlap.</p>
6867
*
6968
* @param seg1 The first given MemorySegment
7069
* @param seg2 The second given MemorySegment
71-
* @return true if both MemorySegments are determined to be the same backing memory.
70+
* @return true if the two given MemorySegments have to the same backing resource.
7271
*/
7372
static boolean isSameResource(final MemorySegment seg1, final MemorySegment seg2) {
74-
if ((seg1 == null) || (seg2 == null)) { return false; }
75-
if (!seg1.scope().isAlive() || !seg2.scope().isAlive()) {
76-
throw new IllegalArgumentException("Both arguments must be alive.");
77-
}
78-
final boolean seg1Native = seg1.isNative();
79-
final boolean seg2Native = seg2.isNative();
80-
if (seg1Native ^ seg2Native) { return false; }
81-
if (seg1Native && seg2Native) { //both off heap
82-
return (seg1.address() == seg2.address()) && (seg1.byteSize() == seg2.byteSize());
83-
}
84-
//both on heap
85-
return seg1 == seg2;
73+
if ((seg1 == null) || (seg2 == null) || !seg1.scope().isAlive() || !seg2.scope().isAlive()) { return false; }
74+
final Optional<MemorySegment> opt = seg1.asOverlappingSlice(seg2);
75+
return opt.isPresent();
8676
}
8777

8878
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ public boolean isOrdered() {
181181

182182
@Override
183183
public boolean isSameResource(final MemorySegment that) {
184-
return ((this instanceof DirectQuickSelectSketchR) && ((DirectQuickSelectSketchR)this).isSameResource(that));
184+
return (this instanceof final DirectQuickSelectSketchR dqssr) && dqssr.isSameResource(that);
185185
}
186186

187187
//UpdateSketch interface

src/test/java/org/apache/datasketches/theta/SketchTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,7 @@ public void checkIsSameResource() {
337337
sketch.update(2);
338338
assertTrue(sketch.isSameResource(seg));
339339
final DirectCompactSketch dcos = (DirectCompactSketch) sketch.compact(true, cseg);
340-
assertTrue(MemorySegmentStatus.isSameResource(dcos.getMemorySegment(), cseg));
340+
assertTrue(MemorySegmentStatus.isSameResource(dcos.getMemorySegment(), cseg));
341341
assertTrue(dcos.isOrdered());
342342
//never create 2 sketches with the same MemorySegment, so don't do as I do :)
343343
final DirectCompactSketch dcs = (DirectCompactSketch) sketch.compact(false, cseg);

src/test/java/org/apache/datasketches/theta/UnionImplTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -227,9 +227,9 @@ public void checkMoveAndResizeOffHeap() {
227227

228228
//Here we create a 2nd Union with a heap segment
229229
final MemorySegment uWsegHeap = MemorySegment.ofArray(new byte[bytes / 2]);
230-
final Union union2 = SetOperation.builder().buildUnion(); //on-heap union
230+
final Union union2 = SetOperation.builder().buildUnion(uWsegHeap); //union with on-heap segment
231231
//As before, this establishes that the empty union2 has the same segment that we gave it.
232-
assertTrue(union2.isSameResource(uWsegHeap)); //But Copilot complained, so here it is!
232+
assertTrue(union2.isSameResource(uWsegHeap)); //Copilot complained this was absent, so here it is!
233233

234234
//These two asserts are calling the Interface static method directly and establish
235235
// what should be obvious, that these are all different segments.

0 commit comments

Comments
 (0)