If/when you bring this to the cargo team, it would be good to include reproduction steps as well as characterize the performance impact (when it happens, how slow it can be, and end-user impact).
In addition to the problems you gave, note that the function you linked to specifically calls out that it is blocking and why.
Git source updates use progress bars, there is no support for concurrent progress bars in cargo, so these must be disabled for concurrent source updates. Should I add support for that first before trying to upstream my changes?
We've been cautious about what terminal features we take advantage of for maximum compatibility. We've branched out more recently with unicode and hyperlink support. Like those, any progress improvements over what we have today will likely need some kind of end-user control and ideally auto-detection.
When we add support for multi-line progress indicators, we should probably do so with Console output should better reflect the build process · Issue #8889 · rust-lang/cargo · GitHub in mind or as part of it to avoid throwaway work.
GlobalContext
is notSync
. With so many fields that are notSync
, it makes me wonder whether it was a design decision. If we want concurrent progress bars, and not clone config options needed for fetching git repositories,GlobalContext
should be made thread-safe. Is there any reason a PR for this wouldn't go through?
Not quite sure if this needs to be thread safe for this. If something can fit within our pre-async
async framework, then Sync
shouldn't be needed, which describes most of our networking and Source
logic.
If we do want to make GlobalContext
Sync
, then that could make it trivial to parse transitive dependency manifests in parallel which could offer another performance boost (Loading of manifests is serial · Issue #15676 · rust-lang/cargo · GitHub).
Making it Sync
will come with some performance costs from the locking needed for it.
If this is needed, worth bringing it up with the cargo team. Usually the way to discuss implementation details like this is part of an Issue or on #t-cargo