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

Make crossbeam::deque::Worker::push return index of relative slot written #743

Closed
wants to merge 1 commit into from

Conversation

Bajix
Copy link

@Bajix Bajix commented Sep 4, 2021

It is useful to know when crossbeam::deque::Worker::push adds the first task to an empty deque as this can be used to facilitate simple batching by triggering tokio::task::spawn and work stealing to completion.

Here's an example of the how this change could be used

pub fn batch_update_cache<K, V>(runtime_handle: &Handle, data: (String, Vec<u8>)) {
  thread_local! {
    static QUEUE: Worker<(String, Vec<u8>)> = Worker::new_fifo();
  }

  QUEUE.with(|queue| {
    if queue.push(data).eq(&0) {
      let stealer = queue.stealer();

      runtime_handle.spawn(async move {
        let mut cache = vec![];

        cache.extend(std::iter::from_fn(|| loop {
          match stealer.steal() {
            Steal::Success(data) => break Some(data),
            Steal::Retry => continue,
            Steal::Empty => break None,
          }
        }));

        /// do something with cache
      });
    }
  });
}

@Bajix
Copy link
Author

Bajix commented Sep 16, 2021

How do we feel about this change? I could update the PR to make the same change for other data structures as well

@Bajix Bajix changed the title Make crossbeam::deque::Worker::push return previous back index Make crossbeam::deque::Worker::push return index of relative slot written Sep 18, 2021
Copy link
Member

@taiki-e taiki-e left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR.

  • First, this is a breaking change.

  • Second, if I understand correctly, the existing features (len, is_empty, etc.) seem to be sufficient for your use-case here.

@Bajix
Copy link
Author

Bajix commented Sep 19, 2021

  • First, this is a breaking change.

Actually it's not a breaking change; we can assume that as this currently returns a unit type, everywhere that currently use this does so without assignment, and as there's no addition of a #[must_use] annotation, this usage will continue to work without change.

  • Second, if I understand correctly, the existing features (len, is_empty, etc.) seem to be sufficient for your use-case here.

Those operations aren't useful for my needs. I specifically need to be able to know if an element was pushed while empty, but if I just do a push and then check len immediately thereafter, there's the possibility that another thread since pushed, and should that happen, there's no way to determine which thread was actually first to push. This PR ensures that my logic isn't prone to race conditions.

@taiki-e
Copy link
Member

taiki-e commented Nov 3, 2021

Actually it's not a breaking change; we can assume that as this currently returns a unit type, everywhere that currently use this does so without assignment, and as there's no addition of a #[must_use] annotation, this usage will continue to work without change.

No, this is breaking because an expression that returns unit (()) can omit the semicolon.

Those operations aren't useful for my needs. I specifically need to be able to know if an element was pushed while empty, but if I just do a push and then check len immediately thereafter, there's the possibility that another thread since pushed, and should that happen, there's no way to determine which thread was actually first to push. This PR ensures that my logic isn't prone to race conditions.

That makes sense.

@Bajix Bajix closed this by deleting the head repository May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

2 participants