Skip to content

Commit 59e612c

Browse files
fangererfangerer
committed
[GR-34651] Fix: Do not assume storage type of bytes object.
PullRequest: graalpython/2008
2 parents 150d263 + 0ab21b7 commit 59e612c

File tree

8 files changed

+87
-126
lines changed

8 files changed

+87
-126
lines changed

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/io/BytesIOBuiltins.java

Lines changed: 31 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,11 @@
9696
import com.oracle.graal.python.builtins.objects.bytes.BytesBuiltins;
9797
import com.oracle.graal.python.builtins.objects.bytes.PBytes;
9898
import com.oracle.graal.python.builtins.objects.common.HashingStorageLibrary;
99-
import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes;
99+
import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes.EnsureCapacityNode;
100+
import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes.GetInternalArrayNode;
101+
import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes.GetInternalByteArrayNode;
102+
import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes.GetInternalObjectArrayNode;
103+
import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes.SetLenNode;
100104
import com.oracle.graal.python.builtins.objects.dict.PDict;
101105
import com.oracle.graal.python.builtins.objects.tuple.PTuple;
102106
import com.oracle.graal.python.lib.PyIndexCheckNode;
@@ -204,7 +208,7 @@ Object exportsError(PBytesIO self, Object size) {
204208
}
205209

