Skip to content
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

Yielding in crossbeam-channel #366

Open
ghost opened this issue Apr 27, 2019 · 5 comments · May be fixed by #1105
Open

Yielding in crossbeam-channel #366

ghost opened this issue Apr 27, 2019 · 5 comments · May be fixed by #1105

Comments

@ghost
Copy link

ghost commented Apr 27, 2019

Hey @papertigers, I just saw your message on IRC, which I am repeating here:

Have you done any testing with sending values at a decently high throughput?

I noticed that sending values relatively quickly through a crossbeam channel results in a lot yields and increased CPU usage compared to std::sync::mpsc

https://play.rust-lang.org/?version=stable&mode=debug&edition=2015&gist=3677d9c50b85252170dc410137d489dd

is a small example that shows what I am talking about

https://gist.github.com/papertigers/0bd33bc0f1241463dcee35763337c579

Thanks for the report! I think it'd be easier to chat either over on GitHub or on our discord channel since we have moved off IRC.

My guess what is going on here is that in std::sync::mpsc, Sender in std::sync::mpsc is slower than its corresponding Receiver so the messages build up and nobody ever has to block.

On the other hand, it may be that in crossbeam-channel, Sender is slower than Receiver so sometimes the Receiver has to block, which is where those yields come from.

Could you perhaps verify if my hypothesis is true?

@ghost
Copy link
Author

ghost commented May 8, 2019

An idea that might alleviate the problem: if backoff.is_completed() is true in push/pop retry loops, we should probably register the sender/receiver and park the thread so that the operation we're blocked on wakes us up.

@papertigers
Copy link

For the purposes of this issue its worth noting that I spoke with @stjepang over discord about the situation.

Your suggestion might be worth exploring. For now I was able to alleviate the problem by attempting to coalesce things coming through the channel:

      loop {
          match sel.ready() {
              i if i == events_ready => {
                  // attempt to wait for more events to enable batch processing and cut down on yields
                  thread::sleep(std::time::Duration::from_nanos(500_000));
                  // use events_ready
              }
              i if i == shutdown_ready => {
                  shutdown.recv()
                  break;
              }
              _ => unreachable!(),
          }
      }

The sleep is not the best solution here but currently is enough to allow the thread to batch things together. It may be better to try something in the backoff module like a short spin lock etc. Really the API that I am trying to mimic here is recv_max_timeout(n: usize, t: Duration). Which ideally receives up to the max specified events or times out before then returning whatever was received in attempt to wait for other things coming down the channel if that makes sense.

@gterzian
Copy link

gterzian commented Nov 7, 2019

I'm thinking the maybe the problem is the call to slot.wait_write() inside read, at

that then uses a backoff without ever blocking if it's completed, at

Also note that this would occur in the context of this

where the "outer" backoff is checked for completeness and the thread blocks, if necessary.

So it appears to that the "inner backoff", the one used inside wait_write, could potentially defeat the completeness check of the outer backoff, the one used in read, with the reading thread stuck in the below loop and calling ::std::thread::yield_now(), while the writing thread is slow to write the message in the block?

while self.state.load(Ordering::Acquire) & WRITE == 0 {

I guess the solution could be rewriting the read logic so that it is better plugged into the outer loop and potentially blocks when the backoff completes.

I can suggest the following:

  • Have read return an Option<T>, with None indicating the slot is not ready to read from yet.
  • Move the token.list.block.is_null() check to outside of read, so that an error can be returned directly by recv.
  • Replace Slot.wait_write with a has_been_written(&self) -> bool method, called by read and used to return None if false.

I'll give it a shot actually...

@gterzian
Copy link

gterzian commented Nov 7, 2019

Really the API that I am trying to mimic here is recv_max_timeout(n: usize, t: Duration). Which ideally receives up to the max specified events or times out before then returning whatever was received in attempt to wait for other things coming down the channel if that makes sense.

Why not use something like:

let mut items = vec![];
let timeout = after(duration);
loop {
    select! {
        recv(timeout) -> _ => { break; }
        recv(chan) -> msg {
            items.push(msg);
            if items.len() >= BATCH_LEN { break; }
        }
    }
}
// Process items...

@gterzian
Copy link

@papertigers could you please try running your test with this PR and see if it helped at all? #447

extrawurst pushed a commit to extrawurst/crossbeam that referenced this issue Mar 29, 2020
found this *working* invite link here: crossbeam-rs#366 (comment)

fixes crossbeam-rs#483
jeehoonkang pushed a commit to tomtomjhj/crossbeam that referenced this issue May 19, 2020
found this *working* invite link here: crossbeam-rs#366 (comment)

fixes crossbeam-rs#483
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants