Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Move "owner" field and thread-confinement checks to MemoryScope #167

Closed
wants to merge 5 commits into from
Closed

Move "owner" field and thread-confinement checks to MemoryScope #167

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0107a42
Move "owner" field and thread-confinement checks from MemorySegment t…
plevart May 15, 2020
3845bcf
Improvements to MemoryScope code considering Maurizio's comments.
plevart May 15, 2020
ad3b5e5
Make private API really private in MemoryScope
plevart May 15, 2020
9fdd5a0
Fix confined root MemoryScope.close() - confinement check is required
plevart May 15, 2020
66a821d
Perform liveness check for unconfined MemoryScope too even if it may …
plevart May 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
44 changes: 18 additions & 26 deletions src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/AbstractMemorySegmentImpl.java
View file
Open in desktop
Expand Up @@ -68,22 +68,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 @@ -100,7 +98,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 @@ -145,12 +143,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 @@ -159,7 +157,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 @@ -177,15 +175,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 @@ -198,7 +195,6 @@ public final void close() {
if (!isSet(CLOSE)) {
throw unsupportedAccessMode(CLOSE);
}
checkValidState();
closeNoCheck();
}

Expand All @@ -210,7 +206,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 @@ -227,7 +223,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 @@ -238,10 +234,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 @@ -415,29 +408,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 @@ -454,7 +446,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
Open in desktop
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
Open in desktop
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