try to avoid copying the backing buffer in a read

Author: timfel

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

@@ -97,6 +97,7 @@
 import com.oracle.graal.python.runtime.exception.PException;
 import com.oracle.graal.python.runtime.exception.PythonErrorType;
 import com.oracle.graal.python.runtime.exception.PythonExitException;
+import com.oracle.graal.python.runtime.sequence.storage.ByteSequenceStorage;
 import com.oracle.truffle.api.CompilerDirectives;
 import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
 import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
@@ -548,11 +549,15 @@ int dup(int fd) {
             return getResources().dup(fd);
         }
 
-        @Specialization
-        @TruffleBoundary
-        int dup(PInt fd) {
+        @Specialization(rewriteOn = ArithmeticException.class)
+        int dupPInt(PInt fd) {
+            return getResources().dup(fd.intValueExact());
+        }
+
+        @Specialization(replaces = "dupPInt")
+        int dupOvf(PInt fd) {
             try {
-                return getResources().dup(fd.intValueExact());
+                return dupPInt(fd);
             } catch (ArithmeticException e) {
                 throw raise(OSError, "invalid fd %r", fd);
             }
@@ -663,11 +668,6 @@ Object close(int fd,
                 throw raise(OSError, "invalid fd");
             } else {
                 resources.close(fd);
-                try {
-                    channel.close();
-                } catch (IOException e) {
-                    throw raise(OSError, e.getMessage());
-                }
             }
             return PNone.NONE;
         }
@@ -789,12 +789,15 @@ public static WriteNode create() {
     @GenerateNodeFactory
     @TypeSystemReference(PythonArithmeticTypes.class)
     public abstract static class ReadNode extends PythonFileNode {
+        private static final int MAX_READ = Integer.MAX_VALUE / 2;
+
         @Specialization(guards = "readOpaque(frame)")
         Object readOpaque(@SuppressWarnings("unused") VirtualFrame frame, int fd, @SuppressWarnings("unused") Object requestedSize,
                         @Cached("createClassProfile()") ValueProfile channelClassProfile) {
             Channel channel = getResources().getFileChannel(fd, channelClassProfile);
             try {
-                return new OpaqueBytes(doRead(channel, Integer.MAX_VALUE));
+                ByteSequenceStorage bytes = doRead(channel, MAX_READ);
+                return new OpaqueBytes(Arrays.copyOf(bytes.getInternalByteArray(), bytes.length()));
             } catch (IOException e) {
                 throw raise(OSError, e.getMessage());
             }
@@ -805,46 +808,41 @@ Object read(@SuppressWarnings("unused") VirtualFrame frame, int fd, long request
                         @Cached("createClassProfile()") ValueProfile channelClassProfile) {
             Channel channel = getResources().getFileChannel(fd, channelClassProfile);
             try {
-                byte[] array = doRead(channel, (int) requestedSize);
+                ByteSequenceStorage array = doRead(channel, (int) requestedSize);
                 return factory().createBytes(array);
             } catch (IOException e) {
                 throw raise(OSError, e.getMessage());
             }
         }
 
-        @TruffleBoundary
-        private byte[] doRead(Channel channel, int requestedSize) throws IOException {
+        private ByteSequenceStorage doRead(Channel channel, int requestedSize) throws IOException {
             if (channel instanceof ReadableByteChannel) {
                 ReadableByteChannel readableChannel = (ReadableByteChannel) channel;
-                int sz;
-                if (requestedSize > (Integer.MAX_VALUE / 2)) {
-                    // common VM limit
-                    sz = Integer.MAX_VALUE / 2;
-                } else {
-                    sz = requestedSize;
-                }
-                ByteBuffer dst;
-                try {
-                    dst = ByteBuffer.allocate(sz);
-                } catch (OutOfMemoryError e) {
-                    // we just read less, that's allowed
-                    dst = ByteBuffer.allocate(8192);
-                }
-                int readSize = readableChannel.read(dst);
-                if (readSize == -1) {
-                    return new byte[0];
+                int sz = Math.min(requestedSize, MAX_READ);
+                ByteBuffer dst = allocateBuffer(sz);
+                int readSize = readIntoBuffer(readableChannel, dst);
+                if (readSize <= 0) {
+                    return new ByteSequenceStorage(0);
                 } else {
                     byte[] array = dst.array();
-                    if (array.length != readSize) {
-                        return Arrays.copyOf(array, readSize);
-                    } else {
-                        return array;
-                    }
+                    ByteSequenceStorage byteSequenceStorage = new ByteSequenceStorage(array);
+                    byteSequenceStorage.setNewLength(readSize);
+                    return byteSequenceStorage;
                 }
             }
             throw raise(OSError, "file not opened for reading");
         }
 
+        @TruffleBoundary
+        private static int readIntoBuffer(ReadableByteChannel readableChannel, ByteBuffer dst) throws IOException {
+            return readableChannel.read(dst);
+        }
+
+        @TruffleBoundary
+        private static ByteBuffer allocateBuffer(int sz) {
+            return ByteBuffer.allocate(sz);
+        }
+
         /**
          * @param frame - only used so the DSL sees this as a dynamic check
          */

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/bytes/BytesBuiltins.java

@@ -68,6 +68,7 @@
 import com.oracle.graal.python.nodes.function.builtins.PythonTernaryBuiltinNode;
 import com.oracle.graal.python.nodes.function.builtins.PythonUnaryBuiltinNode;
 import com.oracle.graal.python.runtime.exception.PythonErrorType;
+import com.oracle.graal.python.runtime.sequence.storage.ByteSequenceStorage;
 import com.oracle.graal.python.runtime.sequence.storage.SequenceStorage;
 import com.oracle.truffle.api.CompilerDirectives;
 import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
@@ -245,7 +246,7 @@ public abstract static class AddNode extends PythonBinaryBuiltinNode {
         @Specialization
         public Object add(PBytes left, PIBytesLike right,
                         @Cached("create()") SequenceStorageNodes.ConcatNode concatNode) {
-            SequenceStorage res = concatNode.execute(left.getSequenceStorage(), right.getSequenceStorage());
+            ByteSequenceStorage res = (ByteSequenceStorage) concatNode.execute(left.getSequenceStorage(), right.getSequenceStorage());
             return factory().createBytes(res);
         }
 
@@ -262,7 +263,7 @@ public abstract static class MulNode extends PythonBinaryBuiltinNode {
         @Specialization
         public Object mul(PBytes self, int times,
                         @Cached("create()") SequenceStorageNodes.RepeatNode repeatNode) {
-            SequenceStorage res = repeatNode.execute(self.getSequenceStorage(), times);
+            ByteSequenceStorage res = (ByteSequenceStorage) repeatNode.execute(self.getSequenceStorage(), times);
             return factory().createBytes(res);
         }
 

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/object/PythonObjectFactory.java

@@ -275,7 +275,7 @@ public PBytes createBytes(LazyPythonClass cls, byte[] array) {
         return trace(new PBytes(cls, array));
     }
 
-    public PBytes createBytes(SequenceStorage storage) {
+    public PBytes createBytes(ByteSequenceStorage storage) {
         return trace(new PBytes(PythonBuiltinClassType.PBytes, storage));
     }