Skip to content

Commit 6a69671

Browse files
aardvark179aardvark179
committed
Changes based on review.
1 parent 9189fe1 commit 6a69671

File tree

4 files changed

+50
-39
lines changed

4 files changed

+50
-39
lines changed

‎src/main/java/org/truffleruby/cext/CExtNodes.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1284,7 +1284,7 @@ public Object unwrap(Object value,
12841284
Object object = unwrapNode.execute(value);
12851285
if (object == null) {
12861286
exceptionProfile.enter();
1287-
throw new RaiseException(getContext(), coreExceptions().runtimeError("Native handle not found", this));
1287+
throw new RaiseException(getContext(), coreExceptions().runtimeError("native handle not found", this));
12881288
} else {
12891289
return object;
12901290
}

‎src/main/java/org/truffleruby/cext/ValueWrapperManager.java

Lines changed: 41 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import org.truffleruby.Layouts;
1515
import org.truffleruby.RubyContext;
1616
import org.truffleruby.collections.LongHashMap;
17+
import org.truffleruby.extra.ffi.Pointer;
1718
import org.truffleruby.language.NotProvided;
1819

1920
import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
@@ -22,29 +23,39 @@
2223

2324
public class ValueWrapperManager {
2425

26+
static final int UNSET_HANDLE = -1;
27+
2528
@CompilationFinal private DynamicObject undefWrapper = null;
2629
@CompilationFinal private DynamicObject trueWrapper = null;
2730
@CompilationFinal private DynamicObject falseWrapper = null;
2831

2932
private final LongHashMap<DynamicObject> longMap = new LongHashMap<>(128);
3033
private final LongHashMap<WeakReference<DynamicObject>> handleMap = new LongHashMap<>(1024);
3134

32-
RubyContext context;
35+
private final RubyContext context;
3336

3437
public ValueWrapperManager(RubyContext context) {
3538
this.context = context;
3639
}
3740

3841
public DynamicObject undefWrapper() {
39-
return undefWrapper != null ? undefWrapper : (undefWrapper = Layouts.VALUE_WRAPPER.createValueWrapper(NotProvided.INSTANCE, ValueWrapperObjectType.UNSET_HANDLE));
40-
42+
if (undefWrapper == null) {
43+
undefWrapper = Layouts.VALUE_WRAPPER.createValueWrapper(NotProvided.INSTANCE, UNSET_HANDLE);
44+
}
45+
return undefWrapper;
4146
}
4247

4348
public DynamicObject booleanWrapper(boolean value) {
4449
if (value) {
45-
return trueWrapper != null ? trueWrapper : (trueWrapper = Layouts.VALUE_WRAPPER.createValueWrapper(true, ValueWrapperObjectType.UNSET_HANDLE));
50+
if (trueWrapper == null) {
51+
trueWrapper = Layouts.VALUE_WRAPPER.createValueWrapper(true, UNSET_HANDLE);
52+
}
53+
return trueWrapper;
4654
} else {
47-
return falseWrapper != null ? falseWrapper : (falseWrapper = createFalseWrapper());
55+
if (falseWrapper == null) {
56+
falseWrapper = createFalseWrapper();
57+
}
58+
return falseWrapper;
4859
}
4960
}
5061

@@ -61,14 +72,14 @@ private DynamicObject createFalseWrapper() {
6172
public synchronized DynamicObject longWrapper(long value) {
6273
DynamicObject wrapper = longMap.get(value);
6374
if (wrapper == null) {
64-
wrapper = Layouts.VALUE_WRAPPER.createValueWrapper(value, ValueWrapperObjectType.UNSET_HANDLE);
75+
wrapper = Layouts.VALUE_WRAPPER.createValueWrapper(value, ValueWrapperManager.UNSET_HANDLE);
6576
longMap.put(value, wrapper);
6677
}
6778
return wrapper;
6879
}
6980

7081
public DynamicObject doubleWrapper(double value) {
71-
return Layouts.VALUE_WRAPPER.createValueWrapper(value, ValueWrapperObjectType.UNSET_HANDLE);
82+
return Layouts.VALUE_WRAPPER.createValueWrapper(value, ValueWrapperManager.UNSET_HANDLE);
7283
}
7384

7485
@TruffleBoundary
@@ -94,4 +105,27 @@ public synchronized void removeFromHandleMap(long handle) {
94105
handleMap.remove(handle);
95106
}
96107

108+
@TruffleBoundary
109+
public synchronized long createNativeHandle(DynamicObject wrapper) {
110+
Pointer handlePointer = Pointer.malloc(8);
111+
long handleAddress = handlePointer.getAddress();
112+
Layouts.VALUE_WRAPPER.setHandle(wrapper, handleAddress);
113+
addToHandleMap(handleAddress, wrapper);
114+
addFinalizer(wrapper, handlePointer);
115+
return handleAddress;
116+
}
117+
118+
public void addFinalizer(DynamicObject wrapper, Pointer handle) {
119+
context.getFinalizationService().addFinalizer(
120+
wrapper, null, ValueWrapperObjectType.class,
121+
createFinalizer(handle), null);
122+
}
123+
124+
private Runnable createFinalizer(Pointer handle) {
125+
return () -> {
126+
this.removeFromHandleMap(handle.getAddress());
127+
handle.free();
128+
};
129+
130+
}
97131
}

‎src/main/java/org/truffleruby/cext/ValueWrapperMessageResolution.java

Lines changed: 5 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,15 @@
1212
import org.truffleruby.Layouts;
1313
import org.truffleruby.RubyContext;
1414
import org.truffleruby.RubyLanguage;
15-
import org.truffleruby.extra.ffi.Pointer;
1615

1716
import com.oracle.truffle.api.CompilerDirectives;
1817
import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
19-
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
2018
import com.oracle.truffle.api.frame.VirtualFrame;
2119
import com.oracle.truffle.api.interop.MessageResolution;
2220
import com.oracle.truffle.api.interop.Resolve;
2321
import com.oracle.truffle.api.nodes.Node;
2422
import com.oracle.truffle.api.object.DynamicObject;
23+
import com.oracle.truffle.api.profiles.BranchProfile;
2524

2625
@MessageResolution(receiverType = ValueWrapperObjectType.class)
2726
public class ValueWrapperMessageResolution {
@@ -45,37 +44,17 @@ protected Object access(VirtualFrame frame, DynamicObject receiver) {
4544
public static abstract class ForeignAsPointerNode extends Node {
4645

4746
@CompilationFinal private RubyContext context;
47+
private final BranchProfile createHandleProfile = BranchProfile.create();
4848

4949
protected long access(VirtualFrame frame, DynamicObject wrapper) {
5050
long handle = Layouts.VALUE_WRAPPER.getHandle(wrapper);
51-
if (handle == ValueWrapperObjectType.UNSET_HANDLE) {
52-
handle = createHandle(wrapper);
51+
if (handle == ValueWrapperManager.UNSET_HANDLE) {
52+
createHandleProfile.enter();
53+
handle = getContext().getValueWrapperManager().createNativeHandle(wrapper);
5354
}
5455
return handle;
5556
}
5657

57-
@TruffleBoundary
58-
protected long createHandle(DynamicObject wrapper) {
59-
synchronized (wrapper) {
60-
Pointer handlePointer = Pointer.malloc(8);
61-
long handleAddress = handlePointer.getAddress();
62-
Layouts.VALUE_WRAPPER.setHandle(wrapper, handleAddress);
63-
getContext().getValueWrapperManager().addToHandleMap(handleAddress, wrapper);
64-
// Add a finaliser to remove the map entry.
65-
getContext().getFinalizationService().addFinalizer(
66-
wrapper, null, ValueWrapperObjectType.class,
67-
finalizer(getContext().getValueWrapperManager(), handlePointer), null);
68-
return handleAddress;
69-
}
70-
}
71-
72-
private static Runnable finalizer(ValueWrapperManager manager, Pointer handle) {
73-
return () -> {
74-
manager.removeFromHandleMap(handle.getAddress());
75-
handle.free();
76-
};
77-
78-
}
7958
public RubyContext getContext() {
8059
if (context == null) {
8160
CompilerDirectives.transferToInterpreterAndInvalidate();

‎src/main/java/org/truffleruby/cext/ValueWrapperObjectType.java

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,8 @@
1818

1919
public class ValueWrapperObjectType extends ObjectType {
2020

21-
static final int UNSET_HANDLE = -1;
22-
2321
public static DynamicObject createValueWrapper(Object value) {
24-
return Layouts.VALUE_WRAPPER.createValueWrapper(value, UNSET_HANDLE);
22+
return Layouts.VALUE_WRAPPER.createValueWrapper(value, ValueWrapperManager.UNSET_HANDLE);
2523
}
2624

2725
public static boolean isInstance(TruffleObject receiver) {
@@ -37,8 +35,8 @@ public boolean equals(DynamicObject object, Object other) {
3735

3836
final long objectHandle = Layouts.VALUE_WRAPPER.getHandle(object);
3937
final long otherHandle = Layouts.VALUE_WRAPPER.getHandle(otherWrapper);
40-
if (objectHandle != UNSET_HANDLE &&
41-
otherHandle != UNSET_HANDLE) {
38+
if (objectHandle != ValueWrapperManager.UNSET_HANDLE &&
39+
otherHandle != ValueWrapperManager.UNSET_HANDLE) {
4240
return objectHandle == otherHandle;
4341
}
4442
return Layouts.VALUE_WRAPPER.getObject(object).equals(Layouts.VALUE_WRAPPER.getObject(otherWrapper));

0 commit comments

Comments
 (0)