Skip to content

Commit cb2ed0b

Browse files
msimacekmsimacek
committed
[GR-34554] Simplify dict check
PullRequest: graalpython/2001
2 parents 8fa857c + 6dd8029 commit cb2ed0b

File tree

8 files changed

+132
-36
lines changed

8 files changed

+132
-36
lines changed

‎graalpython/com.oracle.graal.python.test/src/tests/test_class.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,3 +46,9 @@ def inner(other):
4646
return inner
4747

4848
assert A() == A()
49+
50+
51+
def test_new_not_descriptor():
52+
class C:
53+
__new__ = str
54+
assert C() == str(C)

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/common/DynamicObjectStorage.java

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,10 @@
4848
import com.oracle.graal.python.builtins.objects.PNone;
4949
import com.oracle.graal.python.builtins.objects.common.HashingStorageLibrary.ForEachNode;
5050
import com.oracle.graal.python.builtins.objects.common.HashingStorageLibrary.HashingStorageIterable;
51+
import com.oracle.graal.python.builtins.objects.dict.PDict;
5152
import com.oracle.graal.python.builtins.objects.function.PArguments;
5253
import com.oracle.graal.python.builtins.objects.function.PArguments.ThreadState;
54+
import com.oracle.graal.python.builtins.objects.object.PythonObject;
5355
import com.oracle.graal.python.builtins.objects.str.PString;
5456
import com.oracle.graal.python.lib.PyObjectHashNode;
5557
import com.oracle.graal.python.lib.PyObjectRichCompareBool;
@@ -394,9 +396,30 @@ private static Object runNode(DynamicObjectStorage self, Object key, Object acc,
394396
}
395397

