Skip to content

Replace mark_(un)blocked with (un)park#155997

Open
zetanumbers wants to merge 2 commits into
rust-lang:mainfrom
zetanumbers:waiter_thread_index
Open

Replace mark_(un)blocked with (un)park#155997
zetanumbers wants to merge 2 commits into
rust-lang:mainfrom
zetanumbers:waiter_thread_index

Conversation

@zetanumbers

@zetanumbers zetanumbers commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

View all comments

So here's the thing. We can store condvar within rustc_thread_pool and do all of bookkeeping stuff there. I thought about making a QueryLatch a simple AtomicU64 of which each bit represents a waiting thread ordered by its worker thread index. It's not as simple as that in this PR, but by moving query waiter's condvar we are finally able to use Arc::get_mut for the cycle smart pointer.

@rustbot

rustbot commented Apr 30, 2026

Copy link
Copy Markdown
Collaborator

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

@rustbot rustbot added A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 30, 2026
@rustbot

rustbot commented Apr 30, 2026

Copy link
Copy Markdown
Collaborator

r? @mu001999

rustbot has assigned @mu001999.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler
  • compiler expanded to 73 candidates
  • Random selection from 20 candidates

@zetanumbers

Copy link
Copy Markdown
Contributor Author

r? @nnethercote

@rustbot rustbot assigned nnethercote and unassigned mu001999 Apr 30, 2026
@bjorn3

bjorn3 commented Apr 30, 2026

Copy link
Copy Markdown
Member

Do I understand correctly that blocking no longer releases jobserver tokens?

@zetanumbers

Copy link
Copy Markdown
Contributor Author

Do I understand correctly that blocking no longer releases jobserver tokens?

No, it does.

https://github.com/rust-lang/rust/pull/155997/changes#diff-d53039531e1e9671c009133110e61e29aa96031b55b1f69d4f2a71e89860911eR627-R631

Or am I missing something?

@bjorn3

bjorn3 commented Apr 30, 2026

Copy link
Copy Markdown
Member

Missed those calls.

@nnethercote nnethercote left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seems ok to me, though once again this is code I don't understand well.

@Zoxc: any thoughts before I approve?

View changes since this review

pub struct QueryWaiter<'tcx> {
pub parent: Option<QueryJobId>,
pub condvar: Condvar,
// FIXME: could be made u16 due to rustc_thread_pool limiting number of threads

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think shrinking this field will change the struct size because the other fields are all 64-bits (on 64-bit platforms). I suggest just removing the comment.

@nnethercote

Copy link
Copy Markdown
Contributor

Let's check perf:

@bors try @rust-timer queue

@rust-timer

Copy link
Copy Markdown
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 1, 2026
@rust-bors

rust-bors Bot commented May 1, 2026

Copy link
Copy Markdown
Contributor

⌛ Trying commit 0d8d68d with merge 03472e2

To cancel the try build, run the command @bors try cancel.

Workflow: https://github.com/rust-lang/rust/actions/runs/25202610893

rust-bors Bot pushed a commit that referenced this pull request May 1, 2026
Replace `mark_(un)blocked` with `(un)park`
@rust-bors rust-bors Bot added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label May 1, 2026
@rust-bors

rust-bors Bot commented May 1, 2026

Copy link
Copy Markdown
Contributor

💥 Test timed out after 21600s

@rust-bors rust-bors Bot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 1, 2026
@Zoxc

Zoxc commented May 1, 2026

Copy link
Copy Markdown
Contributor

It looks good to me.

@nnethercote

Copy link
Copy Markdown
Contributor

@bors try @rust-timer queue

@rust-timer

Copy link
Copy Markdown
Collaborator

This pull request is already queued and waiting for a try build to finish.

@rust-bors

rust-bors Bot commented May 5, 2026

Copy link
Copy Markdown
Contributor

⌛ Trying commit 0d8d68d with merge ea11640

To cancel the try build, run the command @bors try cancel.

Workflow: https://github.com/rust-lang/rust/actions/runs/25356066731

rust-bors Bot pushed a commit that referenced this pull request May 5, 2026
Replace `mark_(un)blocked` with `(un)park`
@rust-bors

rust-bors Bot commented May 5, 2026

Copy link
Copy Markdown
Contributor

💥 Test timed out after 21600s

@zetanumbers

Copy link
Copy Markdown
Contributor Author

huh

@nnethercote

Copy link
Copy Markdown
Contributor

This has timed out twice now. I suspect there is a problem with the PR, some kind of hang.

@zetanumbers

Copy link
Copy Markdown
Contributor Author

It hangs on the LLVM build. I am unsure how this PR would've been able to cause any hang tbh.

@zetanumbers zetanumbers force-pushed the waiter_thread_index branch from 0d8d68d to f6fb138 Compare May 6, 2026 10:27
@rustbot

This comment has been minimized.

@zetanumbers zetanumbers force-pushed the waiter_thread_index branch from f6fb138 to 7107849 Compare May 8, 2026 11:13
@rustbot

rustbot commented May 8, 2026

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. S-waiting-on-perf Status: Waiting on a perf run to be completed. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants