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

mpi with rayon #169

Open
wants to merge 12 commits into
base: dev
Choose a base branch
from
Open

mpi with rayon #169

wants to merge 12 commits into from

Conversation

zhenfeizhang
Copy link
Collaborator

No description provided.

#[inline]
/// check the caller's thread is the root thread
pub fn is_root(&self) -> bool {
rayon::current_thread_index().unwrap() == 0
Copy link
Contributor

@zhiyong1997 zhiyong1997 Jan 6, 2025

Choose a reason for hiding this comment

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

I think current_thread_index may not be the actual root. For example, if we start 100 threads with into_par_iter to call current_thread_index only, 50 of them return 0 and the other 50 return 1 on my laptop.

// Use retain to avoid re-checking already synced threads
pending.retain(|&i| {
let len = self.threads[i].size();
if len >= end {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is in general correct to use the len value to check whether new data has been successfully appended to the local buffer. However, I have a concern about the syncronization. In the append function of atomic_vec, the len value seems to have been updated before the actually data is written, which means some other thread may notice the new value of len, and read the dirty data. Will this be a problem?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants