Skip to content

Commit d94c92b

Browse files
cosminbascacosminbasca
committed
GR-14471: function builtins - improve access performance for argument defaults and keyword defaults
1 parent 1c71d93 commit d94c92b

File tree

9 files changed

+291
-270
lines changed

9 files changed

+291
-270
lines changed

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/function/FunctionBuiltins.java

Lines changed: 14 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -46,14 +46,14 @@
4646
import com.oracle.graal.python.builtins.objects.code.PCode;
4747
import com.oracle.graal.python.builtins.objects.common.HashingStorage;
4848
import com.oracle.graal.python.builtins.objects.dict.PDict;
49-
import com.oracle.graal.python.builtins.objects.function.FunctionBuiltinsFactory.GetFunctionDefaultsNodeFactory;
50-
import com.oracle.graal.python.builtins.objects.function.FunctionBuiltinsFactory.GetFunctionKeywordDefaultsNodeFactory;
5149
import com.oracle.graal.python.builtins.objects.method.PMethod;
5250
import com.oracle.graal.python.builtins.objects.str.PString;
5351
import com.oracle.graal.python.builtins.objects.tuple.PTuple;
5452
import com.oracle.graal.python.nodes.attributes.ReadAttributeFromObjectNode;
5553
import com.oracle.graal.python.nodes.attributes.WriteAttributeToObjectNode;
56-
import com.oracle.graal.python.nodes.code.GetFunctionCodeNode;
54+
import com.oracle.graal.python.nodes.builtins.FunctionNodes.GetFunctionCodeNode;
55+
import com.oracle.graal.python.nodes.builtins.FunctionNodes.GetFunctionDefaultsNode;
56+
import com.oracle.graal.python.nodes.builtins.FunctionNodes.GetFunctionKeywordDefaultsNode;
5757
import com.oracle.graal.python.nodes.function.PythonBuiltinBaseNode;
5858
import com.oracle.graal.python.nodes.function.builtins.PythonBinaryBuiltinNode;
5959
import com.oracle.graal.python.nodes.function.builtins.PythonUnaryBuiltinNode;
@@ -139,12 +139,13 @@ Object setName(@SuppressWarnings("unused") Object self, @SuppressWarnings("unuse
139139

140140
@Builtin(name = __DEFAULTS__, minNumOfPositionalArgs = 1, maxNumOfPositionalArgs = 2, isGetter = true, isSetter = true)
141141
@GenerateNodeFactory
142-
public abstract static class GetFunctionDefaultsNode extends PythonBinaryBuiltinNode {
142+
public abstract static class GetDefaultsNode extends PythonBinaryBuiltinNode {
143143
@Specialization(guards = "isNoValue(defaults)")
144-
Object defaults(PFunction self, @SuppressWarnings("unused") PNone defaults) {
145-
Object[] d = self.getDefaults();
146-
assert d != null;
147-
return (d.length == 0) ? PNone.NONE : factory().createTuple(d);
144+
Object defaults(PFunction self, @SuppressWarnings("unused") PNone defaults,
145+
@Cached("create()") GetFunctionDefaultsNode getFunctionDefaultsNode) {
146+
Object[] argDefaults = getFunctionDefaultsNode.execute(self);
147+
assert argDefaults != null;
148+
return (argDefaults.length == 0) ? PNone.NONE : factory().createTuple(argDefaults);
148149
}
149150

150151
@Specialization
@@ -158,23 +159,16 @@ Object setDefaults(PFunction self, @SuppressWarnings("unused") PNone defaults) {
158159
self.setDefaults(new Object[0]);
159160
return PNone.NONE;
160161
}
161-
162-
public static GetFunctionDefaultsNode create() {
163-
return GetFunctionDefaultsNodeFactory.create();
164-
}
165162
}
166163

167164
@Builtin(name = __KWDEFAULTS__, minNumOfPositionalArgs = 1, maxNumOfPositionalArgs = 2, isGetter = true, isSetter = true)
168165
@GenerateNodeFactory
169-
public abstract static class GetFunctionKeywordDefaultsNode extends PythonBinaryBuiltinNode {
166+
public abstract static class GetKeywordDefaultsNode extends PythonBinaryBuiltinNode {
170167
@Specialization(guards = "isNoValue(arg)")
171-
Object get(PFunction self, @SuppressWarnings("unused") PNone arg) {
172-
PKeyword[] kwdefaults = self.getKwDefaults();
173-
if (kwdefaults.length > 0) {
174-
return factory().createDict(kwdefaults);
175-
} else {
176-
return PNone.NONE;
177-
}
168+
Object get(PFunction self, @SuppressWarnings("unused") PNone arg,
169+
@Cached("create()") GetFunctionKeywordDefaultsNode getFunctionKeywordDefaultsNode) {
170+
PKeyword[] kwdefaults = getFunctionKeywordDefaultsNode.execute(self);
171+
return (kwdefaults.length > 0) ? factory().createDict(kwdefaults) : PNone.NONE;
178172
}
179173

180174
@Specialization(guards = "!isNoValue(arg)")
@@ -197,10 +191,6 @@ Object set(PFunction self, PDict arg) {
197191
self.setKwDefaults(keywords.toArray(new PKeyword[keywords.size()]));
198192
return PNone.NONE;
199193
}
200-
201-
public static GetFunctionKeywordDefaultsNode create() {
202-
return GetFunctionKeywordDefaultsNodeFactory.create();
203-
}
204194
}
205195

206196
@Builtin(name = __REDUCE__, minNumOfPositionalArgs = 1)

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/function/PFunction.java

Lines changed: 6 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@
3737
import com.oracle.graal.python.nodes.generator.GeneratorFunctionRootNode;
3838
import com.oracle.truffle.api.Assumption;
3939
import com.oracle.truffle.api.CompilerAsserts;
40-
import com.oracle.truffle.api.CompilerDirectives;
4140
import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
4241
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
4342
import com.oracle.truffle.api.RootCallTarget;
@@ -57,9 +56,7 @@ public class PFunction extends PythonObject {
5756
private final boolean isStatic;
5857
@CompilationFinal private PCode code;
5958
@CompilationFinal(dimensions = 1) private Object[] defaultValues;
60-
private Object[] uncachedDefaultValues;
6159
@CompilationFinal(dimensions = 1) private PKeyword[] kwDefaultValues;
62-
private PKeyword[] uncachedKwDefaultValues;
6360

6461
public PFunction(LazyPythonClass clazz, String name, String enclosingClassName, RootCallTarget callTarget, PythonObject globals, PCell[] closure) {
6562
this(clazz, name, enclosingClassName, callTarget, globals, EMPTY_DEFAULTS, PKeyword.EMPTY_KEYWORDS, closure);
@@ -73,8 +70,8 @@ public PFunction(LazyPythonClass clazz, String name, String enclosingClassName,
7370
this.isStatic = name.equals(SpecialMethodNames.__NEW__);
7471
this.enclosingClassName = enclosingClassName;
7572
this.globals = globals;
76-
this.defaultValues = this.uncachedDefaultValues = defaultValues == null ? EMPTY_DEFAULTS : defaultValues;
77-
this.kwDefaultValues = this.uncachedKwDefaultValues = kwDefaultValues == null ? PKeyword.EMPTY_KEYWORDS : kwDefaultValues;
73+
this.defaultValues = defaultValues == null ? EMPTY_DEFAULTS : defaultValues;
74+
this.kwDefaultValues = kwDefaultValues == null ? PKeyword.EMPTY_KEYWORDS : kwDefaultValues;
7875
this.closure = closure;
7976
addDefaultConstants(this.getStorage(), name, enclosingClassName);
8077
}
@@ -157,41 +154,21 @@ public String getEnclosingClassName() {
157154
}
158155

159156
public Object[] getDefaults() {
160-
Assumption assumption = this.defaultsStableAssumption;
161-
if (CompilerDirectives.isCompilationConstant(this) && CompilerDirectives.isCompilationConstant(assumption)) {
162-
if (assumption.isValid()) {
163-
return defaultValues;
164-
}
165-
}
166-
return uncachedDefaultValues;
167-
}
168-
169-
public Object[] getUncachedDefaultValues() {
170-
return uncachedDefaultValues;
157+
return defaultValues;
171158
}
172159

173160
public void setDefaults(Object[] defaults) {
174161
this.defaultsStableAssumption.invalidate("defaults changed for function " + getName());
175-
this.defaultValues = this.uncachedDefaultValues = defaults;
162+
this.defaultValues = defaults;
176163
}
177164

178165
public PKeyword[] getKwDefaults() {
179-
Assumption assumption = this.defaultsStableAssumption;
180-
if (CompilerDirectives.isCompilationConstant(this) && CompilerDirectives.isCompilationConstant(assumption)) {
181-
if (assumption.isValid()) {
182-
return kwDefaultValues;
183-
}
184-
}
185-
return uncachedKwDefaultValues;
186-
}
187-
188-
public PKeyword[] getUncachedKwDefaults() {
189-
return uncachedKwDefaultValues;
166+
return kwDefaultValues;
190167
}
191168

192169
public void setKwDefaults(PKeyword[] defaults) {
193170
this.defaultsStableAssumption.invalidate("kw defaults changed for function " + getName());
194-
this.kwDefaultValues = this.uncachedKwDefaultValues = defaults;
171+
this.kwDefaultValues = defaults;
195172
}
196173

197174
@TruffleBoundary

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/method/MethodBuiltins.java

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,12 @@
4242
import com.oracle.graal.python.builtins.PythonBuiltinClassType;
4343
import com.oracle.graal.python.builtins.PythonBuiltins;
4444
import com.oracle.graal.python.builtins.objects.PNone;
45-
import com.oracle.graal.python.builtins.objects.function.FunctionBuiltins;
45+
import com.oracle.graal.python.builtins.objects.function.PKeyword;
4646
import com.oracle.graal.python.builtins.objects.type.TypeNodes.GetNameNode;
4747
import com.oracle.graal.python.nodes.SpecialAttributeNames;
4848
import com.oracle.graal.python.nodes.attributes.GetAttributeNode;
49+
import com.oracle.graal.python.nodes.builtins.FunctionNodes.GetDefaultsNode;
50+
import com.oracle.graal.python.nodes.builtins.FunctionNodes.GetKeywordDefaultsNode;
4951
import com.oracle.graal.python.nodes.call.special.LookupAndCallBinaryNode;
5052
import com.oracle.graal.python.nodes.function.PythonBuiltinBaseNode;
5153
import com.oracle.graal.python.nodes.function.PythonBuiltinNode;
@@ -113,18 +115,21 @@ protected static GetAttributeNode createGetAttributeNode() {
113115
public abstract static class GetMethodDefaultsNode extends PythonUnaryBuiltinNode {
114116
@Specialization
115117
Object defaults(PMethod self,
116-
@Cached("create()") FunctionBuiltins.GetFunctionDefaultsNode getFunctionDefaultsNode) {
117-
return getFunctionDefaultsNode.execute(self.getFunction(), PNone.NO_VALUE);
118+
@Cached("create()") GetDefaultsNode getDefaultsNode) {
119+
Object[] argDefaults = getDefaultsNode.execute(self);
120+
assert argDefaults != null;
121+
return (argDefaults.length == 0) ? PNone.NONE : factory().createTuple(argDefaults);
118122
}
119123
}
120124

121125
@Builtin(name = __KWDEFAULTS__, minNumOfPositionalArgs = 1, isGetter = true)
122126
@GenerateNodeFactory
123127
public abstract static class GetMethodKwdefaultsNode extends PythonUnaryBuiltinNode {
124128
@Specialization
125-
Object defaults(PMethod self,
126-
@Cached("create()") FunctionBuiltins.GetFunctionKeywordDefaultsNode getFunctionKwdefaultsNode) {
127-
return getFunctionKwdefaultsNode.execute(self.getFunction(), PNone.NO_VALUE);
129+
Object kwDefaults(PMethod self,
130+
@Cached("create()") GetKeywordDefaultsNode getKeywordDefaultsNode) {
131+
PKeyword[] kwdefaults = getKeywordDefaultsNode.execute(self);
132+
return (kwdefaults.length > 0) ? factory().createDict(kwdefaults) : PNone.NONE;
128133
}
129134
}
130135

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,10 @@ public abstract class PGuards {
8989
* Specialization guards.
9090
*/
9191

92+
public static boolean isSameObject(Object left, Object right) {
93+
return left == right;
94+
}
95+
9296
public static boolean isEmpty(Object[] array) {
9397
return array.length == 0;
9498
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/argument/CreateArgumentsNode.java

Lines changed: 21 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,9 @@
6060
import com.oracle.graal.python.nodes.argument.CreateArgumentsNodeGen.FillKwDefaultsNodeGen;
6161
import com.oracle.graal.python.nodes.argument.CreateArgumentsNodeGen.FindKwDefaultNodeGen;
6262
import com.oracle.graal.python.nodes.argument.CreateArgumentsNodeGen.HandleTooManyArgumentsNodeGen;
63-
import com.oracle.graal.python.nodes.code.GetSignatureNode;
63+
import com.oracle.graal.python.nodes.builtins.FunctionNodes.GetDefaultsNode;
64+
import com.oracle.graal.python.nodes.builtins.FunctionNodes.GetKeywordDefaultsNode;
65+
import com.oracle.graal.python.nodes.builtins.FunctionNodes.GetSignatureNode;
6466
import com.oracle.graal.python.runtime.PythonOptions;
6567
import com.oracle.graal.python.runtime.exception.PException;
6668
import com.oracle.truffle.api.CompilerDirectives;
@@ -81,13 +83,15 @@ public static CreateArgumentsNode create() {
8183
Object[] doMethodCached(PythonObject method, Object[] userArguments, PKeyword[] keywords,
8284
@Cached("create()") CreateAndCheckArgumentsNode createAndCheckArgumentsNode,
8385
@Cached("create()") GetSignatureNode getSignatureNode,
86+
@Cached("create()") GetDefaultsNode getDefaultsNode,
87+
@Cached("create()") GetKeywordDefaultsNode getKwDefaultsNode,
8488
@Cached("method") @SuppressWarnings("unused") PythonObject cachedMethod) {
8589

8690
// We do not directly cache these objects because they are compilation final anyway and the
8791
// getter check the appropriate assumptions.
8892
Signature signature = getSignatureNode.execute(cachedMethod);
89-
Object[] defaults = getDefaults(cachedMethod);
90-
PKeyword[] kwdefaults = getKwDefaults(cachedMethod);
93+
Object[] defaults = getDefaultsNode.execute(cachedMethod);
94+
PKeyword[] kwdefaults = getKwDefaultsNode.execute(cachedMethod);
9195
Object self = getSelf(cachedMethod);
9296
return createAndCheckArgumentsNode.execute(method, userArguments, keywords, signature, self, defaults, kwdefaults, isMethodCall(self));
9397
}
@@ -98,27 +102,31 @@ Object[] doMethodFunctionAndSelfCached(PythonObject method, Object[] userArgumen
98102
@Cached("create()") CreateAndCheckArgumentsNode createAndCheckArgumentsNode,
99103
@Cached("getFunction(method)") @SuppressWarnings("unused") Object cachedFunction,
100104
@Cached("create()") GetSignatureNode getSignatureNode,
105+
@Cached("create()") GetDefaultsNode getDefaultsNode,
106+
@Cached("create()") GetKeywordDefaultsNode getKwDefaultsNode,
101107
@Cached("getSelf(method)") Object cachedSelf) {
102108

103109
// We do not directly cache these objects because they are compilation final anyway and the
104110
// getter check the appropriate assumptions.
105111
Signature signature = getSignatureNode.execute(cachedFunction);
106-
Object[] defaults = getDefaults(cachedFunction);
107-
PKeyword[] kwdefaults = getKwDefaults(cachedFunction);
112+
Object[] defaults = getDefaultsNode.execute(cachedFunction);
113+
PKeyword[] kwdefaults = getKwDefaultsNode.execute(cachedFunction);
108114
return createAndCheckArgumentsNode.execute(method, userArguments, keywords, signature, cachedSelf, defaults, kwdefaults, isMethodCall(cachedSelf));
109115
}
110116

111117
@Specialization(guards = {"isMethod(method)", "getFunction(method) == cachedFunction"}, limit = "getVariableArgumentInlineCacheLimit()", replaces = "doMethodFunctionAndSelfCached")
112118
Object[] doMethodFunctionCached(PythonObject method, Object[] userArguments, PKeyword[] keywords,
113119
@Cached("create()") CreateAndCheckArgumentsNode createAndCheckArgumentsNode,
114120
@Cached("create()") GetSignatureNode getSignatureNode,
121+
@Cached("create()") GetDefaultsNode getDefaultsNode,
122+
@Cached("create()") GetKeywordDefaultsNode getKwDefaultsNode,
115123
@Cached("getFunction(method)") @SuppressWarnings("unused") Object cachedFunction) {
116124

117125
// We do not directly cache these objects because they are compilation final anyway and the
118126
// getter check the appropriate assumptions.
119127
Signature signature = getSignatureNode.execute(cachedFunction);
120-
Object[] defaults = getDefaults(cachedFunction);
121-
PKeyword[] kwdefaults = getKwDefaults(cachedFunction);
128+
Object[] defaults = getDefaultsNode.execute(cachedFunction);
129+
PKeyword[] kwdefaults = getKwDefaultsNode.execute(cachedFunction);
122130
Object self = getSelf(method);
123131
return createAndCheckArgumentsNode.execute(method, userArguments, keywords, signature, self, defaults, kwdefaults, isMethodCall(self));
124132
}
@@ -127,13 +135,15 @@ Object[] doMethodFunctionCached(PythonObject method, Object[] userArguments, PKe
127135
Object[] doFunctionCached(PythonObject callable, Object[] userArguments, PKeyword[] keywords,
128136
@Cached("create()") CreateAndCheckArgumentsNode createAndCheckArgumentsNode,
129137
@Cached("create()") GetSignatureNode getSignatureNode,
138+
@Cached("create()") GetDefaultsNode getDefaultsNode,
139+
@Cached("create()") GetKeywordDefaultsNode getKwDefaultsNode,
130140
@Cached("callable") @SuppressWarnings("unused") PythonObject cachedCallable) {
131141

132142
// We do not directly cache these objects because they are compilation final anyway and the
133143
// getter check the appropriate assumptions.
134144
Signature signature = getSignatureNode.execute(cachedCallable);
135-
Object[] defaults = getDefaults(cachedCallable);
136-
PKeyword[] kwdefaults = getKwDefaults(cachedCallable);
145+
Object[] defaults = getDefaultsNode.execute(cachedCallable);
146+
PKeyword[] kwdefaults = getKwDefaultsNode.execute(cachedCallable);
137147
return createAndCheckArgumentsNode.execute(callable, userArguments, keywords, signature, null, defaults, kwdefaults, false);
138148
}
139149

@@ -589,14 +599,6 @@ protected static PKeyword[] getKwDefaultsUncached(Object callable) {
589599
return getProperty(callable, UncachedKwDefaultsGetter.INSTANCE);
590600
}
591601

592-
protected static Object[] getDefaults(Object callable) {
593-
return getProperty(callable, DefaultsGetter.INSTANCE);
594-
}
595-
596-
protected static PKeyword[] getKwDefaults(Object callable) {
597-
return getProperty(callable, KwDefaultsGetter.INSTANCE);
598-
}
599-
600602
protected static String getName(Object callable) {
601603
return getProperty(callable, NameGetter.INSTANCE);
602604
}
@@ -653,34 +655,6 @@ private abstract static class Getter<T> {
653655
public abstract T fromPBuiltinFunction(PBuiltinFunction fun);
654656
}
655657

656-
private static final class DefaultsGetter extends Getter<Object[]> {
657-
private static final DefaultsGetter INSTANCE = new DefaultsGetter();
658-
659-
@Override
660-
public Object[] fromPFunction(PFunction fun) {
661-
return fun.getDefaults();
662-
}
663-
664-
@Override
665-
public Object[] fromPBuiltinFunction(PBuiltinFunction fun) {
666-
return fun.getDefaults();
667-
}
668-
}
669-
670-
private static final class KwDefaultsGetter extends Getter<PKeyword[]> {
671-
private static final KwDefaultsGetter INSTANCE = new KwDefaultsGetter();
672-
673-
@Override
674-
public PKeyword[] fromPFunction(PFunction fun) {
675-
return fun.getKwDefaults();
676-
}
677-
678-
@Override
679-
public PKeyword[] fromPBuiltinFunction(PBuiltinFunction fun) {
680-
return fun.getKwDefaults();
681-
}
682-
}
683-
684658
private static final class UncachedSignatureGetter extends Getter<Signature> {
685659
private static final UncachedSignatureGetter INSTANCE = new UncachedSignatureGetter();
686660

@@ -700,7 +674,7 @@ private static final class UncachedDefaultsGetter extends Getter<Object[]> {
700674

701675
@Override
702676
public Object[] fromPFunction(PFunction fun) {
703-
return fun.getUncachedDefaultValues();
677+
return fun.getDefaults();
704678
}
705679

706680
@Override
@@ -714,7 +688,7 @@ private static final class UncachedKwDefaultsGetter extends Getter<PKeyword[]> {
714688

715689
@Override
716690
public PKeyword[] fromPFunction(PFunction fun) {
717-
return fun.getUncachedKwDefaults();
691+
return fun.getKwDefaults();
718692
}
719693

720694
@Override

0 commit comments

Comments
 (0)