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.

Sign up for GitHub

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

Jump to bottom

Use futex based thread parker on Fuchsia. #96768

Merged
merged 1 commit into from
Jun 22, 2022
Merged

Use futex based thread parker on Fuchsia. #96768

merged 1 commit into from
Jun 22, 2022

Conversation

m-ou-se
Copy link
Member

@m-ou-se m-ou-se commented May 6, 2022 •
edited
Loading

No description provided.

@m-ou-se m-ou-se added A-concurrency Area: Concurrency O-fuchsia Operating system: Fuchsia T-libs Relevant to the library team, which will review and decide on the PR/issue. labels May 6, 2022
@rust-highfive

This comment was marked as off-topic.

Sign in to view
@rust-highfive
Copy link
Collaborator

r? @kennytm

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 6, 2022
@m-ou-se
Copy link
Member Author

m-ou-se commented May 6, 2022 •
edited
Loading

This is completely untested! @tmandry I believe you're the person I should contact for Fuchsia? Can you test this?

@m-ou-se m-ou-se mentioned this pull request May 6, 2022
Closed
63 tasks
@JohnCSimon
Copy link
Member

Triage:
Seems that there's still work to be done on this one.
@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 20, 2022
@m-ou-se
Copy link
Member Author

m-ou-se commented Jun 20, 2022

@rustbot ping fuchsia

@rustbot
Copy link
Collaborator

rustbot commented Jun 20, 2022

Hey friends of Fuchsia! This issue could use some guidance on how this should be
resolved/implemented on Fuchsia. Could one of you weigh in?

cc @ComputerDruid @djkoloski @P1n3appl3 @tmandry

@tmandry
Copy link
Member

tmandry commented Jun 20, 2022

Sorry that this got lost before. We actually would like to avoid using raw futexes on Fuchsia because it loses priority inheritance information. There are a few options:

  • Use the C11 mutex API available in libc.
    • Fuchsia has a const-constructible extension to this API. (It's an int, you set it to zero.)
    • We would hard code knowledge of the layout and const construction for Fuchsia in std or the libc crate.
  • Use primitives in libsync, a Fuchsia-specific library. The libc API is a thin wrapper around this one.
    • Benefit is that libsync, unlike libc, is distributed as a static lib in the Fuchsia SDK and we can possibly get better performance.
    • Drawback is adding another library dependency of std.
    • Hard coding just as much as above.
  • Use Fuchsia-specific extensions to futexes
    • This could allow us to reuse the futex implementation, but requires keeping track of the thread[s] being waited on. I'm not sure how easy that would be.

@tmandry
Copy link
Member

tmandry commented Jun 20, 2022 •
edited
Loading

I had thought that const constructible Mutex was blocked behind this, but was surprised to see that it landed yesterday. That's great, but how is it working with Fuchsia? EDIT: Ah, I just saw #97647.

By the way, the tracking issue #93740 states that Fuchsia is a Tier 3 target but it is Tier 2.

@m-ou-se
Copy link
Member Author

m-ou-se commented Jun 21, 2022 •
edited
Loading

All those options seem fine to me. Happy to review a PR for any of those options.

I'll update this PR to only switch thread parking to a futex on Fuchsia, and leave the locks untouched for now.

@m-ou-se m-ou-se changed the title Use futex based locks and thread parker on Fuchsia. Use futex based thread parker on Fuchsia. Jun 21, 2022
@m-ou-se
Copy link
Member Author

m-ou-se commented Jun 21, 2022

Updated. @tmandry can you test it?

@tmandry
Copy link
Member

tmandry commented Jun 22, 2022

I managed to get the std::thread tests running, and they pass. If there are deeper problems, we'll probably find out soon after this lands when we try to bring the new compiler into Fuchsia :)

Thanks for working on this!

@bors r+

@bors
Copy link
Contributor

bors commented Jun 22, 2022

📌 Commit ac38258 has been approved by tmandry

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 22, 2022
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jun 22, 2022
@JohnTitor
Rollup merge of rust-lang#96768 - m-ou-se:futex-fuchsia, r=tmandry
4415af5 
Use futex based thread parker on Fuchsia.
@JohnTitor JohnTitor mentioned this pull request Jun 22, 2022
Closed
@JohnTitor JohnTitor mentioned this pull request Jun 22, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 22, 2022
@bors
Auto merge of rust-lang#98375 - JohnTitor:rollup-e5c6rgo, r=JohnTitor
89a0783 
Rollup of 10 pull requests

Successful merges:

 - rust-lang#95446 (update CPU usage script)
 - rust-lang#96768 (Use futex based thread parker on Fuchsia.)
 - rust-lang#97454 (Add release notes for 1.62)
 - rust-lang#97516 (clarify how Rust atomics correspond to C++ atomics)
 - rust-lang#97818 (Point at return expression for RPIT-related error)
 - rust-lang#97895 (Simplify `likely!` and `unlikely!` macro)
 - rust-lang#98005 (Add some tests for impossible bounds)
 - rust-lang#98226 (Document unstable `--extern` options)
 - rust-lang#98356 (Add missing period)
 - rust-lang#98363 (remove use of &Alloc in btree tests)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 897745b into rust-lang:master Jun 22, 2022
@rustbot rustbot added this to the 1.63.0 milestone Jun 22, 2022
@m-ou-se m-ou-se deleted the futex-fuchsia branch June 22, 2022 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@kennytm kennytm

Labels
A-concurrency Area: Concurrency O-fuchsia Operating system: Fuchsia S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.63.0
Development

Successfully merging this pull request may close these issues.
7 participants
@m-ou-se @rust-highfive @JohnCSimon @rustbot @tmandry @bors @kennytm