-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
tree-wide: parallel: Fully removed all Lrc
, replaced with Arc
#136471
Conversation
The rustc-dev-guide subtree was changed. If this PR only touches the dev guide consider submitting a PR directly to rust-lang/rustc-dev-guide otherwise thank you for updating the dev guide with your changes. cc @BoxyUwU, @jieyouxu, @Kobzol Some changes occurred in compiler/rustc_codegen_gcc Some changes occurred in src/tools/clippy cc @rust-lang/clippy Some changes occurred in src/tools/rustfmt cc @rust-lang/rustfmt
cc @davidtwco, @compiler-errors, @TaKO8Ki The Miri subtree was changed cc @rust-lang/miri |
Some changes occurred in src/tools/rustfmt cc @rust-lang/rustfmt The rustc-dev-guide subtree was changed. If this PR only touches the dev guide consider submitting a PR directly to rust-lang/rustc-dev-guide otherwise thank you for updating the dev guide with your changes. cc @BoxyUwU, @jieyouxu, @Kobzol The Miri subtree was changed cc @rust-lang/miri Some changes occurred in src/tools/clippy cc @rust-lang/clippy
cc @davidtwco, @compiler-errors, @TaKO8Ki Some changes occurred in compiler/rustc_codegen_gcc |
This comment has been minimized.
This comment has been minimized.
I rebased |
This comment has been minimized.
This comment has been minimized.
This is our way forward :) Thanks! |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
tree-wide: parallel: Fully removed all `Lrc`, replaced with `Arc` tree-wide: parallel: Fully removed all `Lrc`, replaced with `Arc` This is continuation of rust-lang#132282 . I'm pretty sure I did everything right. In particular, I searched all occurrences of `Lrc` in submodules and made sure that they don't need replacement. There are other possibilities, through. We can define `enum Lrc<T> { Rc(Rc<T>), Arc(Arc<T>) }`. Or we can make `Lrc` a union and on every clone we can read from special thread-local variable. Or we can add a generic parameter to `Lrc` and, yes, this parameter will be everywhere across all codebase. So, if you think we should take some alternative approach, then don't merge this PR. But if it is decided to stick with `Arc`, then, please, merge. cc "Parallel Rustc Front-end" ( rust-lang#113349 ) r? SparrowLii `@rustbot` label WG-compiler-parallel
The compiler should in fact already use |
☀️ Try build successful - checks-actions |
This comment has been minimized.
This comment has been minimized.
Finished benchmarking commit (c1d3b4f): comparison URL. Overall result: no relevant changes - no action neededBenchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf. @bors rollup=never Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)Results (secondary 0.8%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 776.632s -> 777.793s (0.15%) |
@SparrowLii , why this is not merged? |
Just waited a while in case there were other opinions @bors r+ rollup=never |
☀️ Test successful - checks-actions |
Finished benchmarking commit (2f92f05): comparison URL. Overall result: ❌✅ regressions and improvements - no action needed@rustbot label: -perf-regression Instruction countThis is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.
Max RSS (memory usage)Results (secondary 3.9%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResults (primary -3.7%, secondary 2.3%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 779.996s -> 778.937s (-0.14%) |
- Implement internal for RawPtrKind, c.f. rust-lang/rust#136590 - Replace Lrc with Arc, c.f. rust-lang/rust#136471
- Implement internal for RawPtrKind, c.f. rust-lang/rust#136590 - Replace Lrc with Arc, c.f. rust-lang/rust#136471
Upgrade toolchain to 2/10. I **highly recommend** reviewing this PR commit-by-commit. The description in each commit message links to the upstream PRs that prompted those particular changes. ## Callouts - 2/1 had a lot of formatting changes. I split the commits for that day into formatting changes and functionality changes accordingly. - 2/5 introduced a regression in our delayed UB instrumentation, so I made a new fixme test. See #3881 for details. ## Culprit PRs: rust-lang/rust#134424 rust-lang/rust#130514 rust-lang/rust#135748 rust-lang/rust#136590 rust-lang/rust#135318 rust-lang/rust#135265 rust-lang/rust@bcb8565 rust-lang/rust#136471 rust-lang/rust#136645 Resolves #3863 By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.
tree-wide: parallel: Fully removed all
Lrc
, replaced withArc
This is continuation of #132282 .
I'm pretty sure I did everything right. In particular, I searched all occurrences of
Lrc
in submodules and made sure that they don't need replacement.There are other possibilities, through.
We can define
enum Lrc<T> { Rc(Rc<T>), Arc(Arc<T>) }
. Or we can makeLrc
a union and on every clone we can read from special thread-local variable. Or we can add a generic parameter toLrc
and, yes, this parameter will be everywhere across all codebase.So, if you think we should take some alternative approach, then don't merge this PR. But if it is decided to stick with
Arc
, then, please, merge.cc "Parallel Rustc Front-end" ( #113349 )
r? SparrowLii
@rustbot label WG-compiler-parallel