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 parking lot's rwlock even without parallel-rustc #114283

Merged
merged 1 commit into from
Aug 1, 2023
Merged

Use parking lot's rwlock even without parallel-rustc #114283

merged 1 commit into from
Aug 1, 2023

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Jul 31, 2023 •
edited
Loading

Considering that this doesn't affect perf, I think we should use the simplest solution.

@rustbot rustbot added 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 Jul 31, 2023
@oli-obk
Copy link
Contributor Author

oli-obk commented Jul 31, 2023

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

Sign in to view
@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 31, 2023
@bors
Copy link
Contributor

bors commented Jul 31, 2023

⌛ Trying commit 4f4a14e8711ee82ccf5a1e4d6bbf6816ea5306ec with merge adeb316618109101a43a46cff88a869c1fc90bd3...

@bors
Copy link
Contributor

bors commented Jul 31, 2023

☀️ Try build successful - checks-actions
Build commit: adeb316618109101a43a46cff88a869c1fc90bd3 (adeb316618109101a43a46cff88a869c1fc90bd3)

@rust-timer

This comment has been minimized.

Sign in to view
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (adeb316618109101a43a46cff88a869c1fc90bd3): comparison URL.

Overall result: ❌ regressions - no action needed

Benchmarking 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
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 0.6% [0.6%, 0.6%] 1
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results

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.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 -2.7% [-2.7%, -2.7%] 1
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) -2.7% [-2.7%, -2.7%] 1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 652.303s -> 652.414s (0.02%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Aug 1, 2023
@oli-obk oli-obk changed the title WIP: Try out perf of using parking lot's rwlock even without parallel-rustc Try out perf of using parking lot's rwlock even without parallel-rustc Aug 1, 2023
@oli-obk
Copy link
Contributor Author

oli-obk commented Aug 1, 2023

r? @SparrowLii

@rustbot
Copy link
Collaborator

rustbot commented Aug 1, 2023

Failed to set assignee to SparrowLii: invalid assignee

Note: Only org members, users with write permissions, or people who have commented on the PR may be assigned.

@oli-obk oli-obk changed the title Try out perf of using parking lot's rwlock even without parallel-rustc Use parking lot's rwlock even without parallel-rustc Aug 1, 2023
@oli-obk oli-obk force-pushed the parkin_lot_rwlock branch from 4f4a14e to 3eb5733 Compare August 1, 2023 06:56
@SparrowLii
Copy link
Member

Thanks for pushing this!

@oli-obk
Copy link
Contributor Author

oli-obk commented Aug 1, 2023

r? @SparrowLii

@oli-obk
Copy link
Contributor Author

oli-obk commented Aug 1, 2023

lol, good rustbot

@oli-obk
Copy link
Contributor Author

oli-obk commented Aug 1, 2023

@bors delegate+

@bors
Copy link
Contributor

bors commented Aug 1, 2023

✌️ @oli-obk, you can now approve this pull request!

If @oli-obk told you to "r=me" after making some further change, please make that change, then do @bors r=@oli-obk

@oli-obk oli-obk mentioned this pull request Aug 1, 2023
Closed
@SparrowLii
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Aug 1, 2023

@SparrowLii: 🔑 Insufficient privileges: Not in reviewers

@oli-obk
Copy link
Contributor Author

oli-obk commented Aug 1, 2023

@bors r=SparrowLii

@bors
Copy link
Contributor

bors commented Aug 1, 2023

📌 Commit 3eb5733 has been approved by SparrowLii

It is now in the queue for this repository.

@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-review Status: Awaiting review from the assignee but also interested parties. labels Aug 1, 2023
@oli-obk
Copy link
Contributor Author

oli-obk commented Aug 1, 2023

@bors rollup

@lqd
Copy link
Member

lqd commented Aug 1, 2023 •
edited
Loading

I thought it was yet another bors bug vendetta against oli, but no it kinda makes sense, delegation gives r rights to the PR author which is also oli :)

@oli-obk
Copy link
Contributor Author

oli-obk commented Aug 1, 2023

🤦

Should've been

@bors delegate=SparrowLii

@bors
Copy link
Contributor

bors commented Aug 1, 2023

✌️ @SparrowLii, you can now approve this pull request!

If @oli-obk told you to "r=me" after making some further change, please make that change, then do @bors r=@oli-obk

@matthiaskrgr matthiaskrgr mentioned this pull request Aug 1, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 1, 2023
@bors
Auto merge of rust-lang#114331 - matthiaskrgr:rollup-rnrmwcx, r=matth…
4896daa 
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#100455 (Implement RefUnwindSafe for Backtrace)
 - rust-lang#113428 (coverage: Replace `ExpressionOperandId` with enum `Operand`)
 - rust-lang#114283 (Use parking lot's rwlock even without parallel-rustc)
 - rust-lang#114288 (Improve diagnostic for wrong borrow on binary operations)
 - rust-lang#114296 (interpret: fix alignment handling for Repeat expressions)
 - rust-lang#114306 ([rustc_data_structures][perf] Simplify base_n::push_str.)
 - rust-lang#114320 (Cover statements for stable_mir)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit b387180 into rust-lang:master Aug 1, 2023
@rustbot rustbot added this to the 1.73.0 milestone Aug 1, 2023
@klensy klensy mentioned this pull request Aug 3, 2023
Merged
matthiaskrgr referenced this pull request in matthiaskrgr/rust Aug 4, 2023
@matthiaskrgr
Rollup merge of rust-lang#114418 - klensy:parking_lot, r=oli-obk
4669905 
bump parking_lot to 0.12

Bumps parking_lot to 0.12, replaces few explicit uses of parking_lot with rustc_data_structures::sync ones.

<strike>cc `@oli-obk` this touches recent https://github.com/rust-lang/rust/pull/114283</strike>
cc `@SparrowLii` i've checked that this builds with parallel-compiler

measureme's bump rust-lang/measureme#209
https://github.com/rust-lang/rust/blob/fcf3006e0133365ecd26894689c086387edcbecb/compiler/rustc_data_structures/src/sync.rs#L18-L34
RalfJung referenced this pull request in RalfJung/miri Aug 5, 2023
@matthiaskrgr
Rollup merge of #114418 - klensy:parking_lot, r=oli-obk
707c746 
bump parking_lot to 0.12

Bumps parking_lot to 0.12, replaces few explicit uses of parking_lot with rustc_data_structures::sync ones.

<strike>cc `@oli-obk` this touches recent https://github.com/rust-lang/rust/pull/114283</strike>
cc `@SparrowLii` i've checked that this builds with parallel-compiler

measureme's bump rust-lang/measureme#209
https://github.com/rust-lang/rust/blob/fcf3006e0133365ecd26894689c086387edcbecb/compiler/rustc_data_structures/src/sync.rs#L18-L34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@SparrowLii SparrowLii

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.73.0
Development

Successfully merging this pull request may close these issues.
6 participants
@oli-obk @rust-timer @bors @rustbot @SparrowLii @lqd