Skip to content

Commit 5c5e487

Browse files
committed
improve branch profiling for ObjectBuiltins.overridesBuiltinMethod in multi-context case
1 parent 9925d39 commit 5c5e487

File tree

2 files changed

+44
-10
lines changed

2 files changed

+44
-10
lines changed

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/BuiltinConstructors.java

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@
9898
import java.util.Arrays;
9999
import java.util.List;
100100

101+
import com.oracle.graal.python.PythonLanguage;
101102
import com.oracle.graal.python.annotations.ArgumentClinic;
102103
import com.oracle.graal.python.builtins.Builtin;
103104
import com.oracle.graal.python.builtins.CoreFunctions;
@@ -1592,6 +1593,8 @@ public abstract static class ObjectNode extends PythonVarargsBuiltinNode {
15921593
@Child private LookupAttributeInMRONode lookupNew;
15931594
@CompilationFinal private ValueProfile profileInit;
15941595
@CompilationFinal private ValueProfile profileNew;
1596+
@CompilationFinal private ValueProfile profileInitFactory;
1597+
@CompilationFinal private ValueProfile profileNewFactory;
15951598

15961599
@Override
15971600
public final Object varArgExecute(VirtualFrame frame, @SuppressWarnings("unused") Object self, Object[] arguments, PKeyword[] keywords) throws VarargsBuiltinDirectInvocationNotSupported {
@@ -1680,16 +1683,31 @@ private void checkExcessArgs(Object type, Object[] varargs, PKeyword[] kwargs) {
16801683
}
16811684
if (profileNew == null) {
16821685
CompilerDirectives.transferToInterpreterAndInvalidate();
1683-
profileNew = ValueProfile.createIdentityProfile();
1686+
if (PythonLanguage.getCurrent().singleContextAssumption.isValid()) {
1687+
profileNew = ValueProfile.createIdentityProfile();
1688+
} else {
1689+
profileNew = ValueProfile.createClassProfile();
1690+
}
16841691
}
16851692
if (profileInit == null) {
16861693
CompilerDirectives.transferToInterpreterAndInvalidate();
1687-
profileInit = ValueProfile.createIdentityProfile();
1694+
if (PythonLanguage.getCurrent().singleContextAssumption.isValid()) {
1695+
profileInit = ValueProfile.createIdentityProfile();
1696+
} else {
1697+
profileInit = ValueProfile.createClassProfile();
1698+
}
1699+
}
1700+
if (profileNewFactory == null) {
1701+
CompilerDirectives.transferToInterpreterAndInvalidate();
1702+
profileNewFactory = ValueProfile.createIdentityProfile();
1703+
}
1704+
if (profileInitFactory == null) {
1705+
profileInitFactory = ValueProfile.createIdentityProfile();
16881706
}
1689-
if (ObjectBuiltins.InitNode.overridesBuiltinMethod(type, profileNew, lookupNew, BuiltinConstructorsFactory.ObjectNodeFactory.class)) {
1707+
if (ObjectBuiltins.InitNode.overridesBuiltinMethod(type, profileNew, lookupNew, profileNewFactory, BuiltinConstructorsFactory.ObjectNodeFactory.class)) {
16901708
throw raise(TypeError, ErrorMessages.NEW_TAKES_ONE_ARG);
16911709
}
1692-
if (!ObjectBuiltins.InitNode.overridesBuiltinMethod(type, profileInit, lookupInit, ObjectBuiltinsFactory.InitNodeFactory.class)) {
1710+
if (!ObjectBuiltins.InitNode.overridesBuiltinMethod(type, profileInit, lookupInit, profileInitFactory, ObjectBuiltinsFactory.InitNodeFactory.class)) {
16931711
throw raise(TypeError, ErrorMessages.NEW_TAKES_NO_ARGS, type);
16941712
}
16951713
}

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/object/ObjectBuiltins.java

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -232,28 +232,44 @@ public PNone initNoArgs(Object self, Object[] arguments, PKeyword[] keywords) {
232232
@SuppressWarnings("unused")
233233
public PNone init(Object self, Object[] arguments, PKeyword[] keywords,
234234
@CachedLibrary("self") PythonObjectLibrary lib,
235+
@Cached ConditionProfile overridesNew,
236+
@Cached ConditionProfile overridesInit,
235237
@Cached("create(__INIT__)") LookupAttributeInMRONode lookupInit,
236-
@Cached("createIdentityProfile()") ValueProfile profileInit,
238+
@Cached("createLookupProfile()") ValueProfile profileInit,
239+
@Cached("createIdentityProfile()") ValueProfile profileInitFactory,
237240
@Cached("create(__NEW__)") LookupAttributeInMRONode lookupNew,
238-
@Cached("createIdentityProfile()") ValueProfile profileNew) {
241+
@Cached("createLookupProfile()") ValueProfile profileNew,
242+
@Cached("createIdentityProfile()") ValueProfile profileNewFactory) {
239243
if (arguments.length != 0 || keywords.length != 0) {
240244
Object type = lib.getLazyPythonClass(self);
241-
if (overridesBuiltinMethod(type, profileInit, lookupInit, ObjectBuiltinsFactory.InitNodeFactory.class)) {
245+
if (overridesNew.profile(overridesBuiltinMethod(type, profileInit, lookupInit, profileInitFactory, ObjectBuiltinsFactory.InitNodeFactory.class))) {
242246
throw raise(TypeError, ErrorMessages.INIT_TAKES_ONE_ARG_OBJECT);
243247
}
244248

245-
if (!overridesBuiltinMethod(type, profileInit, lookupNew, BuiltinConstructorsFactory.ObjectNodeFactory.class)) {
249+
if (overridesInit.profile(!overridesBuiltinMethod(type, profileInit, lookupNew, profileNewFactory, BuiltinConstructorsFactory.ObjectNodeFactory.class))) {
246250
throw raise(TypeError, ErrorMessages.INIT_TAKES_ONE_ARG, type);
247251
}
248252
}
249253
return PNone.NONE;
250254
}
251255

256+
protected static ValueProfile createLookupProfile() {
257+
if (PythonLanguage.getCurrent().singleContextAssumption.isValid()) {
258+
return ValueProfile.createIdentityProfile();
259+
} else {
260+
return ValueProfile.createClassProfile();
261+
}
262+
}
263+
264+
/**
265+
* Simple utility method to check if a method was overridden. The {@code profile} parameter
266+
* must {@emph not} be an identity profile when AST sharing is enabled.
267+
*/
252268
public static <T extends NodeFactory<? extends PythonBuiltinBaseNode>> boolean overridesBuiltinMethod(Object type, ValueProfile profile, LookupAttributeInMRONode lookup,
253-
Class<T> builtinNodeFactoryClass) {
269+
ValueProfile factoryProfile, Class<T> builtinNodeFactoryClass) {
254270
Object method = profile.profile(lookup.execute(type));
255271
if (method instanceof PBuiltinFunction) {
256-
NodeFactory<? extends PythonBuiltinBaseNode> factory = ((PBuiltinFunction) method).getBuiltinNodeFactory();
272+
NodeFactory<? extends PythonBuiltinBaseNode> factory = factoryProfile.profile(((PBuiltinFunction) method).getBuiltinNodeFactory());
257273
return !builtinNodeFactoryClass.isInstance(factory);
258274
}
259275
return true;

0 commit comments

Comments
 (0)