Skip to content

Commit 5d3edb2

Browse files
fangererfangerer
committed
Inflate primitives in varargs tuple and free their wrappers.
1 parent 1da8a60 commit 5d3edb2

File tree

2 files changed

+184
-12
lines changed

2 files changed

+184
-12
lines changed

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/ExternalFunctionNodes.java‎

Lines changed: 177 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,13 @@
4242

4343
import com.oracle.graal.python.PythonLanguage;
4444
import com.oracle.graal.python.builtins.PythonBuiltinClassType;
45+
import com.oracle.graal.python.builtins.modules.ExternalFunctionNodesFactory.CreateArgsTupleNodeGen;
46+
import com.oracle.graal.python.builtins.modules.ExternalFunctionNodesFactory.MaterializePrimitiveNodeGen;
4547
import com.oracle.graal.python.builtins.modules.ExternalFunctionNodesFactory.ReleaseNativeWrapperNodeGen;
48+
import com.oracle.graal.python.builtins.modules.ExternalFunctionNodesFactory.TraverseNativeWrapperNodeGen;
4649
import com.oracle.graal.python.builtins.modules.PythonCextBuiltins.CheckFunctionResultNode;
4750
import com.oracle.graal.python.builtins.objects.PNone;
51+
import com.oracle.graal.python.builtins.objects.PythonAbstractObject;
4852
import com.oracle.graal.python.builtins.objects.cext.CApiGuards;
4953
import com.oracle.graal.python.builtins.objects.cext.CExtNodes;
5054
import com.oracle.graal.python.builtins.objects.cext.CExtNodes.ConvertArgsToSulongNode;
@@ -53,11 +57,21 @@
5357
import com.oracle.graal.python.builtins.objects.cext.CExtNodesFactory.ToJavaStealingNodeGen;
5458
import com.oracle.graal.python.builtins.objects.cext.common.CExtCommonNodes.ConvertPIntToPrimitiveNode;
5559
import com.oracle.graal.python.builtins.objects.cext.common.CExtCommonNodesFactory.ConvertPIntToPrimitiveNodeGen;
60+
import com.oracle.graal.python.builtins.objects.cext.DynamicObjectNativeWrapper;
61+
import com.oracle.graal.python.builtins.objects.cext.PythonNativeWrapper;
62+
import com.oracle.graal.python.builtins.objects.cext.PythonNativeWrapperLibrary;
63+
import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes.GetElementType;
64+
import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes.ToArrayNode;
65+
import com.oracle.graal.python.builtins.objects.floats.PFloat;
5666
import com.oracle.graal.python.builtins.objects.function.PArguments;
5767
import com.oracle.graal.python.builtins.objects.function.PKeyword;
5868
import com.oracle.graal.python.builtins.objects.function.Signature;
69+
import com.oracle.graal.python.builtins.objects.ints.PInt;
70+
import com.oracle.graal.python.builtins.objects.str.PString;
71+
import com.oracle.graal.python.builtins.objects.tuple.PTuple;
5972
import com.oracle.graal.python.nodes.ErrorMessages;
6073
import com.oracle.graal.python.nodes.IndirectCallNode;
74+
import com.oracle.graal.python.nodes.PGuards;
6175
import com.oracle.graal.python.nodes.PNodeWithContext;
6276
import com.oracle.graal.python.nodes.PRaiseNode;
6377
import com.oracle.graal.python.nodes.PRootNode;
@@ -67,10 +81,13 @@
6781
import com.oracle.graal.python.nodes.argument.ReadVarKeywordsNode;
6882
import com.oracle.graal.python.nodes.call.special.CallVarargsMethodNode;
6983
import com.oracle.graal.python.nodes.interop.PForeignToPTypeNode;
84+
import com.oracle.graal.python.nodes.truffle.PythonTypes;
7085
import com.oracle.graal.python.runtime.ExecutionContext.CalleeContext;
7186
import com.oracle.graal.python.runtime.ExecutionContext.ForeignCallContext;
7287
import com.oracle.graal.python.runtime.PythonContext;
7388
import com.oracle.graal.python.runtime.object.PythonObjectFactory;
89+
import com.oracle.graal.python.runtime.sequence.storage.SequenceStorage;
90+
import com.oracle.graal.python.runtime.sequence.storage.SequenceStorage.ListStorageType;
7491
import com.oracle.graal.python.util.PythonUtils;
7592
import com.oracle.truffle.api.Assumption;
7693
import com.oracle.truffle.api.CompilerDirectives;
@@ -79,13 +96,18 @@
7996
import com.oracle.truffle.api.Truffle;
8097
import com.oracle.truffle.api.TruffleLanguage.ContextReference;
8198
import com.oracle.truffle.api.dsl.Cached;
99+
import com.oracle.truffle.api.dsl.Fallback;
100+
import com.oracle.truffle.api.dsl.ImportStatic;
82101
import com.oracle.truffle.api.dsl.Specialization;
102+
import com.oracle.truffle.api.dsl.TypeSystemReference;
83103
import com.oracle.truffle.api.frame.VirtualFrame;
84104
import com.oracle.truffle.api.interop.ArityException;
85105
import com.oracle.truffle.api.interop.InteropLibrary;
86106
import com.oracle.truffle.api.interop.UnsupportedMessageException;
87107
import com.oracle.truffle.api.interop.UnsupportedTypeException;
108+
import com.oracle.truffle.api.library.CachedLibrary;
88109
import com.oracle.truffle.api.nodes.ExplodeLoop;
110+
import com.oracle.truffle.api.nodes.ExplodeLoop.LoopExplosionKind;
89111
import com.oracle.truffle.api.nodes.Node;
90112
import com.oracle.truffle.api.nodes.NodeCost;
91113
import com.oracle.truffle.api.nodes.UnexpectedResultException;
@@ -304,27 +326,33 @@ abstract static class ReleaseNativeWrapperNode extends Node {
304326
static void doCachedLength(Object[] nativeArguments,
305327
@Cached("nativeArguments.length") int cachedLength,
306328
@Cached(value = "createClassProfiles(cachedLength)", dimensions = 1) ValueProfile[] classProfiles,
329+
@CachedLibrary(limit = "nativeArguments.length") PythonNativeWrapperLibrary lib,
330+
@Cached("createTraverseNodes(cachedLength)") TraverseNativeWrapperNode[] traverseNativeWrapperNodes,
307331
@Cached SubRefCntNode subRefCntNode) {
308332

309333
for (int i = 0; i < cachedLength; i++) {
310-
doCheck(classProfiles[i].profile(nativeArguments[i]), subRefCntNode);
334+
doCheck(classProfiles[i].profile(nativeArguments[i]), subRefCntNode, lib, traverseNativeWrapperNodes[i]);
311335
}
312336
}
313337

314338
@Specialization(replaces = "doCachedLength")
315339
static void doGeneric(Object[] nativeArguments,
316340
@Cached("createClassProfile()") ValueProfile classProfile,
341+
@CachedLibrary(limit = "3") PythonNativeWrapperLibrary lib,
342+
@Cached TraverseNativeWrapperNode traverseNativeWrapperNode,
317343
@Cached SubRefCntNode freeNode) {
318344

319345
for (int i = 0; i < nativeArguments.length; i++) {
320-
doCheck(classProfile.profile(nativeArguments[i]), freeNode);
346+
doCheck(classProfile.profile(nativeArguments[i]), freeNode, lib, traverseNativeWrapperNode);
321347
}
322348
}
323349

324-
private static void doCheck(Object argument, SubRefCntNode refCntNode) {
350+
private static void doCheck(Object argument, SubRefCntNode refCntNode, PythonNativeWrapperLibrary lib, TraverseNativeWrapperNode traverseNativeWrapperNode) {
325351
if (CApiGuards.isNativeWrapper(argument)) {
326352
// in the cached case, refCntNode acts as a branch profile
327-
refCntNode.dec(argument);
353+
if (refCntNode.dec(argument) == 0) {
354+
traverseNativeWrapperNode.execute(lib.getDelegate((PythonNativeWrapper) argument));
355+
}
328356
}
329357
}
330358

@@ -335,6 +363,54 @@ static ValueProfile[] createClassProfiles(int length) {
335363
}
336364
return classProfiles;
337365
}
366+
367+
static TraverseNativeWrapperNode[] createTraverseNodes(int length) {
368+
TraverseNativeWrapperNode[] nodes = new TraverseNativeWrapperNode[length];
369+
for (int i = 0; i < nodes.length; i++) {
370+
nodes[i] = TraverseNativeWrapperNodeGen.create();
371+
}
372+
return nodes;
373+
}
374+
}
375+
376+
/**
377+
* Traverses the items of a tuple and applies {@link ReleaseNativeWrapperNode} on the items if
378+
* the tuple is up to be released.
379+
*/
380+
abstract static class TraverseNativeWrapperNode extends Node {
381+
382+
public abstract void execute(Object containerObject);
383+
384+
@Specialization
385+
static void doTuple(PTuple tuple,
386+
@Cached GetElementType getElementTypeNode,
387+
@Cached ToArrayNode toArrayNode,
388+
@Cached ReleaseNativeWrapperNode releaseNativeWrapperNode) {
389+
390+
SequenceStorage sequenceStorage = tuple.getSequenceStorage();
391+
ListStorageType storageType = getElementTypeNode.execute(sequenceStorage);
392+
if (storageType == ListStorageType.Generic || storageType == ListStorageType.List || storageType == ListStorageType.Tuple) {
393+
Object[] values = toArrayNode.execute(sequenceStorage);
394+
Object[] wrappers = new Object[values.length];
395+
for (int i = 0; i < values.length; i++) {
396+
Object value = values[i];
397+
if (value instanceof PythonAbstractObject) {
398+
DynamicObjectNativeWrapper nativeWrapper = ((PythonAbstractObject) value).getNativeWrapper();
399+
// only traverse if refCnt != 0; this will break the cycle
400+
if (nativeWrapper != null) {
401+
wrappers[i] = nativeWrapper.getRefCount() != 0 ? nativeWrapper : null;
402+
}
403+
}
404+
}
405+
releaseNativeWrapperNode.execute(wrappers);
406+
}
407+
}
408+
409+
@Fallback
410+
static void doOther(@SuppressWarnings("unused") Object other) {
411+
// do nothing
412+
}
413+
338414
}
339415

