[ruby-core:124512] [Ruby Feature#21821] Add Stack and SizedStack classes
Issue #21821 has been updated by Eregon (Benoit Daloze). byroot (Jean Boussier) wrote in #note-8:
About the second one, it's clever but not sure if ideal because it's no longer "atomic" on MRI, so is subject to `Thread#raise`. Looking at the `connection_pool` gem, one thing that slows it down considerably is the numerous calls to `Thread.handle_interrupt` to avoid leaking connections when `Timeout` is used.
I see https://github.com/mperham/connection_pool/blob/f3645821d02fe8de5089f31b6160... There is lots of code in checkin/checkout anyway, those `Thread.handle_interrupt` can't be removed even if SizedStack exists. Even if it's a single method call directly to a SizedStack method, that could still check interrupts, and it definitely would for `pop`, so you'd still need the `Thread.handle_interrupt`. On that topic maybe we should have all `ensure` automatically be `Thread.handle_interrupt(Object => :never)` (https://bugs.ruby-lang.org/issues/17849#note-5), or a way to opt-in per `ensure`. Also [ConnectionPool::TimedStack](https://github.com/mperham/connection_pool/blob/8ba2715edca69d6162fd4f798997...) has a bunch of code which doesn't seem to me like it could be replaced or implemented with a SizedStack (but I haven't tried). It reminds me when I once tried to replace the queue in Puma's [thread pool](https://github.com/puma/puma/blob/main/lib/puma/thread_pool.rb) with a SizedQueue to remove the extra ConditionVariable & Mutex, but it's not possible, it does things that SizedQueue can't do. IOW, I think we should have performance numbers here and implemented use cases that pass the relevant test suites, otherwise we might add a class that actually can't be used for the intended use cases. ---------------------------------------- Feature #21821: Add Stack and SizedStack classes https://bugs.ruby-lang.org/issues/21821#change-116072 * Author: byroot (Jean Boussier) * Status: Open * Target version: 4.1 ---------------------------------------- ### Context `Queue` and `SizedQueue` are very useful and performant constructs, however they only allow for FIFO queues. Some use cases do call for LIFO queues AKA stacks. For instance, in the case of connection pool, it's often preferable to use a stack. ### Feature I'd like to introduce `Stack` and `SizedStack` classes, to mirror `Queue` and `SizedQueue`. They'd have exactly the same method and behavior at the exception of dequeuing order. ### Thread namespace? Currently `Queue` and `SizedQueue` are technically defined under `Thread` and aliased in `Object`. I wonder if that's something we should do for `Stack` too, or just some historical thing we shouldn't perpetuate. -- https://bugs.ruby-lang.org/
participants (1)
-
Eregon (Benoit Daloze)