Skip to content

Commit

Permalink
Move "owner" field and thread-confinement checks to MemoryScope
Browse files Browse the repository at this point in the history 
Reviewed-by: mcimadamore
  • Loading branch information
@mcimadamore
Peter Levart authored and mcimadamore committed May 15, 2020
1 parent 39ab618 commit 928a8af
Show file tree
Hide file tree
Showing 5 changed files with 175 additions and 111 deletions.
44 changes: 18 additions & 26 deletions src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/AbstractMemorySegmentImpl.java
View file
Expand Up @@ -71,22 +71,20 @@ public abstract class AbstractMemorySegmentImpl implements MemorySegment, Memory

final long length;
final int mask;
final Thread owner;
final MemoryScope scope;

@ForceInline
AbstractMemorySegmentImpl(long length, int mask, Thread owner, MemoryScope scope) {
AbstractMemorySegmentImpl(long length, int mask, MemoryScope scope) {
this.length = length;
this.mask = mask;
this.owner = owner;
this.scope = scope;
}

abstract long min();

abstract Object base();

abstract AbstractMemorySegmentImpl dup(long offset, long size, int mask, Thread owner, MemoryScope scope);
abstract AbstractMemorySegmentImpl dup(long offset, long size, int mask, MemoryScope scope);

abstract ByteBuffer makeByteBuffer();

Expand All @@ -103,7 +101,7 @@ public AbstractMemorySegmentImpl asSlice(long offset, long newSize) {
}

private AbstractMemorySegmentImpl asSliceNoCheck(long offset, long newSize) {
return dup(offset, newSize, mask, owner, scope);
return dup(offset, newSize, mask, scope);
}

@SuppressWarnings("unchecked")
Expand Down Expand Up @@ -155,12 +153,12 @@ public final long byteSize() {

@Override
public final boolean isAlive() {
return scope.isAliveThreadSafe();
return scope.isAlive();
}

@Override
public Thread ownerThread() {
return owner;
return scope.ownerThread();
}

@Override
Expand All @@ -169,7 +167,7 @@ public AbstractMemorySegmentImpl withAccessModes(int accessModes) {
if ((~accessModes() & accessModes) != 0) {
throw new UnsupportedOperationException("Cannot acquire more access modes");
}
return dup(0, length, (mask & ~ACCESS_MASK) | accessModes, owner, scope);
return dup(0, length, (mask & ~ACCESS_MASK) | accessModes, scope);
}

@Override
Expand All @@ -187,15 +185,14 @@ private void checkAccessModes(int accessModes) {
@Override
public MemorySegment withOwnerThread(Thread newOwner) {
Objects.requireNonNull(newOwner);
checkValidState();
if (!isSet(HANDOFF)) {
throw unsupportedAccessMode(HANDOFF);
}
if (owner == newOwner) {
if (scope.ownerThread() == newOwner) {
throw new IllegalArgumentException("Segment already owned by thread: " + newOwner);
} else {
try {
return dup(0L, length, mask, newOwner, scope.dup());
return dup(0L, length, mask, scope.dup(newOwner));
} finally {
//flush read/writes to segment memory before returning the new segment
VarHandle.fullFence();
Expand All @@ -208,7 +205,6 @@ public final void close() {
if (!isSet(CLOSE)) {
throw unsupportedAccessMode(CLOSE);
}
checkValidState();
closeNoCheck();
}

Expand All @@ -220,7 +216,7 @@ final AbstractMemorySegmentImpl acquire() {
if (Thread.currentThread() != ownerThread() && !isSet(ACQUIRE)) {
throw unsupportedAccessMode(ACQUIRE);
}
return dup(0, length, mask, Thread.currentThread(), scope.acquire());
return dup(0, length, mask, scope.acquire());
}

@Override
Expand All @@ -237,7 +233,7 @@ boolean isSmall() {
}

void checkRange(long offset, long length, boolean writeAccess) {
checkValidState();
scope.checkValidState();
if (writeAccess && !isSet(WRITE)) {
throw unsupportedAccessMode(WRITE);
} else if (!writeAccess && !isSet(READ)) {
Expand All @@ -248,10 +244,7 @@ void checkRange(long offset, long length, boolean writeAccess) {

@Override
public final void checkValidState() {
if (owner != null && owner != Thread.currentThread()) {
throw new IllegalStateException("Attempt to access segment outside owning thread");
}
scope.checkAliveConfined();
scope.checkValidState();
}

// Helper methods
Expand Down Expand Up @@ -425,29 +418,28 @@ public static AbstractMemorySegmentImpl ofBuffer(ByteBuffer bb) {
AbstractMemorySegmentImpl bufferSegment = (AbstractMemorySegmentImpl)nioAccess.bufferSegment(bb);
final MemoryScope bufferScope;
int modes;
final Thread owner;
if (bufferSegment != null) {
bufferScope = bufferSegment.scope;
modes = bufferSegment.mask;
owner = bufferSegment.owner;
} else {
bufferScope = MemoryScope.create(bb, null);
modes = defaultAccessModes(size);
owner = Thread.currentThread();
}
if (bb.isReadOnly()) {
modes &= ~WRITE;
}
if (base != null) {
return new HeapMemorySegmentImpl<>(bbAddress + pos, () -> (byte[])base, size, modes, owner, bufferScope);
return new HeapMemorySegmentImpl<>(bbAddress + pos, () -> (byte[])base, size, modes, bufferScope);
} else if (unmapper == null) {
return new NativeMemorySegmentImpl(bbAddress + pos, size, modes, owner, bufferScope);
return new NativeMemorySegmentImpl(bbAddress + pos, size, modes, bufferScope);
} else {
return new MappedMemorySegmentImpl(bbAddress + pos, unmapper, size, modes, owner, bufferScope);
return new MappedMemorySegmentImpl(bbAddress + pos, unmapper, size, modes, bufferScope);
}
}

public static AbstractMemorySegmentImpl NOTHING = new AbstractMemorySegmentImpl(0, 0, null, MemoryScope.GLOBAL) {
public static AbstractMemorySegmentImpl NOTHING = new AbstractMemorySegmentImpl(
0, 0, MemoryScope.createUnchecked(null, null, null)
) {
@Override
ByteBuffer makeByteBuffer() {
throw new UnsupportedOperationException();
Expand All @@ -464,7 +456,7 @@ Object base() {
}

@Override
AbstractMemorySegmentImpl dup(long offset, long size, int mask, Thread owner, MemoryScope scope) {
AbstractMemorySegmentImpl dup(long offset, long size, int mask, MemoryScope scope) {
throw new UnsupportedOperationException();
}
};
Expand Down
10 changes: 5 additions & 5 deletions src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/HeapMemorySegmentImpl.java
View file
Expand Up @@ -52,8 +52,8 @@ public class HeapMemorySegmentImpl<H> extends AbstractMemorySegmentImpl {
final Supplier<H> baseProvider;

@ForceInline
HeapMemorySegmentImpl(long offset, Supplier<H> baseProvider, long length, int mask, Thread owner, MemoryScope scope) {
super(length, mask, owner, scope);
HeapMemorySegmentImpl(long offset, Supplier<H> baseProvider, long length, int mask, MemoryScope scope) {
super(length, mask, scope);
this.offset = offset;
this.baseProvider = baseProvider;
}
Expand All @@ -69,8 +69,8 @@ long min() {
}

@Override
HeapMemorySegmentImpl<H> dup(long offset, long size, int mask, Thread owner, MemoryScope scope) {
return new HeapMemorySegmentImpl<H>(this.offset + offset, baseProvider, size, mask, owner, scope);
HeapMemorySegmentImpl<H> dup(long offset, long size, int mask, MemoryScope scope) {
return new HeapMemorySegmentImpl<>(this.offset + offset, baseProvider, size, mask, scope);
}

@Override
Expand Down Expand Up @@ -122,6 +122,6 @@ public static MemorySegment makeArraySegment(double[] arr) {
static <Z> HeapMemorySegmentImpl<Z> makeHeapSegment(Supplier<Z> obj, int length, int base, int scale) {
int byteSize = length * scale;
MemoryScope scope = MemoryScope.create(null, null);
return new HeapMemorySegmentImpl<>(base, obj, byteSize, defaultAccessModes(byteSize), Thread.currentThread(), scope);
return new HeapMemorySegmentImpl<>(base, obj, byteSize, defaultAccessModes(byteSize), scope);
}
}
10 changes: 5 additions & 5 deletions src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/MappedMemorySegmentImpl.java
View file
Expand Up @@ -48,8 +48,8 @@ public class MappedMemorySegmentImpl extends NativeMemorySegmentImpl implements

private final UnmapperProxy unmapper;

MappedMemorySegmentImpl(long min, UnmapperProxy unmapper, long length, int mask, Thread owner, MemoryScope scope) {
super(min, length, mask, owner, scope);
MappedMemorySegmentImpl(long min, UnmapperProxy unmapper, long length, int mask, MemoryScope scope) {
super(min, length, mask, scope);
this.unmapper = unmapper;
}

Expand All @@ -60,8 +60,8 @@ ByteBuffer makeByteBuffer() {
}

@Override
MappedMemorySegmentImpl dup(long offset, long size, int mask, Thread owner, MemoryScope scope) {
return new MappedMemorySegmentImpl(min + offset, unmapper, size, mask, owner, scope);
MappedMemorySegmentImpl dup(long offset, long size, int mask, MemoryScope scope) {
return new MappedMemorySegmentImpl(min + offset, unmapper, size, mask, scope);
}

// mapped segment methods
Expand Down Expand Up @@ -105,7 +105,7 @@ public static MappedMemorySegment makeMappedSegment(Path path, long bytesSize, F
UnmapperProxy unmapperProxy = channelImpl.mapInternal(mapMode, 0L, bytesSize);
MemoryScope scope = MemoryScope.create(null, unmapperProxy::unmap);
return new MappedMemorySegmentImpl(unmapperProxy.address(), unmapperProxy, bytesSize,
defaultAccessModes(bytesSize), Thread.currentThread(), scope);
defaultAccessModes(bytesSize), scope);
}
}

Expand Down

0 comments on commit 928a8af

Please sign in to comment.