search

ruby-concurrency / concurrent-ruby

Modern concurrency tools including agents, futures, promises, thread pools, supervisors, and more. Inspired by Erlang, Clojure, Scala, Go, Java, JavaScript, and classic concurrency patterns.
https://ruby-concurrency.github.io/concurrent-ruby/
Other
5.7k stars 419 forks source link

Should RubySingleThreadExecutor be marked to be a SerialExecutorService again? #1069

Open meineerde opened 2 hours ago

meineerde commented 2 hours ago

In 455203e, the RubySingleThreadExecutor implementation was updated to use the generic RubyThreadPoolExecutor. With that change, the previously included SerialExecutorService module was removed, resulting in RubySingleThreadExecutor#serialized? now returning false.

I'm wondering if that is actually correct, given that we still have exactly one thread which executes a single task at a time which is pop'ed from the queue serially?

Shouldn't the module be included again, or more generally: shouldn't RubyThreadPoolExecutor#serialized? always return true if its max_threads is <= 1?

meineerde commented 2 hours ago

Conversely, in case I'm mistaken and RubySingleThreadExecutor is in fact not serialized?, its documentation should probably be amended to explain why.

bensheldon commented 2 hours ago

I think you're correct that SerialExecutorService shouldn't have been removed. The Java implementation does include the module: https://github.com/ruby-concurrency/concurrent-ruby/blob/56227a4c3ebdd53b8b0976eb8296ceb7a093496f/lib/concurrent-ruby/concurrent/executor/java_single_thread_executor.rb#L12

Could you make a PR for that?