Skip to content

Conversation

susitsm
Copy link
Contributor

@susitsm susitsm commented Sep 15, 2025

What does this PR try to resolve?

By making GlobalContext Sync, it will become much easier to parallelize parts of cargo. A concrete example is #15676.

It was my understanding from #15934 (comment) that this would be a welcome change by the cargo team.

Overview

In GlobalContext and structs used in its fields

  • RefCell-s were replaced by either an std::sync::Mutex or an std::sync::RwLock, depending on API needs
  • LazyCell-s were replaced by a new OnceLock implementation backed by std::sync::OnceLock, emulating unstable features needed by cargo
  • Removed LazyCell/OnceLock from fields where the initialization function is just a Mutex<HashMap>::default()
  • added util::context::tests::sync_context test that does not compile if GlobalContext is not Sync

How to test and review this PR?

This PR should add no user-facing changes. Tests must pass and benchmarks must report no changes.

@rustbot
Copy link
Collaborator

rustbot commented Sep 15, 2025

r? @weihanglo

rustbot has assigned @weihanglo.
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

@rustbot rustbot added A-configuration Area: cargo config files and env vars A-console-output Area: Terminal output, colors, progress bar, etc. A-filesystem Area: issues with filesystems A-git Area: anything dealing with git A-manifest Area: Cargo.toml issues A-registries Area: registries A-workspaces Area: workspaces S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 15, 2025
@epage
Copy link
Contributor

epage commented Sep 16, 2025

Thanks!

@epage
Copy link
Contributor

epage commented Sep 16, 2025

FYI we are fine with editing existing history and, in a case like this, I would prefer it over having fixup commits later in the history as it makes the commits I'm reviewing more understandable on their own.

@susitsm susitsm force-pushed the sync-globalcontext branch 2 times, most recently from eb1396a to 7f8e4f9 Compare September 16, 2025 19:06
This will make it easier to change the implementation in the future.
@epage epage force-pushed the sync-globalcontext branch 2 times, most recently from 64e7db5 to cec5a22 Compare September 22, 2025 15:08
@epage epage force-pushed the sync-globalcontext branch 2 times, most recently from a01b9bf to a590d3e Compare September 22, 2025 15:18
Previously, `gctx.shell().verbosity()` was used to check that `gctx.shell` is
not borrowed. Since shell is now behind a `Mutex` and not a `RefCell`,
this would hang waiting for the unlock instead panicking. Borrow state
checking is now done using `Mutex::try_lock` in
`GlobalContext::debug_assert_shell_not_borrowed`
@epage epage force-pushed the sync-globalcontext branch from a590d3e to 9929c87 Compare September 22, 2025 15:43
@epage epage enabled auto-merge September 22, 2025 15:45
@epage
Copy link
Contributor

epage commented Sep 22, 2025

Thanks!

@epage epage added this pull request to the merge queue Sep 22, 2025
Merged via the queue into rust-lang:master with commit c86bc37 Sep 22, 2025
25 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-configuration Area: cargo config files and env vars A-console-output Area: Terminal output, colors, progress bar, etc. A-filesystem Area: issues with filesystems A-git Area: anything dealing with git A-manifest Area: Cargo.toml issues A-registries Area: registries A-workspaces Area: workspaces
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants