concurrent-ruby
concurrent-ruby copied to clipboard
Remove all global state not just configuration
Removes dependency on global state from all classes. Introduce World
which can have default executors, which are injected when constructors on the world are used. Or instead of world it can be just a module with overridable methods like default_io_executor
.
This would be a wonderful enhancement :+1:
The reasons for removal explained in https://github.com/ruby-concurrency/concurrent-ruby/issues/585#issuecomment-345553980. The removal of global pools will not make the c-r harder to use. The pre-configured pools and any other current or future configuration will live in a Concurrent::World
object. When implemented, to get the current easy to use behavior with world object the user just needs to do CONCURRENT = Concurrent::World.new
and then use abstractions with factory methods, e.g. CONCURRENT.future
or CONCURRENT.agent
or inject the world into the constructor Concurrent::Future.new a_world, ...
. A mild disadvantage is that there will be one more step, creation of the world, but the advantages are that libraries or projects can easily live in one process with differently configured worlds. It is also important for testing to be able to create isolated worlds.
I still don't understand the rationale, and I think this does make c-r harder to use, although possibly only slightly. But there are additional steps.
And these additional steps make it really hard to encapsulate concurrent use in dependencies. Right now, I can supply a gem (eg) that does some things with (say) concurrent futures on the global pool. And you can use it in your app, and it just works without knowing the details.
Under proposed change, I could not do such a thing, my dependency would necessarily have to supply configuration telling it which pools to use for it's futures. Either as a variable/constant I should call eg .future
off of (your CONCURRENT.future
example), or just as a pool I should pass in your Concurrent::Future.new a_world
example. (And if the docs don't tell me which is preferred, I have to spend time understanding the options and choosing one too!).
I think this is counter to the goal of c-r providing a common denominator lingua franca for concurrency, so different dependencies can easily work together using concurrency.
@mathewd has said he also seems problems to this for using c-r in Rails, and it would possibly encourage him to stop using c-r in Rails and write rails-specific concurrency primitives instead. Which would obviously also harm the goal of c-r as a concurrency common denominator in railsland.
I do not understand the basis for the proposal, what actually existing problems it's meant to solve. Can you be clear about what actual problems the current architecture is causing?
@jrochkind I think that the goals of this change have been thoroughly explained. Two gems can try to change the global pools in ways that make it incompatible with one another and will make code break in unexpected ways. It might make it (slightly) "harder to use" , but it'll be harder to break.
The maintainer is also willing to keep the old behavior, provides you activate it explicitly. Which can suit your immediate needs.
If you're building a gem, you can always make the pool an explicit argument (power to the user) and provide your default one. And the end user will be able to choose between knowing the details or provide his desired pool. And there'll be less unexpected resource sharing.
Did you understand my point about dependencies needing their pools configured instead of just using the default global ones?
Do we know of actual cases in the wild with current global pools (I'm talking about what's currently released) where "two gems can try to change the global pools in/ ways that make it incompatible with one another and make code break in unexpected ways"? Is this an actually existing problem or just a hypothetical one, a solution in search of a problem? Is the cure worse than the disease? I have never encountered this problem in practice, but the "fix" will make things harder for me. I think many others are in similar situations.
I do not know of a case where two gems both try to modify the global thread pools. As far as I know this is a hypothetical problem.
I have always believed that the greatest value of this gem is that it makes concurrency easy for most Rubyists. Global thread pools facilitate this. I never wanted this gem to look and feel like Java, where it takes 20 lines of setup code for 2 lines of actual work. By allowing most of ouranstractions to receive an optional executor argument we provide significant customization without affecting the basic use cases.
I do not believe that gems should modify the global thread pools. I consider that an improper use of this gem. My preference would be to remove all configuration of the global thread pools and require gems to expose a custom executor for their users to use.
As I am no longer the maintainer I will support whatever decision @pitr-ch makes.
Did you understand my point about dependencies needing their pools configured instead of just using the default global ones?
Yes, I can tell you that, as a user of c-r in my gems and use cases, I always configure my pools, and I never use the global ones. My reasons for it is not being willing to share the units of concurrency with other functionality.
Do we know of actual cases in the wild with current global pools...
I'm just a user of this gem, don't know of many gems that use it under the hood, and I wouldn't presume to know more than the maintainer of the gem in that department. But he did propose the change, so he must have his reasons.
He did propose the World configuration. Shouldn't that cover your concerns?
I am also a user of c-r, as well as someone who has often evangelized it on reddit and in my blog. I have also contributed documentation to c-r in the past.
Sharing a global thread pool often works out fine for me. The global thread pools are configured properly to be shareable for many use cases if used properly. Of course, configuring a custom thread pool is also always possible, and I do that sometimes too.
The global thread pools make it MUCH easier to add in concurrency in a "light" way. You just need a couple async tasks, just create a couple futures or promises with the global/default pools. Working toward the goals of making concurrency as easy to use in ruby as in other languages/platforms.
If the problem that's being solved is a gem trying to configure the global thread pool inappropriately, I would potentially support removing any api for configuring the global thread pools, as @jdantonio suggests. (and I agree that a gem doing this would be improper use of c-r, although I'm not totally certain there aren't any use cases where it would be reasonable for an app to do it.) Although if this is just a hypothetical problem that has not actually been a bother, even that seems premature engineering -- let alone entirely refactoring the gem for this hypothetical threat.
I would also for sure support additional API to make it easier to use a custom pool, if that seems neccesary. Perhaps the MY_THING.future
API @pitr-ch supports is a good idea in addition to the default global pools, I don't totally understand that but it seems possibly reasonable.
Sure, @pitr-ch presumably has his reasons. I do not understand them, which is why I'm asking here, on the ticket I think @pitr-ch invited such discussion. Again, if the only reason is the hypothetical of gems improperly configuring the global pools, the less intrusive fix would seem to be removing API for mutating the global pools.
I don't think the 'world' idea is neccesarily bad, I just think it should be an option, and there should still be an option to use the same API you use now to use a default global automatically lazily-existing and automatically at_exit safe-shutdown-ing 'world'. With no backwards incompat changes to API to do that. An optional use of separate 'worlds' with convenient API could be a fine enhancement.
Thank you both for sharing your thoughts! After thinking it over, @jrochkind you are right that it would complicates things a bit for casual use, which we want to support as well as more involved usages. Therefore refining the original thought:
a_world = Concurrent::World.new # has pools configured as current global pools
a_world.future { ... }
Concurrent::Future.execute(world: a_world) { ... }
But also a common world should be already provided by c-r for casual use.
CONCURRENT.future { ... }
This very fuzzy draft and will have to be revisited.
1.x versions will never drop the global pool to keep compatibility, it will only deprecate the global configuration.
Thanks @pitr-ch. I'm down with not allowing configuration of the default global pools. If you want to configure, you've got to use your own pools. I don't see what the world:
parameter gets you over the current executor:
or perhaps pool:
parameter. A "world" sounds like it contains more than just a pool, but what about the 'world' do you need to tell a future (or other thing needing a pool) except it's executor pool? I like the simplicity and understandability of giving it a single executor.