[GR-17457] Avoid some additional copy steps while reading from io.

PullRequest: truffleruby/3079

Author: aardvark179

CHANGELOG.md

@@ -60,6 +60,7 @@ Performance:
 * Reduce conversion of `VALUE`s to native handle during common operations in C extensions (@aardvark179).
 * Improved performance of regex boolean matches (e.g., `Regexp#match?`) by avoiding match data allocation in TRegex (#2558, @nirvdrum).
 * Remove overhead when getting using `RDATA_PTR` (@aardvark179).
+* Additional copy operations have been reduced when performing IO (#2536, @aardvark179).
 
 Changes:
 

src/main/java/org/truffleruby/core/thread/ThreadLocalBuffer.java

@@ -17,6 +17,8 @@
 public final class ThreadLocalBuffer {
 
     public static final ThreadLocalBuffer NULL_BUFFER = new ThreadLocalBuffer(new Pointer(0, 0), null);
+    private static final long ALIGNMENT = 8L;
+    private static final long ALIGNMENT_MASK = ALIGNMENT - 1;
 
     public final Pointer start;
     long remaining;
@@ -73,7 +75,7 @@ public Pointer allocate(RubyThread thread, long size, ConditionProfile allocatio
 
         /* We ensure we allocate a non-zero number of bytes so we can track the allocation. This avoids returning null
          * or reallocating a buffer that we technically have a pointer to. */
-        final long allocationSize = Math.max(size, 4);
+        final long allocationSize = alignUp(size);
         if (allocationProfile.profile(remaining >= allocationSize)) {
             final Pointer pointer = new Pointer(cursor(), allocationSize);
             remaining -= allocationSize;
@@ -88,6 +90,10 @@ public Pointer allocate(RubyThread thread, long size, ConditionProfile allocatio
         }
     }
 
+    private static long alignUp(long size) {
+        return (size + ALIGNMENT_MASK) & ~ALIGNMENT_MASK;
+    }
+
     @TruffleBoundary
     private ThreadLocalBuffer allocateNewBlock(RubyThread thread, long size) {
         // Allocate a new buffer. Chain it if we aren't the default thread buffer, otherwise make a new default buffer.

src/main/java/org/truffleruby/extra/ffi/Pointer.java

@@ -185,6 +185,24 @@ public void readBytes(long offset, byte[] buffer, int bufferPos, int length) {
         UNSAFE.copyMemory(null, address + offset, buffer, Unsafe.ARRAY_BYTE_BASE_OFFSET + bufferPos, length);
     }
 
+    @TruffleBoundary
+    public boolean readBytesCheck8Bit(byte[] buffer, int length) {
+        assert address != 0 || length == 0;
+        assert buffer != null;
+        assert length >= 0;
+
+        long base = address;
+        boolean highBitUsed = false;
+        for (int i = 0; i < length; i++) {
+            byte aByte = UNSAFE.getByte(null, base + i);
+            if (aByte < 0) {
+                highBitUsed = true;
+            }
+            buffer[i] = aByte;
+        }
+        return highBitUsed;
+    }
+
     public short readShort(long offset) {
         assert address + offset != 0;
         return UNSAFE.getShort(address + offset);

src/main/java/org/truffleruby/extra/ffi/PointerNodes.java

@@ -31,6 +31,7 @@
 import org.truffleruby.core.rope.RopeConstants;
 import org.truffleruby.core.rope.RopeNodes;
 import org.truffleruby.core.string.RubyString;
+import org.truffleruby.core.support.RubyByteArray;
 import org.truffleruby.core.numeric.RubyBignum;
 import org.truffleruby.core.symbol.RubySymbol;
 import org.truffleruby.language.Nil;
@@ -316,6 +317,27 @@ protected RubyString readStringToNull(long address, Nil limit,
 
     }
 
+    @Primitive(name = "pointer_read_bytes_to_byte_array", lowerFixnum = { 1, 3 })
+    public abstract static class PointerReadBytesToArrayNode extends PointerPrimitiveArrayArgumentsNode {
+
+        @Specialization
+        protected Object readBytes(RubyByteArray array, int arrayOffset, long address, int length,
+                @Cached ConditionProfile zeroProfile,
+                @Cached RopeNodes.MakeLeafRopeNode makeLeafRopeNode) {
+            final Pointer ptr = new Pointer(address);
+            if (zeroProfile.profile(length == 0)) {
+                // No need to check the pointer address if we read nothing
+                return nil;
+            } else {
+                checkNull(ptr);
+                final byte[] bytes = array.bytes;
+                ptr.readBytes(0, bytes, arrayOffset, length);
+                return nil;
+            }
+        }
+
+    }
+
     @Primitive(name = "pointer_read_bytes", lowerFixnum = 1)
     public abstract static class PointerReadBytesNode extends PointerPrimitiveArrayArgumentsNode {
 
@@ -337,9 +359,10 @@ protected RubyString readBytes(long address, int length,
             } else {
                 checkNull(ptr);
                 final byte[] bytes = new byte[length];
-                ptr.readBytes(0, bytes, 0, length);
+                final boolean is8Bit = ptr.readBytesCheck8Bit(bytes, length);
                 final Rope rope = makeLeafRopeNode
-                        .executeMake(bytes, ASCIIEncoding.INSTANCE, CodeRange.CR_UNKNOWN, NotProvided.INSTANCE);
+                        .executeMake(bytes, ASCIIEncoding.INSTANCE, is8Bit ? CodeRange.CR_VALID : CodeRange.CR_7BIT,
+                                length);
                 final RubyString instance = new RubyString(
                         coreLibrary().stringClass,
                         getLanguage().stringShape,

src/main/ruby/truffleruby/core/io.rb

@@ -141,18 +141,18 @@ def fill(io, max = DEFAULT_READ_SIZE)
 
       count = Primitive.min(unused, max)
 
-      buffer = Truffle::POSIX.read_string_at_least_one_byte(io, count)
-      bytes_read = buffer ? buffer.bytesize : 0
-      if bytes_read > 0
+      total_read = 0
+      Truffle::POSIX.read_to_buffer_at_least_one_byte(io, count) do |pointer, bytes_read|
         # Detect if another thread has updated the buffer
         # and now there isn't enough room for this data.
         if bytes_read > unused
           raise RuntimeError, 'internal implementation error - IO buffer overrun'
         end
-        @storage.fill(@used, buffer, 0, bytes_read)
+        Primitive.pointer_read_bytes_to_byte_array(@storage, @used, pointer.address, bytes_read)
         @used += bytes_read
+        total_read += bytes_read
       end
-      bytes_read
+      total_read
     end
 
     # A request to the buffer to have data. The buffer decides whether
@@ -1265,9 +1265,13 @@ def read_to_separator_with_limit
     # Method G
     def read_all
       str = +''
-      until @buffer.exhausted?
-        @buffer.fill_from @io
-        str << @buffer.shift
+      unless @buffer.exhausted?
+        tmp_str = @buffer.shift
+        str << tmp_str unless tmp_str.empty?
+      end
+
+      while (tmp_str = Truffle::POSIX.read_string_at_least_one_byte(@io, InternalBuffer::DEFAULT_READ_SIZE))
+        str << tmp_str
       end
 
       yield_string(str) { |s| yield s }
@@ -1787,9 +1791,14 @@ def read(length=nil, buffer=nil)
   # If the buffer is already exhausted, returns +""+.
   private def read_all
     str = +''
-    until @ibuffer.exhausted?
-      @ibuffer.fill_from self
-      str << @ibuffer.shift
+    unless @ibuffer.exhausted?
+      if !(tmp_str = @ibuffer.shift).empty?
+        str << tmp_str
+      end
+    end
+
+    while (tmp_str = Truffle::POSIX.read_string_at_least_one_byte(self, InternalBuffer::DEFAULT_READ_SIZE))
+      str << tmp_str
     end
 
     str

src/main/ruby/truffleruby/core/posix.rb

@@ -362,6 +362,21 @@ def self.read_string_at_least_one_byte(io, count)
     end
   end
 
+  # Read up to count bytes of io to the thread-local IO buffer, and
+  # yields the buffer (a FFI::Pointer) and bytes_read
+  def self.read_to_buffer_at_least_one_byte(io, count, &block)
+    while true
+      # must call #read_to_buffer in order to properly support polyglot STDIO
+      bytes_read, errno = read_to_buffer(io, count, &block)
+      return bytes_read if errno == 0
+      if errno == EAGAIN_ERRNO
+        IO.select([io])
+      else
+        Errno.handle_errno(errno)
+      end
+    end
+  end
+
   # Used in IO#read_nonblock
 
   def self.read_string_nonblock(io, count, exception)
@@ -405,6 +420,54 @@ def self.read_string_native(io, length)
     end
   end
 
+  def self.read_to_buffer_native(io, length)
+    fd = io.fileno
+    buffer = Primitive.io_thread_buffer_allocate(length)
+    begin
+      bytes_read = Truffle::POSIX.read(fd, buffer, length)
+      if bytes_read < 0
+        bytes_read, errno = bytes_read, Errno.errno
+      elsif bytes_read == 0 # EOF
+        bytes_read, errno = 0, 0
+      else
+        bytes_read, errno = bytes_read, 0
+      end
+
+      if bytes_read < 0
+        [-1, errno]
+      elsif bytes_read == 0 # EOF
+        [0, 0]
+      else
+        yield buffer, bytes_read
+        [bytes_read, 0]
+      end
+    ensure
+      Primitive.io_thread_buffer_free(buffer)
+    end
+  end
+
+  def self.read_to_buffer_polyglot(io, length, &block)
+    fd = io.fileno
+    if fd == 0
+      buffer = Primitive.io_thread_buffer_allocate(length)
+      begin
+        read = Primitive.io_read_polyglot length
+        if read
+          bytes_read = read.bytesize
+          buffer.write_string_length(read, bytes_read)
+          yield buffer, bytes_read
+          [bytes_read, 0]
+        else
+          [0, 0]
+        end
+      ensure
+        Primitive.io_thread_buffer_free(buffer)
+      end
+    else
+      read_to_buffer_native(io, length, &block)
+    end
+  end
+
   def self.read_string_polyglot(io, length)
     fd = io.fileno
     if fd == 0
@@ -513,12 +576,14 @@ def self.write_string_nonblock_polyglot(io, string)
     if Truffle::Boot.get_option('polyglot-stdio')
       class << self
         alias_method :read_string, :read_string_polyglot
+        alias_method :read_to_buffer, :read_to_buffer_polyglot
         alias_method :write_string, :write_string_polyglot
         alias_method :write_string_nonblock, :write_string_nonblock_polyglot
       end
     else
       class << self
         alias_method :read_string, :read_string_native
+        alias_method :read_to_buffer, :read_to_buffer_native
         alias_method :write_string, :write_string_native
         alias_method :write_string_nonblock, :write_string_nonblock_native
       end