Skip to content

Commit 4b4b7f9

Browse files
committed
Move Release/TraverseNativeWrapperNode to CExtNodes
1 parent 29f5b62 commit 4b4b7f9

File tree

2 files changed

+71
-87
lines changed

2 files changed

+71
-87
lines changed

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/capi/CExtNodes.java

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@
118118
import com.oracle.graal.python.builtins.objects.cext.common.GetVaArgsNode;
119119
import com.oracle.graal.python.builtins.objects.cext.common.GetVaArgsNodeGen;
120120
import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes;
121+
import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes.ToArrayNode;
121122
import com.oracle.graal.python.builtins.objects.complex.PComplex;
122123
import com.oracle.graal.python.builtins.objects.floats.PFloat;
123124
import com.oracle.graal.python.builtins.objects.function.PBuiltinFunction;
@@ -128,8 +129,10 @@
128129
import com.oracle.graal.python.builtins.objects.method.PBuiltinMethod;
129130
import com.oracle.graal.python.builtins.objects.module.ModuleGetNameNode;
130131
import com.oracle.graal.python.builtins.objects.module.PythonModule;
132+
import com.oracle.graal.python.builtins.objects.object.PythonObject;
131133
import com.oracle.graal.python.builtins.objects.str.NativeCharSequence;
132134
import com.oracle.graal.python.builtins.objects.str.PString;
135+
import com.oracle.graal.python.builtins.objects.tuple.PTuple;
133136
import com.oracle.graal.python.builtins.objects.type.PythonAbstractClass;
134137
import com.oracle.graal.python.builtins.objects.type.PythonManagedClass;
135138
import com.oracle.graal.python.builtins.objects.type.TypeNodes;
@@ -1963,7 +1966,7 @@ Object upcall(VirtualFrame frame, Object[] args,
19631966
@Cached CallNode callNode,
19641967
@Shared("allToJavaNode") @Cached AllToPythonNode allToPythonNode) {
19651968
Object[] converted = allToPythonNode.execute(args, 1);
1966-
return callNode.execute(frame, args[0], converted, new PKeyword[0]);
1969+
return callNode.execute(frame, args[0], converted, PKeyword.EMPTY_KEYWORDS);
19671970
}
19681971

19691972
public static DirectUpcallNode create() {
@@ -4028,4 +4031,69 @@ static PMemoryView fromNative(PythonNativeObject buf, int flags,
40284031
return (PMemoryView) asPythonObjectNode.execute(result);
40294032
}
40304033
}
4034+
4035+
/**
4036+
* Decrements the ref count by one of any {@link PythonNativeWrapper} object.
4037+
* <p>
4038+
* This node avoids memory leaks for arguments given to native.<br>
4039+
* Problem description:<br>
4040+
* {@link PythonNativeWrapper} objects given to C code may go to native, i.e., a handle will be
4041+
* allocated. In this case, no ref count manipulation is done since the C code considers the
4042+
* reference to be borrowed and the Python code just doesn't do it because we have a GC. This
4043+
* means that the handle will stay allocated and we are leaking the wrapper object.
4044+
* </p>
4045+
*/
4046+
@ImportStatic(CApiGuards.class)
4047+
abstract static class ReleaseNativeWrapperNode extends Node {
4048+
4049+
public abstract void execute(Object pythonObject);
4050+
4051+
@Specialization
4052+
static void doNativeWrapper(PythonNativeWrapper nativeWrapper,
4053+
@Cached TraverseNativeWrapperNode traverseNativeWrapperNode,
4054+
@Cached SubRefCntNode subRefCntNode) {
4055+
// in the cached case, refCntNode acts as a branch profile
4056+
if (subRefCntNode.dec(nativeWrapper) == 0) {
4057+
traverseNativeWrapperNode.execute(nativeWrapper.getDelegateSlowPath());
4058+
}
4059+
}
4060+
4061+
@Specialization(guards = "!isNativeWrapper(object)")
4062+
@SuppressWarnings("unused")
4063+
static void doOther(Object object) {
4064+
// just do nothing; this is an implicit profile
4065+
}
4066+
}
4067+
4068+
/**
4069+
* Traverses the items of a tuple and applies {@link ReleaseNativeWrapperNode} on the items if
4070+
* the tuple is up to be released.
4071+
*/
4072+
abstract static class TraverseNativeWrapperNode extends Node {
4073+
4074+
public abstract void execute(Object containerObject);
4075+
4076+
@Specialization
4077+
static void doTuple(PTuple tuple,
4078+
@Cached ToArrayNode toArrayNode,
4079+
@Cached SubRefCntNode subRefCntNode) {
4080+
4081+
Object[] values = toArrayNode.execute(tuple.getSequenceStorage());
4082+
for (int i = 0; i < values.length; i++) {
4083+
Object value = values[i];
4084+
if (value instanceof PythonObject) {
4085+
DynamicObjectNativeWrapper nativeWrapper = ((PythonObject) value).getNativeWrapper();
4086+
// only traverse if refCnt != 0; this will break the cycle
4087+
if (nativeWrapper != null) {
4088+
subRefCntNode.dec(nativeWrapper);
4089+
}
4090+
}
4091+
}
4092+
}
4093+
4094+
@Fallback
4095+
static void doOther(@SuppressWarnings("unused") Object other) {
4096+
// do nothing
4097+
}
4098+
}
40314099
}

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/capi/ExternalFunctionNodes.java