396398
@ExportMessage
397-
public HashingStorage clear(@Shared("dylib") @CachedLibrary(limit = "3") DynamicObjectLibrary dylib) {
398-
dylib.resetShape(store, PythonLanguage.get(dylib).getEmptyShape());
399-
return this;
399+
@ImportStatic(PGuards.class)
400+
static class Clear {
401+
@Specialization(guards = "!isPythonObject(receiver.getStore())")
402+
static HashingStorage clearPlain(DynamicObjectStorage receiver,
403+
@Shared("dylib") @CachedLibrary(limit = "3") DynamicObjectLibrary dylib) {
404+
dylib.resetShape(receiver.getStore(), PythonLanguage.get(dylib).getEmptyShape());
405+
return receiver;
406+
}
407+
408+
@Specialization(guards = "isPythonObject(receiver.getStore())")
409+
static HashingStorage clearObjectBacked(DynamicObjectStorage receiver,
410+
@Shared("dylib") @CachedLibrary(limit = "3") DynamicObjectLibrary dylib) {
411+
/*
412+
* We cannot use resetShape as that would lose hidden keys, such as CLASS or OBJ_ID.
413+
* Construct a new storage instead and set it as the object's __dict__'s storage.
414+
*/
415+
DynamicObjectStorage newStorage = new DynamicObjectStorage(new Store(PythonLanguage.get(dylib).getEmptyShape()));
416+
PythonObject owner = (PythonObject) receiver.getStore();
417+
PDict dict = (PDict) dylib.getOrDefault(owner, PythonObject.DICT, null);
418+
if (dict != null && dict.getDictStorage() == receiver) {
419+
dict.setDictStorage(newStorage);
420+
}
421+
return newStorage;
422+
}
400423
}
401424

402425
@ExportMessage

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

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import com.oracle.graal.python.builtins.PythonBuiltinClassType;
3434
import com.oracle.graal.python.builtins.objects.PNone;
3535
import com.oracle.graal.python.builtins.objects.PythonAbstractObject;
36+
import com.oracle.graal.python.builtins.objects.dict.PDict;
3637
import com.oracle.graal.python.builtins.objects.type.PythonBuiltinClass;
3738
import com.oracle.graal.python.builtins.objects.type.PythonManagedClass;
3839
import com.oracle.graal.python.nodes.HiddenAttributes;
@@ -47,13 +48,20 @@
4748

4849
public class PythonObject extends PythonAbstractObject {
4950
public static final HiddenKey DICT = HiddenAttributes.DICT;
50-
public static final byte CLASS_CHANGED_FLAG = 1;
51-
public static final byte HAS_SLOTS_BUT_NO_DICT_FLAG = 2;
51+
public static final byte CLASS_CHANGED_FLAG = 0b1;
52+
/**
53+
* Indicates that the object doesn't allow {@code __dict__}, but may have slots
54+
*/
55+
public static final byte HAS_SLOTS_BUT_NO_DICT_FLAG = 0b10;
5256
/**
5357
* Indicates that the shape has some properties that may contain {@link PNone#NO_VALUE} and
5458
* therefore the shape itself is not enough to resolve any lookups.
5559
*/
56-
public static final byte HAS_NO_VALUE_PROPERTIES = 4;
60+
public static final byte HAS_NO_VALUE_PROPERTIES = 0b100;
61+
/**
62+
* Indicates that the object has a dict in the form of an actual dictionary
63+
*/
64+
public static final byte HAS_MATERIALIZED_DICT = 0b1000;
5765

5866
private final Object initialPythonClass;
5967

@@ -85,6 +93,11 @@ public void setPythonClass(Object cls, DynamicObjectLibrary dylib) {
8593
dylib.put(this, CLASS, cls);
8694
}
8795

96+
public void setDict(DynamicObjectLibrary dylib, PDict dict) {
97+
dylib.setShapeFlags(this, dylib.getShapeFlags(this) | HAS_MATERIALIZED_DICT);
98+
dylib.put(this, DICT, dict);
99+
}
100+
88101
public Object getInitialPythonClass() {
89102
return initialPythonClass;
90103
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/PythonClass.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,7 @@ public void setMRO(PythonAbstractClass[] mro, PythonLanguage language) {
230230
}
231231

232232
public void setDictHiddenProp(DynamicObjectLibrary dylib, BranchProfile hasMroShapeProfile, Object value) {
233+
dylib.setShapeFlags(this, dylib.getShapeFlags(this) | HAS_MATERIALIZED_DICT);
233234
dylib.put(this, DICT, value);
234235
if (mroShapeSubTypes != null) {
235236
hasMroShapeProfile.enter();

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/TypeBuiltins.java

Lines changed: 34 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@
101101
import com.oracle.graal.python.nodes.BuiltinNames;
102102
import com.oracle.graal.python.nodes.ErrorMessages;
103103
import com.oracle.graal.python.nodes.PGuards;
104+
import com.oracle.graal.python.nodes.PNodeWithContext;
104105
import com.oracle.graal.python.nodes.PNodeWithRaise;
105106
import com.oracle.graal.python.nodes.SpecialAttributeNames;
106107
import com.oracle.graal.python.nodes.argument.positional.PositionalArgumentsNode;
@@ -113,7 +114,6 @@
113114
import com.oracle.graal.python.nodes.call.special.CallTernaryMethodNode;
114115
import com.oracle.graal.python.nodes.call.special.CallVarargsMethodNode;
115116
import com.oracle.graal.python.nodes.call.special.LookupAndCallBinaryNode;
116-
import com.oracle.graal.python.nodes.call.special.LookupAndCallTernaryNode;
117117
import com.oracle.graal.python.nodes.call.special.LookupSpecialMethodNode;
118118
import com.oracle.graal.python.nodes.classes.AbstractObjectGetBasesNode;
119119
import com.oracle.graal.python.nodes.classes.AbstractObjectIsSubclassNode;
@@ -363,11 +363,42 @@ Object selfSeparate(VirtualFrame frame, Object self, Object[] arguments, PKeywor
363363
}
364364
}
365365

366+
@ReportPolymorphism
367+
protected abstract static class BindNew extends PNodeWithContext {
368+
public abstract Object execute(VirtualFrame frame, Object descriptor, Object type);
369+
370+
@Specialization
371+
static Object doBuiltin(PBuiltinFunction descriptor, @SuppressWarnings("unused") Object type) {
372+
return descriptor;
373+
}
374+
375+
@Specialization
376+
static Object doFunction(PFunction descriptor, @SuppressWarnings("unused") Object type) {
377+
return descriptor;
378+
}
379+
380+
@Fallback
381+
static Object doBind(VirtualFrame frame, Object descriptor, Object type,
382+
@Cached GetClassNode getClassNode,
383+
@Cached(parameters = "Get") LookupCallableSlotInMRONode lookupGet,
384+
@Cached CallTernaryMethodNode callGet) {
385+
Object getMethod = lookupGet.execute(getClassNode.execute(descriptor));
386+
if (getMethod != PNone.NO_VALUE) {
387+
return callGet.execute(frame, getMethod, descriptor, PNone.NONE, type);
388+
}
389+
return descriptor;
390+
}
391+
392+
public static BindNew create() {
393+
return TypeBuiltinsFactory.BindNewNodeGen.create();
394+
}
395+
}
396+
366397
@ReportPolymorphism
367398
protected abstract static class CallNodeHelper extends PNodeWithRaise {
368399
@Child private CallVarargsMethodNode dispatchNew = CallVarargsMethodNode.create();
369-
@Child private LookupAndCallTernaryNode callNewGet = LookupAndCallTernaryNode.create(__GET__);
370400
@Child private LookupAttributeInMRONode lookupNew = LookupAttributeInMRONode.create(__NEW__);
401+
@Child private BindNew bindNew = BindNew.create();
371402
@Child private CallVarargsMethodNode dispatchInit;
372403
@Child private LookupSpecialMethodNode lookupInit;
373404
@Child private IsSubtypeNode isSubTypeNode;
@@ -378,8 +409,6 @@ protected abstract static class CallNodeHelper extends PNodeWithRaise {
378409
@CompilationFinal private ConditionProfile hasInit = ConditionProfile.createBinaryProfile();
379410
@CompilationFinal private ConditionProfile gotInitResult = ConditionProfile.createBinaryProfile();
380411

381-
@CompilationFinal private boolean newWasDescriptor = false;
382-
383412
abstract Object execute(VirtualFrame frame, Object self, Object[] args, PKeyword[] keywords, boolean doCreateArgs);
384413

385414
@Specialization(limit = "getCallSiteInlineCacheMaxDepth()", guards = {"self == cachedSelf"}, assumptions = "singleContextAssumption()")
@@ -450,17 +479,7 @@ private Object op(VirtualFrame frame, Object self, Object[] arguments, PKeyword[
450479
if (hasNew.profile(newMethod != PNone.NO_VALUE)) {
451480
CompilerAsserts.partialEvaluationConstant(doCreateArgs);
452481
Object[] newArgs = doCreateArgs ? PositionalArgumentsNode.prependArgument(self, arguments) : arguments;
453-
Object newInstance;
454-
if (!newWasDescriptor && (newMethod instanceof PFunction || newMethod instanceof PBuiltinFunction)) {
455-
newInstance = dispatchNew.execute(frame, newMethod, newArgs, keywords);
456-
} else {
457-
if (!newWasDescriptor) {
458-
CompilerDirectives.transferToInterpreterAndInvalidate();
459-
reportPolymorphicSpecialize();
460-
newWasDescriptor = true;
461-
}
462-
newInstance = dispatchNew.execute(frame, callNewGet.execute(frame, newMethod, PNone.NONE, self), newArgs, keywords);
463-
}
482+
Object newInstance = dispatchNew.execute(frame, bindNew.execute(frame, newMethod, self), newArgs, keywords);
464483

465484
// see typeobject.c#type_call()
466485
// Ugly exception: when the call was type(something),

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/object/DeleteDictNode.java

Lines changed: 30 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -40,33 +40,49 @@
4040
*/
4141
package com.oracle.graal.python.nodes.object;
4242

43+
import com.oracle.graal.python.builtins.objects.common.DynamicObjectStorage;
44+
import com.oracle.graal.python.builtins.objects.common.HashingStorage;
45+
import com.oracle.graal.python.builtins.objects.common.HashingStorageLibrary;
46+
import com.oracle.graal.python.builtins.objects.dict.PDict;
4347
import com.oracle.graal.python.builtins.objects.object.PythonObject;
44-
import com.oracle.graal.python.builtins.objects.type.PythonClass;
48+
import com.oracle.graal.python.nodes.PGuards;
4549
import com.oracle.graal.python.nodes.PNodeWithContext;
50+
import com.oracle.graal.python.runtime.object.PythonObjectFactory;
4651
import com.oracle.truffle.api.dsl.Cached;
47-
import com.oracle.truffle.api.dsl.Cached.Shared;
48-
import com.oracle.truffle.api.dsl.Fallback;
4952
import com.oracle.truffle.api.dsl.GenerateUncached;
5053
import com.oracle.truffle.api.dsl.Specialization;
5154
import com.oracle.truffle.api.library.CachedLibrary;
5255
import com.oracle.truffle.api.object.DynamicObjectLibrary;
53-
import com.oracle.truffle.api.profiles.BranchProfile;
5456

5557
@GenerateUncached
5658
public abstract class DeleteDictNode extends PNodeWithContext {
5759
public abstract void execute(PythonObject object);
5860

5961
@Specialization
60-
static void doPythonClass(PythonClass object,
61-
@Shared("dylib") @CachedLibrary(limit = "4") DynamicObjectLibrary dylib,
62-
@Cached BranchProfile hasMroShapeProfile) {
63-
object.setDictHiddenProp(dylib, hasMroShapeProfile, null);
64-
}
65-
66-
@Fallback
67-
static void doPythonObjectNotClass(PythonObject object,
68-
@Shared("dylib") @CachedLibrary(limit = "4") DynamicObjectLibrary dylib) {
69-
dylib.put(object, PythonObject.DICT, null);
62+
static void doPythonObject(PythonObject object,
63+
@CachedLibrary(limit = "4") DynamicObjectLibrary dylib,
64+
@CachedLibrary(limit = "1") HashingStorageLibrary hlib,
65+
@Cached PythonObjectFactory factory) {
66+
/* There is no special handling for class MROs because type.__dict__ cannot be deleted. */
67+
assert !PGuards.isPythonClass(object);
68+
PDict oldDict = (PDict) dylib.getOrDefault(object, PythonObject.DICT, null);
69+
if (oldDict != null) {
70+
HashingStorage storage = oldDict.getDictStorage();
71+
if (storage instanceof DynamicObjectStorage && ((DynamicObjectStorage) storage).getStore() == object) {
72+
/*
73+
* We have to dissociate the dict from this DynamicObject so that changes to it no
74+
* longer affect this object.
75+
*/
76+
oldDict.setDictStorage(hlib.copy(storage));
77+
}
78+
}
79+
/*
80+
* Ideally we would use resetShape, but that would lose all the hidden keys. Creating a new
81+
* empty dict dissociated from this object seems like the cleanest option. The disadvantage
82+
* is that the current values won't get garbage collected.
83+
*/
84+
PDict newDict = factory.createDict();
85+
object.setDict(dylib, newDict);
7086
}
7187

7288
public static DeleteDictNode create() {

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/object/GetDictIfExistsNode.java

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,13 +60,31 @@
6060
import com.oracle.truffle.api.dsl.Specialization;
6161
import com.oracle.truffle.api.library.CachedLibrary;
6262
import com.oracle.truffle.api.object.DynamicObjectLibrary;
63+
import com.oracle.truffle.api.object.Shape;
6364

6465
@GenerateUncached
6566
public abstract class GetDictIfExistsNode extends PNodeWithContext {
6667
public abstract PDict execute(Object object);
6768

6869
public abstract PDict execute(PythonObject object);
6970

71+
@Specialization(guards = {"object.getShape() == cachedShape", "hasNoDict(cachedShape)"}, limit = "1")
72+
static PDict getNoDictCachedShape(@SuppressWarnings("unused") PythonObject object,
73+
@SuppressWarnings("unused") @Cached("object.getShape()") Shape cachedShape) {
74+
assert doPythonObject(object, DynamicObjectLibrary.getUncached()) == null;
75+
return null;
76+
}
77+
78+
@Specialization(guards = "hasNoDict(object.getShape())", replaces = "getNoDictCachedShape")
79+
static PDict getNoDict(@SuppressWarnings("unused") PythonObject object) {
80+
assert doPythonObject(object, DynamicObjectLibrary.getUncached()) == null;
81+
return null;
82+
}
83+
84+
protected static boolean hasNoDict(Shape shape) {
85+
return (shape.getFlags() & PythonObject.HAS_MATERIALIZED_DICT) == 0;
86+
}
87+
7088
@Specialization(guards = {"object == cached", "dictIsConstant(cached)", "dict != null"}, assumptions = "singleContextAssumption()", limit = "1")
7189
static PDict getConstant(@SuppressWarnings("unused") PythonObject object,
7290
@SuppressWarnings("unused") @Cached(value = "object", weak = true) PythonObject cached,

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/object/SetDictNode.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ static void doPythonClass(PythonClass object, PDict dict,
6767
@Fallback
6868
static void doPythonObjectNotClass(PythonObject object, PDict dict,
6969
@Shared("dylib") @CachedLibrary(limit = "4") DynamicObjectLibrary dylib) {
70-
dylib.put(object, PythonObject.DICT, dict);
70+
object.setDict(dylib, dict);
7171
}
7272

7373
public static SetDictNode create() {

0 commit comments

Comments
 (0)