340416
abstract static class MethodDescriptorRoot extends PRootNode {
@@ -443,6 +519,7 @@ public static final class MethKeywordsRoot extends MethodDescriptorRoot {
443519
@Child private PythonObjectFactory factory;
444520
@Child private ReadVarArgsNode readVarargsNode;
445521
@Child private ReadVarKeywordsNode readKwargsNode;
522+
@Child private CreateArgsTupleNode createArgsTupleNode;
446523

447524
public MethKeywordsRoot(PythonLanguage language, String name, Object callable) {
448525
super(language, name, callable);
@@ -453,14 +530,15 @@ public MethKeywordsRoot(PythonLanguage language, String name, Object callable, C
453530
this.factory = PythonObjectFactory.create();
454531
this.readVarargsNode = ReadVarArgsNode.create(1, true);
455532
this.readKwargsNode = ReadVarKeywordsNode.create(new String[0]);
533+
this.createArgsTupleNode = CreateArgsTupleNodeGen.create();
456534
}
457535

458536
@Override
459537
protected Object[] prepareCArguments(VirtualFrame frame) {
460538
Object self = readSelfNode.execute(frame);
461539
Object[] args = readVarargsNode.executeObjectArray(frame);
462540
PKeyword[] kwargs = readKwargsNode.executePKeyword(frame);
463-
return new Object[]{self, factory.createTuple(args), factory.createDict(kwargs)};
541+
return new Object[]{self, createArgsTupleNode.execute(factory, args), factory.createDict(kwargs)};
464542
}
465543

466544
@Override
@@ -473,6 +551,7 @@ public static final class MethVarargsRoot extends MethodDescriptorRoot {
473551
private static final Signature SIGNATURE = new Signature(-1, false, 1, false, new String[]{"self"}, new String[0]);
474552
@Child private PythonObjectFactory factory;
475553
@Child private ReadVarArgsNode readVarargsNode;
554+
@Child private CreateArgsTupleNode createArgsTupleNode;
476555

477556
public MethVarargsRoot(PythonLanguage language, String name, Object callable) {
478557
super(language, name, callable);
@@ -482,13 +561,14 @@ public MethVarargsRoot(PythonLanguage language, String name, Object callable, Co
482561
super(language, name, callable, convertArgsToSulongNode);
483562
this.factory = PythonObjectFactory.create();
484563
this.readVarargsNode = ReadVarArgsNode.create(1, true);
564+
this.createArgsTupleNode = CreateArgsTupleNodeGen.create();
485565
}
486566

487567
@Override
488568
protected Object[] prepareCArguments(VirtualFrame frame) {
489569
Object self = readSelfNode.execute(frame);
490570
Object[] args = readVarargsNode.executeObjectArray(frame);
491-
return new Object[]{self, factory.createTuple(args)};
571+
return new Object[]{self, createArgsTupleNode.execute(factory, args)};
492572
}
493573

494574
@Override
@@ -934,4 +1014,95 @@ public Signature getSignature() {
9341014
return SIGNATURE;
9351015
}
9361016
}
1017+
1018+
/**
1019+
* We need to inflate all primitives in order to avoid memory leaks. Explanation: Primitives
1020+
* would currently be wrapped into a PrimitiveNativeWrapper. If any of those will receive a
1021+
* toNative message, the managed code will be the only owner of those wrappers. But we will
1022+
* never be able to reach the wrapper from the arguments if they are just primitive. So, we
1023+
* inflate the primitives and we can then traverse the tuple and reach the wrappers of its
1024+
* arguments after the call returned.
1025+
*/
1026+
abstract static class CreateArgsTupleNode extends Node {
1027+
public abstract PTuple execute(PythonObjectFactory factory, Object[] args);
1028+
1029+
@Specialization(guards = {"args.length == cachedLen", "cachedLen <= 16"})
1030+
@ExplodeLoop(kind = LoopExplosionKind.FULL_UNROLL)
1031+
static PTuple doCachedLen(PythonObjectFactory factory, Object[] args,
1032+
@Cached("args.length") int cachedLen,
1033+
@Cached("createMaterializeNodes(args.length)") MaterializePrimitiveNode[] materializePrimitiveNodes) {
1034+
1035+
for (int i = 0; i < cachedLen; i++) {
1036+
args[i] = materializePrimitiveNodes[i].execute(factory, args[i]);
1037+
}
1038+
return factory.createTuple(args);
1039+
}
1040+
1041+
@Specialization(replaces = "doCachedLen")
1042+
static PTuple doGeneric(PythonObjectFactory factory, Object[] args,
1043+
@Cached MaterializePrimitiveNode materializePrimitiveNode) {
1044+
1045+
for (int i = 0; i < args.length; i++) {
1046+
args[i] = materializePrimitiveNode.execute(factory, args[i]);
1047+
}
1048+
return factory.createTuple(args);
1049+
}
1050+
1051+
static MaterializePrimitiveNode[] createMaterializeNodes(int length) {
1052+
MaterializePrimitiveNode[] materializePrimitiveNodes = new MaterializePrimitiveNode[length];
1053+
for (int i = 0; i < length; i++) {
1054+
materializePrimitiveNodes[i] = MaterializePrimitiveNodeGen.create();
1055+
}
1056+
return materializePrimitiveNodes;
1057+
}
1058+
}
1059+
1060+
/**
1061+
* Special helper nodes that materializes any primitive that would leak the wrapper if the
1062+
* reference is owned by managed code only.
1063+
*/
1064+
@ImportStatic(CApiGuards.class)
1065+
@TypeSystemReference(PythonTypes.class)
1066+
abstract static class MaterializePrimitiveNode extends Node {
1067+
1068+
public abstract Object execute(PythonObjectFactory factory, Object object);
1069+
1070+
// NOTE: Booleans don't need to be materialized because they are singletons.
1071+
1072+
@Specialization(guards = "!isSmallInteger(i)")
1073+
static PInt doInteger(PythonObjectFactory factory, int i) {
1074+
return factory.createInt(i);
1075+
}
1076+
1077+
@Specialization(guards = "!isSmallLong(l)", replaces = "doInteger")
1078+
static PInt doLong(PythonObjectFactory factory, long l) {
1079+
return factory.createInt(l);
1080+
}
1081+
1082+
@Specialization
1083+
static PFloat doDouble(PythonObjectFactory factory, double d) {
1084+
return factory.createFloat(d);
1085+
}
1086+
1087+
@Specialization
1088+
static PString doString(PythonObjectFactory factory, String s) {
1089+
return factory.createString(s);
1090+
}
1091+
1092+
@Specialization(guards = "!needsMaterialization(object)")
1093+
static Object doObject(@SuppressWarnings("unused") PythonObjectFactory factory, Object object) {
1094+
return object;
1095+
}
1096+
1097+
static boolean needsMaterialization(Object object) {
1098+
if (object instanceof Integer) {
1099+
return !CApiGuards.isSmallInteger((Integer) object);
1100+
}
1101+
if (object instanceof Long) {
1102+
return !CApiGuards.isSmallLong((Long) object);
1103+
}
1104+
return PGuards.isDouble(object) || object instanceof String;
1105+
}
1106+
}
1107+
9371108
}

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

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2990,14 +2990,14 @@ static Object doNativeObject(Object object, long value,
29902990
public abstract static class SubRefCntNode extends PNodeWithContext {
29912991
private static final TruffleLogger LOGGER = PythonLanguage.getLogger(SubRefCntNode.class);
29922992

2993-
public final Object dec(Object object) {
2993+
public final long dec(Object object) {
29942994
return execute(object, 1);
29952995
}
29962996

2997-
public abstract Object execute(Object object, long value);
2997+
public abstract long execute(Object object, long value);
29982998

29992999
@Specialization
3000-
static Object doNativeWrapper(PythonNativeWrapper nativeWrapper, long value,
3000+
static long doNativeWrapper(PythonNativeWrapper nativeWrapper, long value,
30013001
@Cached FreeNode freeNode,
30023002
@Cached BranchProfile negativeProfile) {
30033003
long refCount = nativeWrapper.getRefCount() - value;
@@ -3009,11 +3009,11 @@ static Object doNativeWrapper(PythonNativeWrapper nativeWrapper, long value,
30093009
negativeProfile.enter();
30103010
LOGGER.severe(() -> "native wrapper has negative ref count: " + nativeWrapper);
30113011
}
3012-
return nativeWrapper;
3012+
return refCount;
30133013
}
30143014

30153015
@Specialization(guards = "!isNativeWrapper(object)", limit = "2")
3016-
static Object doNativeObject(Object object, long value,
3016+
static long doNativeObject(Object object, long value,
30173017
@CachedContext(PythonLanguage.class) PythonContext context,
30183018
@Cached PCallCapiFunction callAddRefCntNode,
30193019
@CachedLibrary("object") InteropLibrary lib) {
@@ -3024,8 +3024,9 @@ static Object doNativeObject(Object object, long value,
30243024
if (context.getOption(PythonOptions.TraceNativeMemory) && newRefcnt < 0) {
30253025
LOGGER.severe(() -> "object has negative ref count: " + CApiContext.asHex(object));
30263026
}
3027+
return newRefcnt;
30273028
}
3028-
return object;
3029+
return 1;
30293030
}
30303031
}
30313032

0 commit comments

Comments
 (0)