Skip to content

mpsc: completely split bounded and unbounded and optimize unbounded receiver #1337

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

stepancheg
Copy link
Contributor

@stepancheg stepancheg commented Nov 16, 2018

On top of #1326 two commits:

The first finally splits bounded and unbounded queues.

The second replaces atomic decrement of num_messages with a local increment of UnboundedReceiver::num_sent field.

Bench before:

test unbounded_uncontended ... bench:     111,980 ns/iter (+/- 8,636)

After:

test unbounded_uncontended ... bench:     103,043 ns/iter (+/- 7,359)

(Edit: numbers after revert of atomic waker change.)

It's hard (or impossible, I tried to and failed) to do the same
optimization for the bounded queue because bounded sender needs
to atomically track message queue size. Thus the previous commit
completely splits bounded and unbounded queue implementations.

The major downside of code sharing is performance overhead (I've
noticed it in profiler) in an unbounded queue: `clone`/`drop` of
`UnboundedSender` perform unnecessary atomic increment/decrement
of `num_senders` field

The downside of this patch is the increase of LOC because of a
copy-paste of `next_message` function.

`next_message` functions could be merged if `unpark_one()` and
`dec_num_messages()` calls could be swapped. And I don't see why
`dec_num_messages()` cannot be called before `unpark_one()`.  However
the original comment by Carl Lerche says that the order is
important, so I decided to keep it as is.

rust-lang@193b53a
Mechanically inline `UnboundedInner` into `BoundedInner`
Instead have a local non-atomic `num_received` variable and considers
queue exhausted when the number of sent messages is equal to the
number of received messages.

Before:

```
test unbounded_uncontended ... bench:     111,980 ns/iter (+/- 8,636)
```

After:

```
test unbounded_uncontended ... bench:     103,043 ns/iter (+/- 7,359)
```

It's hard (or impossible, I tried to and failed) to do the same
optimization for the bounded queue, because bounded sender needs
to atomically track message queue size. Thus previous commit
completely splits bounded and unbounded queue implementations.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-channel Area: futures::channel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants