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.68k stars 418 forks source link

Allow overriding default min_theads/max_threads/max_queue #956

Closed c960657 closed 2 years ago

c960657 commented 2 years ago

If I specify min_threads/max_threads/max_queue in the constructor for CachedThreadPool, these options are just ignored. I don't understand why I should not be allowed to override the defaults.

pool = Concurrent::CachedThreadPool.new(min_threads: 10, max_threads: 100, max_queue: 1000)

irb(main):002:0> pool.max_length
=> 2147483647

irb(main):003:0> pool.min_length
=> 0

irb(main):004:0> pool.max_queue
=> 0
chrisseaton commented 2 years ago

Hi is there a reason you decided to close this? We're open to considering contributions.

c960657 commented 2 years ago

Hi @chrisseaton, When I saw that the JRuby tests were failing, I realised that CachedThreadPool is closely based on java.util.concurrent.Executors.newCachedThreadPool() which does not provide these options. So I gave up the idea.

jdantonio commented 2 years ago

The thread pools were actually originally designed to be 100% consistent with java.util.concurrent. I figured that the people who created those thread pools were a lot smarter than me. When I did that work originally I created thin Ruby wrappers around the java thread pools then wrote a bunch of RSpec tests that validated the documented behavior. When I created the first Ruby thread pools I built them to pass all the tests I had written against the Java ones. There's been a lot of refactoring since then but I'm pretty sure that original set of unit tests remains. The original intent was to always retain that 100% compatibility across Ruby engines so that consumers of the library didn't have to think about platform. I stepped down a long time ago so I have no opinion about how this should be handled now. I'm just popping in to give the historical context. :-)

c960657 commented 2 years ago

@jdantonio Cool, thanks :-)