Expose scheduler as public interface & bug fixes.

[ioquatix]

Because socket is an extension to Ruby, we need to expose the scheduler interface to implement https://bugs.ruby-lang.org/issues/17370

[ioquatix]

By the way, I don't like all source files being in root directory. So this is the first step to improve the situation. I will also move coroutine implementation.

[nobu]

I'm curious why under "source/" too.
It doesn't feel nice to move just one file, but just confusing.
@ko1 "source/" differs from the srcdir, I think.

[ioquatix]

I am happy to move all files to source directory but it feels out of scope for this PR. So I just want to try with one file. If it’s too hard we can certainly just use the same structure as before.

[ioquatix]

I gave up trying to make windows build work with nested source path.

[nobu]

I am happy to move all files to source directory but it feels out of scope for this PR.

Agreed.

So I just want to try with one file. If it’s too hard we can certainly just use the same structure as before.

It is not good to mix a part of out-of-scope change into a different PR.

[ioquatix]

@nobu is it easier just to use

#include "ruby/fiber/scheduler.h"

in console.c?

[nobu]

As 3.0 has been released with rb_scheduler_timeout, we have to consider the compatibility to backport this change to ruby/io-console.

1. define HAVE_RUBY_FIBER_SCHEDULER_H in include/ruby.h.
   (this should be required by .github/workflows/check_misc.yml now)
2. leave console/extconf.rb as before, check the previous name.
3. in console.c:
   • if defined the header macro, include that header.

     #if defined HAVE_RUBY_FIBER_SCHEDULER_H
     # include "ruby/fiber/scheduler.h"
     

   • otherwise, if the previous name is found, define the new name to it.

     #elif defined HAVE_RB_SCHEDULER_TIMEOUT
     extern VALUE rb_scheduler_timeout(struct timeval *timeout);
     # define rb_fiber_scheduler_timeout rb_scheduler_timeout

4. now you can use the new name in console_getch.

[ioquatix]

As @nobu said, rb_scheduler_timeout was leaked into public interface of Ruby 3 because it was used in the IO extension. So, we need to maintain backwards compatibility. This PR renames rb_scheduler to rb_fiber_scheduler so we must introduce compatibility patch for Ruby 3.0 as he suggested if we want to proceed with this rename.

The C interface should be exposed because C extensions need it. I'd prefer if we don't spread all those hooks across different extensions using rb_funcall. You assume that the implementation may not change, but long term that may not be true as we develop the interface. scheduler.h serves as both an interface and documentation.

[ioquatix]

@ko1 said:

how to use C-APIs for socket? (small example is enough to explain why it is needed)

VALUE io = ...;
VALUE scheduler = rb_fiber_scheduler_current();

rb_fiber_scheduler_io_read(scheduler, io, ...);

I don't advise this because user should prefer IO#read, but it is true that it's less efficient.

@eregon I thought about it and I don't see why we should hide this C interface, it seems arbitrary and makes maintenance more burdensome. Another option is to have two headers, one for internal and one for C extensions, but it seems super arbitrary, so I'm against any changes/limitations in this regard and am happy with this PR.

[eregon]

Kernel#sleep always take a single argument, is there any reason to expose this variadic variant, in addition to the one above?

[eregon]

The C interface should be exposed because C extensions need it. I'd prefer if we don't spread all those hooks across different extensions using rb_funcall. You assume that the implementation may not change, but long term that may not be true as we develop the interface. scheduler.h serves as both an interface and documentation.

No they don't need it, the only thing that probably needs to be exposed for convenience/efficiency is rb_fiber_scheduler_current().
How are this new C API functions any more flexible?

If the number of arguments are changed for the scheduler, it's going to break compatibility with existing schedulers anyway.
I guess the only real advantage of the C API is doing some of conversions between Ruby and C types (like SIZET2NUM(), but that seems very minor, and C extensions already have to do such conversions all the time).
Regarding the interface, I think it's better defined in docs, and that applies to both Ruby and C code, than some C function prototypes.

In fact I'd argue it's less flexible because it slows down scheduler API evolution, i.e., with these new C APIs, C extensions will probably consider they can only use exposed functions, but if it was all rb_check_funcall() then C extensions/gems could just check if the scheduler supports a given hook, regardless of the Ruby version, which seems more flexible.

Anyway, if you really want to add these new C APIs I don't strongly object (although I think it's not worth it and I think it's less flexible), but then please write specs or tests for it.

update

[ioquatix]

they can only use exposed functions, but if it was all rb_check_funcall() then C extensions/gems could just check if the scheduler supports a given hook, regardless of the Ruby version, which seems more flexible.

I see this as an advantage in terms of stability which is pretty important. If we have extensions making their own decisions about data types etc it could be a nightmare to support/test.

Anyway, if you really want to add these new C APIs I don't strongly object (although I think it's not worth it and I think it's less flexible), but then please write specs or tests for it.

Ok.

[ioquatix]

I understand there is some contention around exposing C interface. I do understand both sides. However, I'd like to keep this PR a bit simple in terms of "public interface" for scheduler is exposed. If we decided it is too much, we can revert before 3.1 is released.