Lines changed: 2 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -56,13 +56,14 @@
5656
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodes.FastCallArgsToSulongNode;
5757
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodes.FastCallWithKeywordsArgsToSulongNode;
5858
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodes.PCallCapiFunction;
59+
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodes.ReleaseNativeWrapperNode;
5960
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodes.SSizeArgProcToSulongNode;
6061
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodes.SSizeObjArgProcToSulongNode;
61-
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodes.SubRefCntNode;
6262
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodes.TernaryFirstSecondToSulongNode;
6363
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodes.TernaryFirstThirdToSulongNode;
6464
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodes.ToBorrowedRefNode;
6565
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodes.ToJavaStealingNode;
66+
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodesFactory.ReleaseNativeWrapperNodeGen;
6667
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodesFactory.ToBorrowedRefNodeGen;
6768
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodesFactory.ToJavaStealingNodeGen;
6869
import com.oracle.graal.python.builtins.objects.cext.capi.ExternalFunctionNodesFactory.CheckInquiryResultNodeGen;
@@ -72,21 +73,18 @@
7273
import com.oracle.graal.python.builtins.objects.cext.capi.ExternalFunctionNodesFactory.DefaultCheckFunctionResultNodeGen;
7374
import com.oracle.graal.python.builtins.objects.cext.capi.ExternalFunctionNodesFactory.InitCheckFunctionResultNodeGen;
7475
import com.oracle.graal.python.builtins.objects.cext.capi.ExternalFunctionNodesFactory.MaterializePrimitiveNodeGen;
75-
import com.oracle.graal.python.builtins.objects.cext.capi.ExternalFunctionNodesFactory.ReleaseNativeWrapperNodeGen;
7676
import com.oracle.graal.python.builtins.objects.cext.common.CExtCommonNodes.CheckFunctionResultNode;
7777
import com.oracle.graal.python.builtins.objects.cext.common.CExtCommonNodes.ConvertPIntToPrimitiveNode;
7878
import com.oracle.graal.python.builtins.objects.cext.common.CExtCommonNodes.GetIndexNode;
7979
import com.oracle.graal.python.builtins.objects.cext.common.CExtCommonNodesFactory.ConvertPIntToPrimitiveNodeGen;
8080
import com.oracle.graal.python.builtins.objects.cext.common.CExtContext;
81-
import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes.ToArrayNode;
8281
import com.oracle.graal.python.builtins.objects.exception.PBaseException;
8382
import com.oracle.graal.python.builtins.objects.floats.PFloat;
8483
import com.oracle.graal.python.builtins.objects.function.PArguments;
8584
import com.oracle.graal.python.builtins.objects.function.PBuiltinFunction;
8685
import com.oracle.graal.python.builtins.objects.function.PKeyword;
8786
import com.oracle.graal.python.builtins.objects.function.Signature;
8887
import com.oracle.graal.python.builtins.objects.ints.PInt;
89-
import com.oracle.graal.python.builtins.objects.object.PythonObject;
9088
import com.oracle.graal.python.builtins.objects.str.PString;
9189
import com.oracle.graal.python.builtins.objects.tuple.PTuple;
9290
import com.oracle.graal.python.nodes.ErrorMessages;
@@ -126,7 +124,6 @@
126124
import com.oracle.truffle.api.dsl.Cached;
127125
import com.oracle.truffle.api.dsl.Cached.Exclusive;
128126
import com.oracle.truffle.api.dsl.Cached.Shared;
129-
import com.oracle.truffle.api.dsl.Fallback;
130127
import com.oracle.truffle.api.dsl.GenerateUncached;
131128
import com.oracle.truffle.api.dsl.ImportStatic;
132129
import com.oracle.truffle.api.dsl.Specialization;
@@ -616,71 +613,6 @@ public static ExternalFunctionInvokeNode create(CheckFunctionResultNode checkFun
616613
}
617614
}
618615

