Skip to content

Commit 60d113b

Browse files
steve-ssteve-s
committed
[GR-40277] Improve the limiting of the AST inlining of builtins.
PullRequest: graalpython/2386
2 parents 9619be2 + a944d28 commit 60d113b

File tree

7 files changed

+127
-22
lines changed

7 files changed

+127
-22
lines changed

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/PRootNode.java

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,16 @@
4242

4343
import java.util.ArrayList;
4444

45+
import com.oracle.graal.python.PythonLanguage;
4546
import com.oracle.graal.python.builtins.Python3Core;
4647
import com.oracle.graal.python.builtins.PythonBuiltinClassType;
4748
import com.oracle.graal.python.builtins.objects.function.Signature;
4849
import com.oracle.graal.python.nodes.function.BuiltinFunctionRootNode;
4950
import com.oracle.graal.python.parser.PythonParserImpl;
5051
import com.oracle.graal.python.runtime.PythonContext;
52+
import com.oracle.graal.python.runtime.PythonOptions;
5153
import com.oracle.graal.python.util.PythonUtils;
54+
import com.oracle.graal.python.util.PythonUtils.NodeCounterWithLimit;
5255
import com.oracle.truffle.api.Assumption;
5356
import com.oracle.truffle.api.CompilerAsserts;
5457
import com.oracle.truffle.api.CompilerDirectives;
@@ -58,7 +61,6 @@
5861
import com.oracle.truffle.api.TruffleLanguage;
5962
import com.oracle.truffle.api.frame.FrameDescriptor;
6063
import com.oracle.truffle.api.nodes.Node;
61-
import com.oracle.truffle.api.nodes.NodeUtil;
6264
import com.oracle.truffle.api.nodes.RootNode;
6365
import com.oracle.truffle.api.profiles.ConditionProfile;
6466

@@ -97,13 +99,26 @@ protected PRootNode(TruffleLanguage<?> language, FrameDescriptor frameDescriptor
9799
super(language, frameDescriptor);
98100
}
99101

