From: "Eregon (Benoit Daloze)" <noreply@...>
Date: 2022-08-18T18:41:52+00:00
Subject: [ruby-core:109560] [Ruby master Feature#18965] Further Thread::Queue improvements

Issue #18965 has been updated by Eregon (Benoit Daloze).


> Then there's the question of how SizedQueue would behave if it's not full, but still doesn't have space for all the elements. e.g.
> I think the simplest would be to wait for enough space to append the entire set, because combined with a timeout, it would be awkward if only part of the array was concatenated.

That would mean basically ignoring the timeout, not great.
And other options like returning the number of elements pushed or mutating the array don't seem too nice either.

I think for these batch push/pop, they should be motivated with benchmarks (ideally based on some real use-case) and a proof-of-concept PR.
IMHO it makes sense to add them only if there is a significant performance gain.

Also this can cause contention and e.g. starve the opposite operation, e.g. a big batch push of 1000 elements and other threads trying to pop are all blocked while pushing these elements, not good for latency/contention.

> Non blocking mode, without exception

Agreed we should add that.

The kwarg seems a bit confusing given the existing positional arg (`Queue#pop(non_block=false)`).
OTOH `nonblock` is kind of the established term for this (e.g., `read_nonblock`).
Is there any core method currently with a `nonblock` keyword argument?

Why not `exception: false` which seems more standard and established, i.e., `queue.pop(true, exception: false)`? Too verbose/inconvenient?
Some related discussion in https://bugs.ruby-lang.org/issues/18774#note-11

Could also be a new method like `pop_nonblock` (like `read_nonblock`), not sure if a good idea but putting it out there for thoughts.

----------------------------------------
Feature #18965: Further Thread::Queue improvements
https://bugs.ruby-lang.org/issues/18965#change-98731

* Author: byroot (Jean Boussier)
* Status: Open
* Priority: Normal
----------------------------------------
Following the recent addition of a `timeout` parameter to `Queue#pop`, there are a handful of other improvements I'd like to make.


### Batch insert

When using the queue for batch processing, it would be good to be able to push multiple elements at once:

Currently you have to call `push` repeatedly

```ruby
items.each do |item|
  queue.push(item)
end
```

That's wasteful because on each call we check wether the queue is closed, try to wakeup blocked threads, etc.

It would be much better if you could do:

```ruby
queue.concat(items)
```

With of course both `nonblock` and `timeout` support.

Then there's the question of how `SizedQueue` would behave if it's not full, but still doesn't have space for all the elements. e.g.
```ruby
queue = SizedQueue.new(10)
queue.concat(6.times.to_a)
queue.concat(6.times.to_a) # Block until there is 6 free slots?
```

I think the simplest would be to wait for enough space to append the entire set, because combined with a timeout, it would be awkward if only part of the array was concatenated.


### Batch pop

Similarly, sometimes the consumer of a queue is capable of batching, and right now it's not efficient:


```ruby
loop do
  items = [queue.pop]
  begin
    99.times do
      items << queue.pop(true) # true is for nonblock
    end
  rescue ThreadError # empty queue
  end
  process_items(items)
end
```

It would be much more efficient if `pop` accepted a `count` parameter:

```ruby
loop do
  items = queue.pop(count: 100)
  process_items(items)
end
```

The behavior would be:
  - Block if the queue is empty
  - If it's not empty, return **up to** `count` items (Just like `Array#pop`)


### Non blocking mode, without exception

As shown above, the current `nonblock` parameter is a bit awkward, because:

  - It raises an exception, which is very expensive for a construct often used in "low level" code.
  - The exception is `ThreadError`, so you may have to match the error message for `"queue empty"`, to make sure it doesn't come from a Mutex issue or something like that.


I believe that we could introduce a keyword argument:

```ruby
Queue.new.pop(nonblock: true) # => nil
```




-- 
https://bugs.ruby-lang.org/

Unsubscribe: <mailto:ruby-core-request@ruby-lang.org?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>