206210
static PBytes readBytes(PBytesIO self, int size,
207-
SequenceStorageNodes.GetInternalByteArrayNode getBytes,
211+
GetInternalByteArrayNode getBytes,
208212
PythonObjectFactory factory) {
209213
if (size == 0) {
210214
return factory.createBytes(PythonUtils.EMPTY_BYTE_ARRAY);
@@ -235,7 +239,7 @@ protected ArgumentClinicProvider getArgumentClinic() {
235239

236240
@Specialization(guards = "self.hasBuf()")
237241
Object read(PBytesIO self, int len,
238-
@Cached SequenceStorageNodes.GetInternalByteArrayNode getBytes) {
242+
@Cached GetInternalByteArrayNode getBytes) {
239243
int size = len;
240244
/* adjust invalid sizes */
241245
int n = self.getStringSize() - self.getPos();
@@ -261,7 +265,7 @@ protected ArgumentClinicProvider getArgumentClinic() {
261265
}
262266

263267
static int scanEOL(PBytesIO self, int l,
264-
SequenceStorageNodes.GetInternalByteArrayNode getBytes) {
268+
GetInternalByteArrayNode getBytes) {
265269
assert (self.hasBuf());
266270
assert (self.getPos() >= 0);
267271

@@ -308,7 +312,7 @@ protected ArgumentClinicProvider getArgumentClinic() {
308312

309313
@Specialization(guards = "self.hasBuf()")
310314
Object readline(PBytesIO self, int size,
311-
@Cached SequenceStorageNodes.GetInternalByteArrayNode getBytes) {
315+
@Cached GetInternalByteArrayNode getBytes) {
312316
int n = scanEOL(self, size, getBytes);
313317
return readBytes(self, n, getBytes, factory());
314318
}
@@ -326,7 +330,7 @@ protected ArgumentClinicProvider getArgumentClinic() {
326330

327331
@Specialization(guards = "self.hasBuf()")
328332
Object readlines(PBytesIO self, int maxsize,
329-
@Cached SequenceStorageNodes.GetInternalByteArrayNode getBytes) {
333+
@Cached GetInternalByteArrayNode getBytes) {
330334
ArrayBuilder<Object> result = new ArrayBuilder<>();
331335

332336
int n;
@@ -396,15 +400,15 @@ protected ArgumentClinicProvider getArgumentClinic() {
396400

397401
@Specialization(guards = {"self.hasBuf()", "checkExports(self)"})
398402
Object truncate(PBytesIO self, @SuppressWarnings("unused") PNone size,
399-
@Shared("i") @Cached SequenceStorageNodes.GetInternalArrayNode internalArray,
400-
@Shared("l") @Cached SequenceStorageNodes.SetLenNode setLenNode) {
403+
@Shared("i") @Cached GetInternalArrayNode internalArray,
404+
@Shared("l") @Cached SetLenNode setLenNode) {
401405
return truncate(self, self.getPos(), internalArray, setLenNode);
402406
}
403407

404408
@Specialization(guards = {"self.hasBuf()", "checkExports(self)", "size >= 0", "size < self.getStringSize()"})
405409
Object truncate(PBytesIO self, int size,
406-
@Shared("i") @Cached SequenceStorageNodes.GetInternalArrayNode internalArray,
407-
@Shared("l") @Cached SequenceStorageNodes.SetLenNode setLenNode) {
410+
@Shared("i") @Cached GetInternalArrayNode internalArray,
411+
@Shared("l") @Cached SetLenNode setLenNode) {
408412
self.setStringSize(size);
409413
resizeBuffer(self, size, internalArray, factory());
410414
setLenNode.execute(self.getBuf().getSequenceStorage(), size);
@@ -420,8 +424,8 @@ static Object same(@SuppressWarnings("unused") PBytesIO self, int size) {
420424
Object obj(VirtualFrame frame, PBytesIO self, Object arg,
421425
@Cached PyNumberAsSizeNode asSizeNode,
422426
@Cached PyNumberIndexNode indexNode,
423-
@Shared("i") @Cached SequenceStorageNodes.GetInternalArrayNode internalArray,
424-
@Shared("l") @Cached SequenceStorageNodes.SetLenNode setLenNode) {
427+
@Shared("i") @Cached GetInternalArrayNode internalArray,
428+
@Shared("l") @Cached SetLenNode setLenNode) {
425429
int size = asSizeNode.executeExact(frame, indexNode.execute(frame, arg), OverflowError);
426430
if (size >= 0) {
427431
if (size < self.getStringSize()) {
@@ -459,14 +463,14 @@ protected static void unshareBuffer(PBytesIO self, int size, byte[] buf,
459463
}
460464

461465
protected static void unshareBuffer(PBytesIO self, int size,
462-
SequenceStorageNodes.GetInternalArrayNode internalArray,
466+
GetInternalArrayNode internalArray,
463467
PythonObjectFactory factory) {
464468
byte[] buf = (byte[]) internalArray.execute(self.getBuf().getSequenceStorage());
465469
unshareBuffer(self, size, buf, factory);
466470
}
467471

468472
protected static void resizeBuffer(PBytesIO self, int size,
469-
SequenceStorageNodes.GetInternalArrayNode internalArray,
473+
GetInternalArrayNode internalArray,
470474
PythonObjectFactory factory) {
471475
int alloc = self.getStringSize();
472476
if (size < alloc) {
@@ -486,9 +490,9 @@ abstract static class WriteNode extends ClosedCheckPythonBinaryBuiltinNode {
486490
Object doWrite(PBytesIO self, Object b,
487491
@CachedLibrary("b") PythonBufferAcquireLibrary acquireLib,
488492
@CachedLibrary(limit = "2") PythonBufferAccessLibrary bufferLib,
489-
@Cached SequenceStorageNodes.GetInternalArrayNode internalArray,
490-
@Cached SequenceStorageNodes.EnsureCapacityNode ensureCapacityNode,
491-
@Cached SequenceStorageNodes.SetLenNode setLenNode) {
493+
@Cached GetInternalArrayNode internalArray,
494+
@Cached EnsureCapacityNode ensureCapacityNode,
495+
@Cached SetLenNode setLenNode) {
492496
Object buffer = acquireLib.acquireReadonly(b);
493497
try {
494498
int len = bufferLib.getBufferLength(buffer);
@@ -661,7 +665,7 @@ Object closedError(PBytesIO self, int pos, int whence) {
661665
abstract static class GetBufferNode extends ClosedCheckPythonUnaryBuiltinNode {
662666
@Specialization(guards = "self.hasBuf()")
663667
Object doit(VirtualFrame frame, PBytesIO self,
664-
@Cached SequenceStorageNodes.GetInternalArrayNode internalArray,
668+
@Cached GetInternalArrayNode internalArray,
665669
@Cached PyMemoryViewFromObject memoryViewNode) {
666670
// if (SHARED_BUF(b))
667671
unshareBuffer(self, self.getStringSize(), internalArray, factory());
@@ -681,24 +685,25 @@ protected static boolean shouldCopy(PBytesIO self) {
681685
}
682686

683687
protected static boolean shouldUnshare(PBytesIO self) {
684-
return self.getStringSize() != self.getBufCapacity();
688+
int capacity = self.getBuf().getSequenceStorage().getCapacity();
689+
return self.getStringSize() != capacity;
685690
}
686691

687692
@Specialization(guards = {"self.hasBuf()", "shouldCopy(self)"})
688-
Object copy(PBytesIO self,
689-
@Cached SequenceStorageNodes.GetInternalByteArrayNode getBytes) {
693+
Object doCopy(PBytesIO self,
694+
@Cached GetInternalByteArrayNode getBytes) {
690695
byte[] buf = getBytes.execute(self.getBuf().getSequenceStorage());
691696
return factory().createBytes(PythonUtils.arrayCopyOf(buf, self.getStringSize()));
692697
}
693698

694699
@Specialization(guards = {"self.hasBuf()", "!shouldCopy(self)", "!shouldUnshare(self)"})
695-
static Object doit(PBytesIO self) {
700+
static Object doShare(PBytesIO self) {
696701
return self.getBuf();
697702
}
698703

699704
@Specialization(guards = {"self.hasBuf()", "!shouldCopy(self)", "shouldUnshare(self)"})
700-
Object unshare(PBytesIO self,
701-
@Cached SequenceStorageNodes.GetInternalArrayNode internalArray) {
705+
Object doUnshare(PBytesIO self,
706+
@Cached GetInternalArrayNode internalArray) {
702707
// if (SHARED_BUF(self))
703708
unshareBuffer(self, self.getStringSize(), internalArray, factory());
704709
// else resize self.buf
@@ -724,7 +729,7 @@ Object doit(VirtualFrame frame, PBytesIO self,
724729
abstract static class SetStateNode extends PythonBinaryBuiltinNode {
725730
@Specialization(guards = "checkExports(self)")
726731
Object doit(VirtualFrame frame, PBytesIO self, PTuple state,
727-
@Cached SequenceStorageNodes.GetInternalObjectArrayNode getArray,
732+
@Cached GetInternalObjectArrayNode getArray,
728733
@Cached WriteNode writeNode,
729734
@Cached PyIndexCheckNode indexCheckNode,
730735
@Cached PyNumberAsSizeNode asSizeNode,
@@ -867,7 +872,7 @@ abstract static class IternextNode extends ClosedCheckPythonUnaryBuiltinNode {
867872

868873
@Specialization(guards = "self.hasBuf()")
869874
Object doit(PBytesIO self,
870-
@Cached SequenceStorageNodes.GetInternalByteArrayNode getBytes) {
875+
@Cached GetInternalByteArrayNode getBytes) {
871876
int n = scanEOL(self, -1, getBytes);
872877
if (n == 0) {
873878
throw raise(StopIteration);

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/io/PBytesIO.java

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,9 @@
4343
import com.oracle.graal.python.builtins.objects.bytes.PBytes;
4444
import com.oracle.graal.python.builtins.objects.memoryview.BufferLifecycleManager;
4545
import com.oracle.graal.python.builtins.objects.object.PythonBuiltinObject;
46-
import com.oracle.graal.python.runtime.sequence.storage.BasicSequenceStorage;
4746
import com.oracle.truffle.api.object.Shape;
4847

49-
public class PBytesIO extends PythonBuiltinObject {
48+
public final class PBytesIO extends PythonBuiltinObject {
5049
private PBytes buf;
5150
private int pos;
5251
private int stringSize;
@@ -89,11 +88,6 @@ public void setStringSize(int size) {
8988
this.stringSize = size;
9089
}
9190

92-
public int getBufCapacity() {
93-
// Casting is safe as we only create/replace buf internally.
94-
return ((BasicSequenceStorage) buf.getSequenceStorage()).capacity();
95-
}
96-
9791
public int getExports() {
9892
return exports.getExports().get();
9993
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2839,7 +2839,7 @@ static SequenceStorage doManaged(BasicSequenceStorage s, Object val, GenNodeSupp
28392839
@Shared("genNode") @Cached DoGeneralizationNode doGenNode) {
28402840
int len = lenNode.execute(s);
28412841
int newLen = len + 1;
2842-
int capacity = s.capacity();
2842+
int capacity = s.getCapacity();
28432843
if (newLen > capacity) {
28442844
increaseCapacity.enter();
28452845
ensureCapacity.execute(s, len + 1);

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/literal/ListLiteralNode.java

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ private int updateFrom(int newSizeEstimate) {
7676
}
7777
}
7878

79-
@CompilationFinal private SizeEstimate initialCapacity;
79+
private final SizeEstimate initialCapacity;
8080
private final boolean hasStarredExpressions;
8181

8282
public ListLiteralNode(ExpressionNode[] values) {
@@ -136,18 +136,14 @@ private SequenceStorageNodes.AppendNode ensureAppendNode() {
136136
public void reportUpdatedCapacity(BasicSequenceStorage newStore) {
137137
if (CompilerDirectives.inInterpreter()) {
138138
if (PythonContext.get(this).getOption(PythonOptions.OverallocateLiteralLists)) {
139-
if (newStore.capacity() > initialCapacity.estimate()) {
140-
initialCapacity.updateFrom(newStore.capacity());
141-
LOGGER.finest(() -> {
142-
return String.format("Updating list size estimate at %s. Observed capacity: %d, new estimate: %d", getSourceSection().toString(), newStore.capacity(),
143-
initialCapacity.estimate());
144-
});
139+
if (newStore.getCapacity() > initialCapacity.estimate()) {
140+
initialCapacity.updateFrom(newStore.getCapacity());
141+
LOGGER.finest(() -> String.format("Updating list size estimate at %s. Observed capacity: %d, new estimate: %d", getSourceSection().toString(), newStore.getCapacity(),
142+
initialCapacity.estimate()));
145143
}
146144
if (newStore.getElementType().generalizesFrom(type)) {
147145
type = newStore.getElementType();
148-
LOGGER.finest(() -> {
149-
return String.format("Updating list type estimate at %s. New type: %s", getSourceSection().toString(), type.name());
150-
});
146+
LOGGER.finest(() -> String.format("Updating list type estimate at %s. New type: %s", getSourceSection().toString(), type.name()));
151147
}
152148
}
153149
}

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

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -29,30 +29,6 @@
2929

3030
public abstract class BasicSequenceStorage extends SequenceStorage {
3131

32-
// nominated storage length
33-
protected int length;
34-
35-
// physical storage length
36-
protected int capacity;
37-
38-
@Override
39-
public final int length() {
40-
return length;
41-
}
42-
43-
@Override
44-
public void setNewLength(int length) {
45-
this.length = length;
46-
}
47-
48-
protected final void incLength() {
49-
this.length++;
50-
}
51-
52-
public final int capacity() {
53-
return capacity;
54-
}
55-
5632
public abstract Object getCopyOfInternalArrayObject();
5733

5834
public abstract void setInternalArrayObject(Object arrayObject);

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

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@
3535
public final class EmptySequenceStorage extends SequenceStorage {
3636

3737
public static final EmptySequenceStorage INSTANCE = new EmptySequenceStorage();
38-
private static final Object[] EMPTY_ARRAY = new Object[0];
3938

4039
@Override
4140
public SequenceStorage generalizeFor(Object value, SequenceStorage target) {
@@ -71,11 +70,6 @@ public Object getIndicativeValue() {
7170
return null;
7271
}
7372

74-
@Override
75-
public int length() {
76-
return 0;
77-
}
78-
7973
@Override
8074
public void setNewLength(int length) {
8175
if (length != 0) {
@@ -96,12 +90,12 @@ public SequenceStorage createEmpty(int newCapacity) {
9690

9791
@Override
9892
public Object[] getInternalArray() {
99-
return EMPTY_ARRAY;
93+
return PythonUtils.EMPTY_OBJECT_ARRAY;
10094
}
10195

10296
@Override
10397
public Object[] getCopyOfInternalArray() {
104-
return EMPTY_ARRAY;
98+
return PythonUtils.EMPTY_OBJECT_ARRAY;
10599
}
106100

107101
@Override

0 commit comments

Comments
 (0)