Skip to content

Commit 83d1e31

Browse files
tomasstupkatomasstupka
committed
avoid copying mro on LookupAtributeInMRONode
1 parent af6b9f7 commit 83d1e31

File tree

5 files changed

+59
-119
lines changed

5 files changed

+59
-119
lines changed

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

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@
6060
public abstract class PythonManagedClass extends PythonObject implements PythonAbstractClass {
6161
@CompilationFinal(dimensions = 1) private PythonAbstractClass[] baseClasses;
6262

63-
private final MroSequenceStorage methodResolutionOrder;
63+
@CompilationFinal private MroSequenceStorage methodResolutionOrder;
6464

6565
private final Set<PythonAbstractClass> subClasses = Collections.newSetFromMap(new WeakHashMap<PythonAbstractClass, Boolean>());
6666
private final Shape instanceShape;
@@ -70,6 +70,7 @@ public abstract class PythonManagedClass extends PythonObject implements PythonA
7070
/** {@code true} if the MRO contains a native class. */
7171
private final boolean needsNativeAllocation;
7272
@CompilationFinal private Object sulongType;
73+
@CompilationFinal private boolean mroInitialized = false;
7374

7475
@TruffleBoundary
7576
protected PythonManagedClass(PythonLanguage lang, Object typeClass, Shape classShape, Shape instanceShape, String name, PythonAbstractClass... baseClasses) {
@@ -90,9 +91,9 @@ protected PythonManagedClass(PythonLanguage lang, Object typeClass, Shape classS
9091
unsafeSetSuperClass(baseClasses);
9192
}
9293

93-
this.methodResolutionOrder.setInternalArrayObject(ComputeMroNode.doSlowPath(this, invokeMro));
94+
this.setMRO(ComputeMroNode.doSlowPath(this, invokeMro));
9495
if (invokeMro) {
95-
this.methodResolutionOrder.setInitialized();
96+
mroInitialized = true;
9697
}
9798

9899
this.needsNativeAllocation = computeNeedsNativeAllocation();
@@ -107,16 +108,20 @@ protected PythonManagedClass(PythonLanguage lang, Object typeClass, Shape classS
107108
}
108109
}
109110

111+
public boolean isMROInitialized() {
112+
return mroInitialized;
113+
}
114+
110115
/**
111116
* Invoke metaclass mro() method and set the result as new method resolution order.
112117
*/
113118
@TruffleBoundary
114119
public void invokeMro() {
115120
PythonAbstractClass[] mro = ComputeMroNode.invokeMro(this);
116121
if (mro != null) {
117-
this.methodResolutionOrder.setInternalArrayObject(mro);
122+
this.setMRO(mro);
118123
}
119-
this.methodResolutionOrder.setInitialized();
124+
mroInitialized = true;
120125
}
121126

122127
private static String getBaseName(String qname) {
@@ -152,6 +157,10 @@ PythonAbstractClass getSuperClass() {
152157
return getBaseClasses().length > 0 ? getBaseClasses()[0] : null;
153158
}
154159

160+
public void setMRO(PythonAbstractClass[] mro) {
161+
methodResolutionOrder = new MroSequenceStorage(name, mro);
162+
}
163+
155164
public MroSequenceStorage getMethodResolutionOrder() {
156165
return methodResolutionOrder;
157166
}
@@ -209,7 +218,7 @@ private void unsafeSetSuperClass(PythonAbstractClass... newBaseClasses) {
209218
this.baseClasses = newBaseClasses;
210219

211220
for (PythonAbstractClass base : getBaseClasses()) {
212-
if (base instanceof PythonManagedClass && !((PythonManagedClass) base).getMethodResolutionOrder().isInitialized()) {
221+
if (base instanceof PythonManagedClass && !((PythonManagedClass) base).mroInitialized) {
213222
throw PRaiseNode.getUncached().raise(TypeError, ErrorMessages.CANNOT_EXTEND_INCOMPLETE_P, base);
214223
}
215224
}
@@ -228,29 +237,29 @@ public void setSuperClass(PythonAbstractClass... newBaseClasses) {
228237
}
229238

230239
PythonAbstractClass[] oldBaseClasses = getBaseClasses();
231-
Object[] oldMRO = this.methodResolutionOrder.getInternalArray();
240+
PythonAbstractClass[] oldMRO = (PythonAbstractClass[]) this.methodResolutionOrder.getInternalArray();
232241

233242
Set<PythonAbstractClass> subclasses = GetSubclassesNode.getUncached().execute(this);
234243
PythonAbstractClass[] subclassesArray = subclasses.toArray(new PythonAbstractClass[subclasses.size()]);
235-
Object[][] oldSubClasssMROs = new Object[subclasses.size()][];
244+
PythonAbstractClass[][] oldSubClasssMROs = new PythonAbstractClass[subclasses.size()][];
236245
for (int i = 0; i < subclassesArray.length; i++) {
237246
PythonAbstractClass scls = subclassesArray[i];
238247
if (scls instanceof PythonManagedClass) {
239-
oldSubClasssMROs[i] = ((PythonManagedClass) scls).methodResolutionOrder.getInternalArray();
248+
oldSubClasssMROs[i] = (PythonAbstractClass[]) ((PythonManagedClass) scls).methodResolutionOrder.getInternalArray();
240249
}
241250
}
242251

243252
try {
244253
// for what follows see also typeobject.c#type_set_bases()
245254
this.baseClasses = newBaseClasses;
246-
this.methodResolutionOrder.setInternalArrayObject(ComputeMroNode.doSlowPath(this));
247255
this.methodResolutionOrder.lookupChanged();
256+
this.setMRO(ComputeMroNode.doSlowPath(this));
248257

249258
for (PythonAbstractClass scls : subclasses) {
250259
if (scls instanceof PythonManagedClass) {
251260
PythonManagedClass pmc = (PythonManagedClass) scls;
252-
pmc.methodResolutionOrder.setInternalArrayObject(ComputeMroNode.doSlowPath(scls));
253261
pmc.methodResolutionOrder.lookupChanged();
262+
pmc.setMRO(ComputeMroNode.doSlowPath(scls));
254263
}
255264
}
256265
if (this.baseClasses == newBaseClasses) {
@@ -283,15 +292,15 @@ public void setSuperClass(PythonAbstractClass... newBaseClasses) {
283292
// e.g. the mro() call might have manipulated __bases__
284293
this.baseClasses = oldBaseClasses;
285294
}
286-
this.methodResolutionOrder.setInternalArrayObject(oldMRO);
287295
this.methodResolutionOrder.lookupChanged();
296+
this.setMRO(oldMRO);
288297

289298
for (int i = 0; i < subclassesArray.length; i++) {
290299
PythonAbstractClass scls = subclassesArray[i];
291300
if (oldSubClasssMROs[i] != null) {
292301
PythonManagedClass pmc = (PythonManagedClass) scls;
293-
pmc.methodResolutionOrder.setInternalArrayObject(oldSubClasssMROs[i]);
294302
pmc.methodResolutionOrder.lookupChanged();
303+
pmc.setMRO(oldSubClasssMROs[i]);
295304
}
296305
}
297306
throw pe;

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,9 +186,8 @@ abstract static class MroAttrNode extends PythonBuiltinNode {
186186
@Specialization
187187
Object doit(Object klass,
188188
@Cached("create()") TypeNodes.GetMroNode getMroNode,
189-
@Cached TypeNodes.GetMroStorageNode getMroStorageNode,
190189
@Cached("createBinaryProfile()") ConditionProfile notInitialized) {
191-
if (notInitialized.profile(klass instanceof PythonManagedClass && !getMroStorageNode.execute(klass).isInitialized())) {
190+
if (notInitialized.profile(klass instanceof PythonManagedClass && !((PythonManagedClass) klass).isMROInitialized())) {
192191
return PNone.NONE;
193192
}
194193
PythonAbstractClass[] mro = getMroNode.execute(klass);

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -316,8 +316,8 @@ public abstract static class GetMroStorageNode extends PNodeWithContext {
316316
@Specialization
317317
static MroSequenceStorage doPythonClass(PythonManagedClass obj,
318318
@Cached("createBinaryProfile()") ConditionProfile notInitialized) {
319-
if (!notInitialized.profile(obj.getMethodResolutionOrder().isInitialized())) {
320-
obj.getMethodResolutionOrder().setInternalArrayObject(TypeNodes.ComputeMroNode.doSlowPath(obj, false));
319+
if (!notInitialized.profile(obj.isMROInitialized())) {
320+
obj.setMRO(TypeNodes.ComputeMroNode.doSlowPath(obj, false));
321321
}
322322
return obj.getMethodResolutionOrder();
323323
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/attributes/LookupAttributeInMRONode.java

Lines changed: 12 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -255,14 +255,8 @@ protected Object lookupConstantMRO(@SuppressWarnings("unused") Object klass,
255255
@Cached("mro.getLookupStableAssumption()") @SuppressWarnings("unused") Assumption lookupStable,
256256
@Cached("mro.length()") int mroLength,
257257
@Cached("create(mroLength)") ReadAttributeFromObjectNode[] readAttrNodes) {
258-
// create snapshot, mro storage can change during lookup
259-
Object[] mroArray = new Object[mroLength];
260-
for (int i = 0; i < mroArray.length; i++) {
261-
mroArray[i] = mro.getItemNormalized(i);
262-
}
263-
264-
for (int i = 0; i < mroArray.length; i++) {
265-
Object kls = mroArray[i];
258+
for (int i = 0; i < mroLength; i++) {
259+
Object kls = mro.getItemNormalized(i);
266260
if (skipPythonClasses && kls instanceof PythonClass) {
267261
continue;
268262
}
@@ -294,21 +288,16 @@ protected MroSequenceStorage getMro(Object clazz) {
294288

295289
public static Object lookupSlow(Object klass, Object key, GetMroStorageNode getMroNode, ReadAttributeFromObjectNode readAttrNode, boolean skipPythonClasses) {
296290
MroSequenceStorage mro = getMroNode.execute(klass);
297-
// create snapshot, mro storage could change during lookup
298-
Object[] mroArray = new Object[mro.length()];
299-
for (int i = 0; i < mroArray.length; i++) {
300-
mroArray[i] = mro.getItemNormalized(i);
301-
}
302-
for (int i = 0; i < mroArray.length; i++) {
303-
Object kls = mroArray[i];
304-
if (skipPythonClasses && kls instanceof PythonClass) {
305-
continue;
306-
}
307-
Object value = readAttrNode.execute(kls, key);
308-
if (value != PNone.NO_VALUE) {
309-
return value;
310-
}
311-
}
291+
for (int i = 0; i < mro.length(); i++) {
292+
Object kls = mro.getItemNormalized(i);
293+
if (skipPythonClasses && kls instanceof PythonClass) {
294+
continue;
295+
}
296+
Object value = readAttrNode.execute(kls, key);
297+
if (value != PNone.NO_VALUE) {
298+
return value;
299+
}
300+
}
312301
return PNone.NO_VALUE;
313302
}
314303

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/sequence/storage/MroSequenceStorage.java

Lines changed: 22 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -49,15 +49,13 @@
4949
import java.util.function.Predicate;
5050

5151
import com.oracle.graal.python.builtins.objects.type.PythonAbstractClass;
52-
import com.oracle.graal.python.nodes.PGuards;
5352
import com.oracle.graal.python.util.PythonUtils;
5453
import com.oracle.truffle.api.Assumption;
5554
import com.oracle.truffle.api.CompilerAsserts;
5655
import com.oracle.truffle.api.CompilerDirectives;
5756
import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
5857
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
5958
import com.oracle.truffle.api.Truffle;
60-
import com.oracle.truffle.api.profiles.ConditionProfile;
6159
import com.oracle.truffle.api.utilities.CyclicAssumption;
6260

6361
public final class MroSequenceStorage extends TypedSequenceStorage {
@@ -74,9 +72,7 @@ public final class MroSequenceStorage extends TypedSequenceStorage {
7472
*/
7573
private final Map<String, List<Assumption>> attributesInMROFinalAssumptions;
7674

77-
@CompilationFinal(dimensions = 1) private PythonAbstractClass[] values;
78-
79-
@CompilationFinal private boolean initialized = false;
75+
@CompilationFinal(dimensions = 1) private final PythonAbstractClass[] values;
8076

8177
@TruffleBoundary
8278
public MroSequenceStorage(String className, PythonAbstractClass[] elements) {
@@ -104,40 +100,24 @@ public final PythonAbstractClass getItemNormalized(int idx) {
104100
}
105101

106102
@Override
103+
@SuppressWarnings("unused")
107104
public void setItemNormalized(int idx, Object value) {
108-
if (PGuards.isPythonClass(value)) {
109-
setClassItemNormalized(idx, (PythonAbstractClass) value);
110-
} else {
111-
throw new SequenceStoreException(value);
112-
}
113-
}
114-
115-
public void setClassItemNormalized(int idx, PythonAbstractClass value) {
116-
if (values[idx] != value) {
117-
lookupChanged("direct assignment to MRO");
118-
}
119-
values[idx] = value;
105+
CompilerDirectives.transferToInterpreterAndInvalidate();
106+
throw new IllegalStateException("should not be reached");
120107
}
121108

122109
@Override
110+
@SuppressWarnings("unused")
123111
public void insertItem(int idx, Object value) {
124-
ensureCapacity(length + 1);
125-
126-
// shifting tail to the right by one slot
127-
for (int i = values.length - 1; i > idx; i--) {
128-
values[i] = values[i - 1];
129-
}
130-
131-
setItemNormalized(idx, value);
132-
length++;
112+
CompilerDirectives.transferToInterpreterAndInvalidate();
113+
throw new IllegalStateException("should not be reached");
133114
}
134115

135116
@Override
117+
@SuppressWarnings("unused")
136118
public void copyItem(int idxTo, int idxFrom) {
137-
if (idxTo != idxFrom) {
138-
lookupChanged("item move within MRO");
139-
}
140-
values[idxTo] = values[idxFrom];
119+
CompilerDirectives.transferToInterpreterAndInvalidate();
120+
throw new IllegalStateException("should not be reached");
141121
}
142122

143123
@Override
@@ -156,27 +136,6 @@ public MroSequenceStorage getSliceInBound(int start, int stop, int step, int sli
156136
return new MroSequenceStorage(getClassName(), newArray);
157137
}
158138

159-
public void setObjectSliceInBound(int start, int stop, int step, MroSequenceStorage sequence, ConditionProfile sameLengthProfile) {
160-
int otherLength = sequence.length();
161-
162-
// range is the whole sequence?
163-
if (sameLengthProfile.profile(start == 0 && stop == length && step == 1)) {
164-
values = Arrays.copyOf(sequence.values, otherLength);
165-
length = otherLength;
166-
minimizeCapacity();
167-
return;
168-
}
169-
170-
ensureCapacity(stop);
171-
172-
for (int i = start, j = 0; i < stop; i += step, j++) {
173-
values[i] = sequence.values[j];
174-
}
175-
176-
length = length > stop ? length : stop;
177-
lookupChanged("slice assignment to MRO");
178-
}
179-
180139
public String getClassName() {
181140
return className;
182141
}
@@ -200,16 +159,18 @@ public final PythonAbstractClass[] getInternalClassArray() {
200159
return values;
201160
}
202161

162+
@SuppressWarnings("unused")
203163
@Override
204164
public void increaseCapacityExactWithCopy(int newCapacity) {
205-
values = Arrays.copyOf(values, newCapacity);
206-
capacity = values.length;
165+
CompilerDirectives.transferToInterpreterAndInvalidate();
166+
throw new IllegalStateException("should not be reached");
207167
}
208168

169+
@SuppressWarnings("unused")
209170
@Override
210171
public void increaseCapacityExact(int newCapacity) {
211-
values = new PythonAbstractClass[newCapacity];
212-
capacity = values.length;
172+
CompilerDirectives.transferToInterpreterAndInvalidate();
173+
throw new IllegalStateException("should not be reached");
213174
}
214175

215176
public Object popObject() {
@@ -218,20 +179,11 @@ public Object popObject() {
218179
return pop;
219180
}
220181

182+
@SuppressWarnings("unused")
221183
@Override
222184
public void reverse() {
223-
if (length > 0) {
224-
int head = 0;
225-
int tail = length - 1;
226-
int middle = (length - 1) / 2;
227-
228-
for (; head <= middle; head++, tail--) {
229-
PythonAbstractClass temp = values[head];
230-
values[head] = values[tail];
231-
values[tail] = temp;
232-
}
233-
lookupChanged("MRO reversed");
234-
}
185+
CompilerDirectives.transferToInterpreterAndInvalidate();
186+
throw new IllegalStateException("should not be reached");
235187
}
236188

237189
@Override
@@ -255,20 +207,11 @@ public Object getCopyOfInternalArrayObject() {
255207
return Arrays.copyOf(values, length);
256208
}
257209

210+
@SuppressWarnings("unused")
258211
@Override
259212
public void setInternalArrayObject(Object arrayObject) {
260-
PythonAbstractClass[] classArray = (PythonAbstractClass[]) arrayObject;
261-
this.values = classArray;
262-
this.length = classArray.length;
263-
this.capacity = classArray.length;
264-
}
265-
266-
public boolean isInitialized() {
267-
return initialized;
268-
}
269-
270-
public boolean setInitialized() {
271-
return initialized = true;
213+
CompilerDirectives.transferToInterpreterAndInvalidate();
214+
throw new IllegalStateException("should not be reached");
272215
}
273216

274217
@Override

0 commit comments

Comments
 (0)