619-
/**
620-
* Decrements the ref count by one of any {@link PythonNativeWrapper} object.
621-
* <p>
622-
* This node avoids memory leaks for arguments given to native.<br>
623-
* Problem description:<br>
624-
* {@link PythonNativeWrapper} objects given to C code may go to native, i.e., a handle will be
625-
* allocated. In this case, no ref count manipulation is done since the C code considers the
626-
* reference to be borrowed and the Python code just doesn't do it because we have a GC. This
627-
* means that the handle will stay allocated and we are leaking the wrapper object.
628-
* </p>
629-
*/
630-
@ImportStatic(CApiGuards.class)
631-
abstract static class ReleaseNativeWrapperNode extends Node {
632-
633-
public abstract void execute(Object pythonObject);
634-
635-
@Specialization
636-
static void doNativeWrapper(PythonNativeWrapper nativeWrapper,
637-
@Cached TraverseNativeWrapperNode traverseNativeWrapperNode,
638-
@Cached SubRefCntNode subRefCntNode) {
639-
// in the cached case, refCntNode acts as a branch profile
640-
if (subRefCntNode.dec(nativeWrapper) == 0) {
641-
traverseNativeWrapperNode.execute(nativeWrapper.getDelegateSlowPath());
642-
}
643-
}
644-
645-
@Specialization(guards = "!isNativeWrapper(object)")
646-
@SuppressWarnings("unused")
647-
static void doOther(Object object) {
648-
// just do nothing; this is an implicit profile
649-
}
650-
}
651-
652-
/**
653-
* Traverses the items of a tuple and applies {@link ReleaseNativeWrapperNode} on the items if
654-
* the tuple is up to be released.
655-
*/
656-
abstract static class TraverseNativeWrapperNode extends Node {
657-
658-
public abstract void execute(Object containerObject);
659-
660-
@Specialization
661-
static void doTuple(PTuple tuple,
662-
@Cached ToArrayNode toArrayNode,
663-
@Cached SubRefCntNode subRefCntNode) {
664-
665-
Object[] values = toArrayNode.execute(tuple.getSequenceStorage());
666-
for (int i = 0; i < values.length; i++) {
667-
Object value = values[i];
668-
if (value instanceof PythonObject) {
669-
DynamicObjectNativeWrapper nativeWrapper = ((PythonObject) value).getNativeWrapper();
670-
// only traverse if refCnt != 0; this will break the cycle
671-
if (nativeWrapper != null) {
672-
subRefCntNode.dec(nativeWrapper);
673-
}
674-
}
675-
}
676-
}
677-
678-
@Fallback
679-
static void doOther(@SuppressWarnings("unused") Object other) {
680-
// do nothing
681-
}
682-
}
683-
684616
abstract static class MethodDescriptorRoot extends PRootNode {
685617
@Child private CalleeContext calleeContext = CalleeContext.create();
686618
@Child private CallVarargsMethodNode invokeNode;
@@ -765,18 +697,6 @@ protected Object[] preparePArguments(VirtualFrame frame) {
765697
return arguments;
766698
}
767699

768-
static Object[] copyPArguments(VirtualFrame frame) {
769-
return copyPArguments(frame, PArguments.getUserArgumentLength(frame));
770-
}
771-
772-
static Object[] copyPArguments(VirtualFrame frame, int newUserArgumentLength) {
773-
Object[] objects = PArguments.create(newUserArgumentLength);
774-
PArguments.setGlobals(objects, PArguments.getGlobals(frame));
775-
PArguments.setClosure(objects, PArguments.getClosure(frame));
776-
PArguments.setSpecialArgument(objects, PArguments.getSpecialArgument(frame));
777-
return objects;
778-
}
779-
780700
private ReadIndexedArgumentNode ensureReadCallableNode() {
781701
if (readCallableNode == null) {
782702
CompilerDirectives.transferToInterpreterAndInvalidate();
@@ -1594,10 +1514,6 @@ abstract static class GetSetRootNode extends MethodDescriptorRoot {
15941514

15951515
@Child private ReadIndexedArgumentNode readClosureNode;
15961516

1597-
GetSetRootNode(PythonLanguage language, String name) {
1598-
super(language, name, false);
1599-
}
1600-
16011517
GetSetRootNode(PythonLanguage language, String name, PExternalFunctionWrapper provider) {
16021518
super(language, name, false, provider);
16031519
}

0 commit comments

Comments
 (0)