100-
public final int getNodeCount() {
102+
/**
103+
* Imprecise node count used for inlining heuristics, saturated at
104+
* {@link PythonOptions#BuiltinsInliningMaxCallerSize}.
105+
*/
106+
public final int getNodeCountForInlining() {
101107
CompilerAsserts.neverPartOfCompilation();
102108
int n = nodeCount;
103109
if (n != -1) {
104110
return n;
105111
}
106-
return nodeCount = NodeUtil.countNodes(this);
112+
int maxSize = PythonLanguage.get(this).getEngineOption(PythonOptions.BuiltinsInliningMaxCallerSize);
113+
NodeCounterWithLimit counter = new NodeCounterWithLimit(maxSize);
114+
accept(counter);
115+
return nodeCount = counter.getCount();
116+
}
117+
118+
public final void setNodeCountForInlining(int newCount) {
119+
// We accept the potential race in the callers of the getter and this setter
120+
assert newCount > 0 && newCount <= PythonLanguage.get(this).getEngineOption(PythonOptions.BuiltinsInliningMaxCallerSize);
121+
nodeCount = newCount;
107122
}
108123

109124
public ConditionProfile getFrameEscapedProfile() {

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/call/special/AbstractCallMethodNode.java

Lines changed: 68 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,9 @@
4646
import com.oracle.graal.python.builtins.Builtin;
4747
import com.oracle.graal.python.builtins.PythonBuiltinClassType;
4848
import com.oracle.graal.python.builtins.objects.function.BuiltinMethodDescriptor;
49+
import com.oracle.graal.python.builtins.objects.function.BuiltinMethodDescriptor.BinaryBuiltinDescriptor;
50+
import com.oracle.graal.python.builtins.objects.function.BuiltinMethodDescriptor.TernaryBuiltinDescriptor;
51+
import com.oracle.graal.python.builtins.objects.function.BuiltinMethodDescriptor.UnaryBuiltinDescriptor;
4952
import com.oracle.graal.python.builtins.objects.function.PBuiltinFunction;
5053
import com.oracle.graal.python.builtins.objects.method.PBuiltinMethod;
5154
import com.oracle.graal.python.builtins.objects.method.PMethod;
@@ -63,6 +66,7 @@
6366
import com.oracle.graal.python.nodes.function.builtins.PythonVarargsBuiltinNode;
6467
import com.oracle.graal.python.nodes.truffle.PythonTypes;
6568
import com.oracle.graal.python.runtime.PythonOptions;
69+
import com.oracle.graal.python.util.PythonUtils.NodeCounterWithLimit;
6670
import com.oracle.truffle.api.CompilerAsserts;
6771
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
6872
import com.oracle.truffle.api.RootCallTarget;
@@ -72,7 +76,7 @@
7276
import com.oracle.truffle.api.dsl.NodeField;
7377
import com.oracle.truffle.api.dsl.TypeSystemReference;
7478
import com.oracle.truffle.api.frame.VirtualFrame;
75-
import com.oracle.truffle.api.nodes.NodeUtil;
79+
import com.oracle.truffle.api.nodes.Node;
7680
import com.oracle.truffle.api.nodes.RootNode;
7781
import com.oracle.truffle.api.nodes.UnexpectedResultException;
7882

@@ -112,17 +116,75 @@ private <T extends PythonBuiltinBaseNode> T getBuiltin(VirtualFrame frame, PBuil
112116
return null;
113117
}
114118

119+
public PythonUnaryBuiltinNode getBuiltin(UnaryBuiltinDescriptor descriptor) {
120+
PythonUnaryBuiltinNode builtin = descriptor.createNode();
121+
if (!callerExceedsMaxSize(builtin)) {
122+
return builtin;
123+
}
124+
return null;
125+
}
126+
127+
public PythonBinaryBuiltinNode getBuiltin(BinaryBuiltinDescriptor descriptor) {
128+
PythonBinaryBuiltinNode builtin = descriptor.createNode();
129+
if (!callerExceedsMaxSize(builtin)) {
130+
return builtin;
131+
}
132+
return null;
133+
}
134+
135+
public PythonTernaryBuiltinNode getBuiltin(TernaryBuiltinDescriptor descriptor) {
136+
PythonTernaryBuiltinNode builtin = descriptor.createNode();
137+
if (!callerExceedsMaxSize(builtin)) {
138+
return builtin;
139+
}
140+
return null;
141+
}
142+
115143
private <T extends PythonBuiltinBaseNode> boolean callerExceedsMaxSize(T builtinNode) {
116144
CompilerAsserts.neverPartOfCompilation();
117145
if (isAdoptable() && !isMaxSizeExceeded()) {
146+
// to avoid building up AST of recursive builtin calls we check that the same builtin
147+
// isn't already our parent.
148+
Class<? extends PythonBuiltinBaseNode> builtinClass = builtinNode.getClass();
149+
Node parent = getParent();
150+
int recursiveCalls = 0;
151+
while (parent != null && !(parent instanceof RootNode)) {
152+
if (parent.getClass() == builtinClass) {
153+
int recursionLimit = PythonLanguage.get(this).getEngineOption(PythonOptions.NodeRecursionLimit);
154+
if (recursiveCalls == recursionLimit) {
155+
return true;
156+
}
157+
recursiveCalls++;
158+
}
159+
parent = parent.getParent();
160+
}
161+
118162
RootNode root = getRootNode();
119-
int n = root instanceof PRootNode ? ((PRootNode) root).getNodeCount() : NodeUtil.countNodes(root);
120163
// nb: option 'BuiltinsInliningMaxCallerSize' is defined as a compatible option, i.e.,
121-
// ASTs will only we shared between contexts that have the same value for this option.
164+
// ASTs will only be shared between contexts that have the same value for this option.
122165
int maxSize = PythonLanguage.get(this).getEngineOption(PythonOptions.BuiltinsInliningMaxCallerSize);
123-
if (n >= maxSize || n + NodeUtil.countNodes(builtinNode) >= maxSize) {
124-
setMaxSizeExceeded(true);
125-
return true;
166+
if (root instanceof PRootNode) {
167+
PRootNode pRoot = (PRootNode) root;
168+
int rootNodeCount = pRoot.getNodeCountForInlining();
169+
if (rootNodeCount < maxSize) {
170+
NodeCounterWithLimit counter = new NodeCounterWithLimit(rootNodeCount, maxSize);
171+
builtinNode.accept(counter);
172+
if (counter.isOverLimit()) {
173+
setMaxSizeExceeded(true);
174+
return true;
175+
}
176+
pRoot.setNodeCountForInlining(counter.getCount());
177+
}
178+
} else {
179+
NodeCounterWithLimit counter = new NodeCounterWithLimit(maxSize);
180+
root.accept(counter);
181+
if (!counter.isOverLimit()) {
182+
builtinNode.accept(counter);
183+
}
184+
if (counter.isOverLimit()) {
185+
setMaxSizeExceeded(true);
186+
return true;
187+
}
126188
}
127189
return false;
128190
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/call/special/CallBinaryMethodNode.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -88,10 +88,10 @@ public final Object executeObject(Object callable, Object arg1, Object arg2) {
8888
return executeObject(null, callable, arg1, arg2);
8989
}
9090

91-
@Specialization(guards = "cachedInfo == info", limit = "getCallSiteInlineCacheMaxDepth()")
91+
@Specialization(guards = {"cachedInfo == info", "node != null"}, limit = "getCallSiteInlineCacheMaxDepth()")
9292
static Object callBinarySpecialMethodSlotInlined(VirtualFrame frame, @SuppressWarnings("unused") BinaryBuiltinDescriptor info, Object arg1, Object arg2,
9393
@SuppressWarnings("unused") @Cached("info") BinaryBuiltinDescriptor cachedInfo,
94-
@Cached("cachedInfo.createNode()") PythonBinaryBuiltinNode node) {
94+
@Cached("getBuiltin(cachedInfo)") PythonBinaryBuiltinNode node) {
9595
if (cachedInfo.isReverseOperation()) {
9696
return node.execute(frame, arg2, arg1);
9797
} else {
@@ -103,11 +103,11 @@ protected static boolean hasAllowedArgsNum(BuiltinMethodDescriptor descr) {
103103
return descr.minNumOfPositionalArgs() <= 2;
104104
}
105105

106-
@Specialization(guards = "cachedInfo == info", limit = "getCallSiteInlineCacheMaxDepth()")
106+
@Specialization(guards = {"cachedInfo == info", "node != null"}, limit = "getCallSiteInlineCacheMaxDepth()")
107107
Object callTernarySpecialMethodSlotInlined(VirtualFrame frame, @SuppressWarnings("unused") TernaryBuiltinDescriptor info, Object arg1, Object arg2,
108108
@SuppressWarnings("unused") @Cached("info") TernaryBuiltinDescriptor cachedInfo,
109109
@Cached("hasAllowedArgsNum(cachedInfo)") boolean hasValidArgsNum,
110-
@Cached("cachedInfo.createNode()") PythonTernaryBuiltinNode node) {
110+
@Cached("getBuiltin(cachedInfo)") PythonTernaryBuiltinNode node) {
111111
raiseInvalidArgsNumUncached(hasValidArgsNum, cachedInfo);
112112
if (cachedInfo.isReverseOperation()) {
113113
return node.execute(frame, arg2, arg1, PNone.NO_VALUE);

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/call/special/CallTernaryMethodNode.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,10 +73,10 @@ public static CallTernaryMethodNode getUncached() {
7373

7474
public abstract Object execute(Frame frame, Object callable, Object arg1, Object arg2, Object arg3);
7575

76-
@Specialization(guards = "cachedInfo == info", limit = "getCallSiteInlineCacheMaxDepth()")
76+
@Specialization(guards = {"cachedInfo == info", "node != null"}, limit = "getCallSiteInlineCacheMaxDepth()")
7777
static Object callSpecialMethodSlotInlined(VirtualFrame frame, @SuppressWarnings("unused") TernaryBuiltinDescriptor info, Object arg1, Object arg2, Object arg3,
7878
@SuppressWarnings("unused") @Cached("info") TernaryBuiltinDescriptor cachedInfo,
79-
@Cached("cachedInfo.createNode()") PythonTernaryBuiltinNode node) {
79+
@Cached("getBuiltin(cachedInfo)") PythonTernaryBuiltinNode node) {
8080
return node.execute(frame, arg1, arg2, arg3);
8181
}
8282

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/call/special/CallUnaryMethodNode.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -84,31 +84,31 @@ public final Object executeObject(Object callable, Object receiver) {
8484
return executeObject(null, callable, receiver);
8585
}
8686

87-
@Specialization(guards = "cachedInfo == info", limit = "getCallSiteInlineCacheMaxDepth()")
87+
@Specialization(guards = {"cachedInfo == info", "node != null"}, limit = "getCallSiteInlineCacheMaxDepth()")
8888
Object callUnarySpecialMethodSlotInlined(VirtualFrame frame, @SuppressWarnings("unused") UnaryBuiltinDescriptor info, Object receiver,
8989
@SuppressWarnings("unused") @Cached("info") UnaryBuiltinDescriptor cachedInfo,
90-
@Cached("cachedInfo.createNode()") PythonUnaryBuiltinNode node) {
90+
@Cached("getBuiltin(cachedInfo)") PythonUnaryBuiltinNode node) {
9191
return node.execute(frame, receiver);
9292
}
9393

9494
protected static boolean hasAllowedArgsNum(BuiltinMethodDescriptor descr) {
9595
return descr.minNumOfPositionalArgs() <= 1;
9696
}
9797

98-
@Specialization(guards = "cachedInfo == info", limit = "getCallSiteInlineCacheMaxDepth()")
98+
@Specialization(guards = {"cachedInfo == info", "node != null"}, limit = "getCallSiteInlineCacheMaxDepth()")
9999
Object callBinarySpecialMethodSlotInlined(VirtualFrame frame, @SuppressWarnings("unused") BinaryBuiltinDescriptor info, Object receiver,
100100
@SuppressWarnings("unused") @Cached("info") BinaryBuiltinDescriptor cachedInfo,
101101
@Cached("hasAllowedArgsNum(cachedInfo)") boolean hasValidArgsNum,
102-
@Cached("cachedInfo.createNode()") PythonBinaryBuiltinNode node) {
102+
@Cached("getBuiltin(cachedInfo)") PythonBinaryBuiltinNode node) {
103103
raiseInvalidArgsNumUncached(hasValidArgsNum, cachedInfo);
104104
return node.execute(frame, receiver, PNone.NO_VALUE);
105105
}
106106

107-
@Specialization(guards = "cachedInfo == info", limit = "getCallSiteInlineCacheMaxDepth()")
107+
@Specialization(guards = {"cachedInfo == info", "node != null"}, limit = "getCallSiteInlineCacheMaxDepth()")
108108
Object callTernarySpecialMethodSlotInlined(VirtualFrame frame, @SuppressWarnings("unused") TernaryBuiltinDescriptor info, Object receiver,
109109
@SuppressWarnings("unused") @Cached("info") TernaryBuiltinDescriptor cachedInfo,
110110
@Cached("hasAllowedArgsNum(cachedInfo)") boolean hasValidArgsNum,
111-
@Cached("cachedInfo.createNode()") PythonTernaryBuiltinNode node) {
111+
@Cached("getBuiltin(cachedInfo)") PythonTernaryBuiltinNode node) {
112112
raiseInvalidArgsNumUncached(hasValidArgsNum, cachedInfo);
113113
return node.execute(frame, receiver, PNone.NO_VALUE, PNone.NO_VALUE);
114114
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/PythonOptions.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,7 @@ private PythonOptions() {
278278
public static final OptionKey<Boolean> UseReprForPrintString = new OptionKey<>(true);
279279

280280
@EngineOption @Option(category = OptionCategory.EXPERT, usageSyntax = "<limit>", help = "Stop inlining of builtins if caller's cumulative tree size would exceed this limit") //
281-
public static final OptionKey<Integer> BuiltinsInliningMaxCallerSize = new OptionKey<>(2250);
281+
public static final OptionKey<Integer> BuiltinsInliningMaxCallerSize = new OptionKey<>(2500);
282282

283283
@Option(category = OptionCategory.EXPERT, usageSyntax = "true|false", help = "Disable weakref callback processing, signal handling, and other periodic async actions.") //
284284
public static final OptionKey<Boolean> NoAsyncActions = new OptionKey<>(false);

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/util/PythonUtils.java

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,8 @@
6060
import javax.management.ObjectName;
6161
import javax.management.ReflectionException;
6262

63+
import com.oracle.truffle.api.nodes.Node;
64+
import com.oracle.truffle.api.nodes.NodeVisitor;
6365
import org.graalvm.nativeimage.ImageInfo;
6466

6567
import com.oracle.graal.python.PythonLanguage;
@@ -690,4 +692,30 @@ public static TruffleString[] objectArrayToTruffleStringArray(Object[] array, Ca
690692
}
691693
return result;
692694
}
695+
696+
public static final class NodeCounterWithLimit implements NodeVisitor {
697+
private int count;
698+
private final int limit;
699+
700+
public NodeCounterWithLimit(int counterStart, int limit) {
701+
this.count = counterStart;
702+
this.limit = limit;
703+
}
704+
705+
public NodeCounterWithLimit(int limit) {
706+
this.limit = limit;
707+
}
708+
709+
public boolean visit(Node node) {
710+
return ++count < limit;
711+
}
712+
713+
public int getCount() {
714+
return count;
715+
}
716+
717+
public boolean isOverLimit() {
718+
return count >= limit;
719+
}
720+
}
693721
}

0 commit comments

Comments
 (0)