From b30a4461ab63299630836f3c3641a92004534b2a Mon Sep 17 00:00:00 2001 From: Lars Kanis Date: Mon, 20 Mar 2023 11:16:09 +0100 Subject: [PATCH 1/6] Implement IO#wait with recent changes to MRI IO#wait used to be a alias to wait_readable years ago, but the interface changed a lot. Also update the related tests from MRI. --- lib/truffle/io/wait.rb | 66 +++++++++++++++++-- src/main/c/truffleposix/truffleposix.c | 2 +- .../truffleruby/core/truffle/io_operations.rb | 8 +-- test/mri/tests/io/wait/test_io_wait.rb | 36 ++++++++-- .../tests/io/wait/test_io_wait_uncommon.rb | 3 +- 5 files changed, 100 insertions(+), 15 deletions(-) diff --git a/lib/truffle/io/wait.rb b/lib/truffle/io/wait.rb index c76a8358a74e..9f670921cf2c 100644 --- a/lib/truffle/io/wait.rb +++ b/lib/truffle/io/wait.rb @@ -14,17 +14,75 @@ def nread def ready? ensure_open_and_readable - Truffle::IOOperations.poll(self, Truffle::IOOperations::POLLIN, 0) + Truffle::IOOperations.poll(self, Truffle::IOOperations::POLLIN, 0) > 0 end def wait_readable(timeout = nil) ensure_open_and_readable - Truffle::IOOperations.poll(self, Truffle::IOOperations::POLLIN, timeout) ? self : nil + Truffle::IOOperations.poll(self, Truffle::IOOperations::POLLIN, timeout) > 0 ? self : nil end - alias_method :wait, :wait_readable def wait_writable(timeout = nil) ensure_open_and_writable - Truffle::IOOperations.poll(self, Truffle::IOOperations::POLLOUT, timeout) ? self : nil + Truffle::IOOperations.poll(self, Truffle::IOOperations::POLLOUT, timeout) > 0 ? self : nil + end + + + # call-seq: + # io.wait(events, timeout) -> event mask, false or nil + # io.wait(timeout = nil, mode = :read) -> self, true, or false + # + # Waits until the IO becomes ready for the specified events and returns the + # subset of events that become ready, or a falsy value when times out. + # + # The events can be a bit mask of +IO::READABLE+, +IO::WRITABLE+ or + # +IO::PRIORITY+. + # + # Returns a truthy value immediately when buffered data is available. + # + # Optional parameter +mode+ is one of +:read+, +:write+, or + # +:read_write+. + # + def wait(*args) + ensure_open + + if args.size != 2 || args[0].is_a?(Symbol) || args[1].is_a?(Symbol) + # Slow/messy path: + + timeout = :undef + events = 0 + args.each do |arg| + if arg.is_a?(Symbol) + events |= case arg + when :r, :read, :readable then IO::READABLE + when :w, :write, :writable then IO::WRITABLE + when :rw, :read_write, :readable_writable then IO::READABLE | IO::WRITABLE + else + raise ArgumentError, "unsupported mode: #{mode.inspect}" + end + + elsif timeout == :undef + timeout = arg + else + raise ArgumentError, 'timeout given more than once' + end + end + + timeout = nil if timeout == :undef + + events = IO::READABLE if events == 0 + + res = Truffle::IOOperations.poll(self, events, timeout) + res == 0 ? nil : self + else + # argc == 2 and neither are symbols + # This is the fast path and the new interface: + + events, timeout = *args + raise ArgumentError, 'Events must be positive integer!' if events <= 0 + res = Truffle::IOOperations.poll(self, events, timeout) + # return events as bit mask + res == 0 ? nil : res + end end end diff --git a/src/main/c/truffleposix/truffleposix.c b/src/main/c/truffleposix/truffleposix.c index 2bff24122c84..5a6702c9e7d8 100644 --- a/src/main/c/truffleposix/truffleposix.c +++ b/src/main/c/truffleposix/truffleposix.c @@ -117,7 +117,7 @@ int truffleposix_poll(int fd, int events, int timeout_ms) { fds.fd = fd; fds.events = events; - return poll(&fds, 1, timeout_ms); + return poll(&fds, 1, timeout_ms) >= 0 ? fds.revents : -1; } int truffleposix_select(int nread, int *readfds, int nwrite, int *writefds, diff --git a/src/main/ruby/truffleruby/core/truffle/io_operations.rb b/src/main/ruby/truffleruby/core/truffle/io_operations.rb index af72bdba475d..0552fb0946a3 100644 --- a/src/main/ruby/truffleruby/core/truffle/io_operations.rb +++ b/src/main/ruby/truffleruby/core/truffle/io_operations.rb @@ -205,9 +205,9 @@ def self.select(readables, readable_ios, writables, writable_ios, errorables, er end - # This method will return a true if poll returned without error - # with an event within the timeout, false if the timeout expired, - # or raises an exception for an errno. + # This method will return an event mask if poll returned without error. + # The event mask is >0 when an event occured within the timeout, 0 if the timeout expired. + # Raises an exception for an errno. def self.poll(io, event_mask, timeout) if (event_mask & POLLIN) != 0 return 1 unless io.__send__(:buffer_empty?) @@ -257,7 +257,7 @@ def self.poll(io, event_mask, timeout) Errno.handle_errno(errno) end else - primitive_result > 0 + primitive_result end end while result == :retry diff --git a/test/mri/tests/io/wait/test_io_wait.rb b/test/mri/tests/io/wait/test_io_wait.rb index 6b4722e1be0b..7997a4814f07 100644 --- a/test/mri/tests/io/wait/test_io_wait.rb +++ b/test/mri/tests/io/wait/test_io_wait.rb @@ -3,10 +3,9 @@ require 'test/unit' require 'timeout' require 'socket' -begin - require 'io/wait' -rescue LoadError -end + +# For `IO#ready?` and `IO#nread`: +require 'io/wait' class TestIOWait < Test::Unit::TestCase @@ -50,6 +49,7 @@ def test_buffered_ready? end def test_wait + omit 'unstable on MinGW' if /mingw/ =~ RUBY_PLATFORM assert_nil @r.wait(0) @w.syswrite "." sleep 0.1 @@ -161,6 +161,34 @@ def test_wait_readwrite_timeout assert_equal @w, @w.wait(0.01, :read_write) end + def test_wait_mask_writable + omit("Missing IO::WRITABLE!") unless IO.const_defined?(:WRITABLE) + assert_equal IO::WRITABLE, @w.wait(IO::WRITABLE, 0) + end + + def test_wait_mask_readable + omit("Missing IO::READABLE!") unless IO.const_defined?(:READABLE) + @w.write("Hello World\n" * 3) + assert_equal IO::READABLE, @r.wait(IO::READABLE, 0) + + @r.gets + assert_equal IO::READABLE, @r.wait(IO::READABLE, 0) + end + + def test_wait_mask_zero + omit("Missing IO::WRITABLE!") unless IO.const_defined?(:WRITABLE) + assert_raise(ArgumentError) do + @w.wait(0, 0) + end + end + + def test_wait_mask_negative + omit("Missing IO::WRITABLE!") unless IO.const_defined?(:WRITABLE) + assert_raise(ArgumentError) do + @w.wait(-6, 0) + end + end + private def fill_pipe diff --git a/test/mri/tests/io/wait/test_io_wait_uncommon.rb b/test/mri/tests/io/wait/test_io_wait_uncommon.rb index 75cd335802ba..0f922f4e24b3 100644 --- a/test/mri/tests/io/wait/test_io_wait_uncommon.rb +++ b/test/mri/tests/io/wait/test_io_wait_uncommon.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true require 'test/unit' -require 'io/wait' # test uncommon device types to check portability problems # We may optimize IO#wait_*able for non-Linux kernels in the future @@ -13,7 +12,7 @@ def test_tty_wait end def test_fifo_wait - skip 'no mkfifo' unless File.respond_to?(:mkfifo) && IO.const_defined?(:NONBLOCK) + omit 'no mkfifo' unless File.respond_to?(:mkfifo) && IO.const_defined?(:NONBLOCK) require 'tmpdir' Dir.mktmpdir('rubytest-fifo') do |dir| fifo = "#{dir}/fifo" From 04f0b60fd6efbda12affe65b119fcd9de4ceabdb Mon Sep 17 00:00:00 2001 From: Lars Kanis Date: Mon, 20 Mar 2023 11:58:27 +0100 Subject: [PATCH 2/6] Adjust test excludes, since some pass now --- test/mri/excludes/TestIOWait.rb | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/test/mri/excludes/TestIOWait.rb b/test/mri/excludes/TestIOWait.rb index 6f9f5ca58bca..0e1b25afd216 100644 --- a/test/mri/excludes/TestIOWait.rb +++ b/test/mri/excludes/TestIOWait.rb @@ -1,12 +1,4 @@ -exclude :test_nread, "needs investigation" -exclude :test_nread_buffered, "needs investigation" -exclude :test_wait_buffered, "needs investigation" -exclude :test_wait_readable_buffered, "needs investigation" -exclude :test_wait_readwrite, "needs investigation" -exclude :test_wait_readwrite_timeout, "needs investigation" -exclude :test_nread, "needs investigation" -exclude :test_nread_buffered, "needs investigation" -exclude :test_wait_buffered, "needs investigation" -exclude :test_wait_readable_buffered, "needs investigation" -exclude :test_wait_readwrite, "needs investigation" -exclude :test_wait_readwrite_timeout, "needs investigation" +exclude :test_nread, "buffered reads are not implemented" +exclude :test_nread_buffered, "buffered reads are not implemented" +exclude :test_wait_buffered, "buffered reads are not implemented" +exclude :test_wait_readable_buffered, "buffered reads are not implemented" From 8f3cfb6dfe109834e0e78e26365752c24664b4d0 Mon Sep 17 00:00:00 2001 From: Lars Kanis Date: Mon, 20 Mar 2023 12:31:07 +0100 Subject: [PATCH 3/6] Add IO#wait_priority --- lib/truffle/io/wait.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/truffle/io/wait.rb b/lib/truffle/io/wait.rb index 9f670921cf2c..47120c15b7ab 100644 --- a/lib/truffle/io/wait.rb +++ b/lib/truffle/io/wait.rb @@ -27,6 +27,11 @@ def wait_writable(timeout = nil) Truffle::IOOperations.poll(self, Truffle::IOOperations::POLLOUT, timeout) > 0 ? self : nil end + def wait_priority(timeout = nil) + ensure_open_and_readable + Truffle::IOOperations.poll(self, Truffle::IOOperations::POLLPRI, timeout) > 0 ? self : nil + end + # call-seq: # io.wait(events, timeout) -> event mask, false or nil From 134c6707337ea9b3f083b0e21a2acf1235fd48f3 Mon Sep 17 00:00:00 2001 From: Lars Kanis Date: Mon, 20 Mar 2023 14:27:34 +0100 Subject: [PATCH 4/6] Use IO constants, now that they are supported --- lib/truffle/io/wait.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/truffle/io/wait.rb b/lib/truffle/io/wait.rb index 47120c15b7ab..2d6d71f0c2be 100644 --- a/lib/truffle/io/wait.rb +++ b/lib/truffle/io/wait.rb @@ -14,22 +14,22 @@ def nread def ready? ensure_open_and_readable - Truffle::IOOperations.poll(self, Truffle::IOOperations::POLLIN, 0) > 0 + Truffle::IOOperations.poll(self, IO::READABLE, 0) > 0 end def wait_readable(timeout = nil) ensure_open_and_readable - Truffle::IOOperations.poll(self, Truffle::IOOperations::POLLIN, timeout) > 0 ? self : nil + Truffle::IOOperations.poll(self, IO::READABLE, timeout) > 0 ? self : nil end def wait_writable(timeout = nil) ensure_open_and_writable - Truffle::IOOperations.poll(self, Truffle::IOOperations::POLLOUT, timeout) > 0 ? self : nil + Truffle::IOOperations.poll(self, IO::WRITABLE, timeout) > 0 ? self : nil end def wait_priority(timeout = nil) ensure_open_and_readable - Truffle::IOOperations.poll(self, Truffle::IOOperations::POLLPRI, timeout) > 0 ? self : nil + Truffle::IOOperations.poll(self, IO::PRIORITY, timeout) > 0 ? self : nil end From 965ca286f14c4b023b0f2e316f41ae34c3eda8e9 Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Tue, 21 Mar 2023 16:41:25 +0100 Subject: [PATCH 5/6] Small cleanups --- lib/truffle/io/wait.rb | 11 +++++------ .../ruby/truffleruby/core/truffle/io_operations.rb | 2 +- test/mri/tests/io/wait/test_io_wait_uncommon.rb | 3 ++- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/truffle/io/wait.rb b/lib/truffle/io/wait.rb index 2d6d71f0c2be..13215a466ae2 100644 --- a/lib/truffle/io/wait.rb +++ b/lib/truffle/io/wait.rb @@ -1,3 +1,5 @@ +# truffleruby_primitives: true + # Copyright (c) 2017, 2023 Oracle and/or its affiliates. All rights reserved. This # code is released under a tri EPL/GPL/LGPL license. You can use it, # redistribute it and/or modify it under the terms of the: @@ -47,17 +49,15 @@ def wait_priority(timeout = nil) # # Optional parameter +mode+ is one of +:read+, +:write+, or # +:read_write+. - # def wait(*args) ensure_open - if args.size != 2 || args[0].is_a?(Symbol) || args[1].is_a?(Symbol) + if args.size != 2 || Primitive.object_kind_of?(args[0], Symbol) || Primitive.object_kind_of?(args[1], Symbol) # Slow/messy path: - timeout = :undef events = 0 args.each do |arg| - if arg.is_a?(Symbol) + if Primitive.object_kind_of?(arg, Symbol) events |= case arg when :r, :read, :readable then IO::READABLE when :w, :write, :writable then IO::WRITABLE @@ -80,9 +80,8 @@ def wait(*args) res = Truffle::IOOperations.poll(self, events, timeout) res == 0 ? nil : self else - # argc == 2 and neither are symbols + # args.size == 2 and neither are symbols # This is the fast path and the new interface: - events, timeout = *args raise ArgumentError, 'Events must be positive integer!' if events <= 0 res = Truffle::IOOperations.poll(self, events, timeout) diff --git a/src/main/ruby/truffleruby/core/truffle/io_operations.rb b/src/main/ruby/truffleruby/core/truffle/io_operations.rb index 0552fb0946a3..9eed9f7fc14d 100644 --- a/src/main/ruby/truffleruby/core/truffle/io_operations.rb +++ b/src/main/ruby/truffleruby/core/truffle/io_operations.rb @@ -206,7 +206,7 @@ def self.select(readables, readable_ios, writables, writable_ios, errorables, er end # This method will return an event mask if poll returned without error. - # The event mask is >0 when an event occured within the timeout, 0 if the timeout expired. + # The event mask is > 0 when an event occurred within the timeout, 0 if the timeout expired. # Raises an exception for an errno. def self.poll(io, event_mask, timeout) if (event_mask & POLLIN) != 0 diff --git a/test/mri/tests/io/wait/test_io_wait_uncommon.rb b/test/mri/tests/io/wait/test_io_wait_uncommon.rb index 0f922f4e24b3..75cd335802ba 100644 --- a/test/mri/tests/io/wait/test_io_wait_uncommon.rb +++ b/test/mri/tests/io/wait/test_io_wait_uncommon.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true require 'test/unit' +require 'io/wait' # test uncommon device types to check portability problems # We may optimize IO#wait_*able for non-Linux kernels in the future @@ -12,7 +13,7 @@ def test_tty_wait end def test_fifo_wait - omit 'no mkfifo' unless File.respond_to?(:mkfifo) && IO.const_defined?(:NONBLOCK) + skip 'no mkfifo' unless File.respond_to?(:mkfifo) && IO.const_defined?(:NONBLOCK) require 'tmpdir' Dir.mktmpdir('rubytest-fifo') do |dir| fifo = "#{dir}/fifo" From 272eb885200c79dbf7cace07b8a34f239620caff Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Tue, 21 Mar 2023 16:57:57 +0100 Subject: [PATCH 6/6] Add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 54a50622d864..102156442967 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -117,6 +117,7 @@ Compatibility: * Add `Refinement#import_methods` method and add deprecation warning for `Refinement#include` and `Refinement#prepend` (#2733, @horakivo). * Upgrading `UNICODE` version to 13.0.0 and `EMOJI` version to 13.1 (#2733, @horakivo). * Add `rb_io_maybe_wait_readable`, `rb_io_maybe_wait_writable` and `rb_io_maybe_wait` functions (#2733, @andrykonchin). +* Implement changes of Ruby 3.0 to `IO#wait` (#2953, @larskanis). Performance: