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

ty::tls cleanups #107150

Merged
merged 3 commits into from
Jan 26, 2023
Merged

ty::tls cleanups #107150

merged 3 commits into from
Jan 26, 2023

Conversation

Noratrieb
Copy link
Member

Pull it out into a separate file, make the conditional compilation more obvious and give the internal functions better names.

Pulled out of #106311

r? cjgillot

@Noratrieb
Move ty::tls to seperate file
afd5307
@Noratrieb
Some ty::tls cleanups
0d11b77 
Putting the cfged functions into a seperate module and giving them
better names.
@Noratrieb
Use strict provenance APIs in ty::tls
db305d0
@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 Jan 21, 2023
@Noratrieb
Copy link
Member Author

Shouldn't have a perf impact but just to be sure
@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 Jan 21, 2023
@bors
Copy link
Contributor

bors commented Jan 21, 2023

⌛ Trying commit a968d4c919331908f1e90adcba37d14bb1c70f01 with merge 868e8c74a103a33132cd1aedb37f7a3abc9cb92f...

@Noratrieb
Copy link
Member Author

Oops, small mistake, it needs to be ty::context::tls, ty::tls doesn't work.
@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

Sign in to view
@bors
Copy link
Contributor

bors commented Jan 21, 2023

⌛ Trying commit db305d0 with merge 1bf1caa28ff86bd33dae8fa7d4d49a8cc180129c...

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bors
Copy link
Contributor

bors commented Jan 21, 2023

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

@rust-timer

This comment has been minimized.

Sign in to view
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (1bf1caa28ff86bd33dae8fa7d4d49a8cc180129c): comparison URL.

Overall result: ❌ regressions - 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.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@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.4% [0.4%, 0.4%] 2
Regressions ❌
(secondary)		 0.7% [0.7%, 0.9%] 6
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) 0.4% [0.4%, 0.4%] 2

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)		 4.7% [4.7%, 4.7%] 1
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) 4.7% [4.7%, 4.7%] 1

Cycles

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

@rustbot rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Jan 21, 2023
@Noratrieb
Copy link
Member Author

This is noise that also showed up in other places this week, rust-lang/rustc-perf#1516 (for example here).

@Noratrieb Noratrieb added perf-regression-triaged The performance regression has been triaged. perf-regression Performance regression. and removed perf-regression Performance regression. labels Jan 25, 2023
@cjgillot
Copy link
Contributor

To verify this is noise.
@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 Jan 25, 2023
@bors
Copy link
Contributor

bors commented Jan 25, 2023

⌛ Trying commit db305d0 with merge 03d0942596fde23339919dea17ede06c563e1438...

@bors
Copy link
Contributor

bors commented Jan 25, 2023

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

1 similar comment
@bors
Copy link
Contributor

bors commented Jan 25, 2023

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

@rust-timer

This comment has been minimized.

Sign in to view
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (03d0942596fde23339919dea17ede06c563e1438): comparison URL.

Overall result: no relevant changes - 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 benchmark run did not return any relevant results for this metric.

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)		 2.6% [2.1%, 2.9%] 3
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 -1.1% [-1.1%, -1.1%] 2
All ❌✅ (primary) - - 0

Cycles

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

@rustbot rustbot removed S-waiting-on-perf Status: Waiting on a perf run to be completed. perf-regression Performance regression. labels Jan 25, 2023
@cjgillot
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jan 26, 2023

📌 Commit db305d0 has been approved by cjgillot

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 Jan 26, 2023
@oli-obk
Copy link
Contributor

oli-obk commented Jan 26, 2023

@bors rollup=always

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 26, 2023
@matthiaskrgr
Rollup merge of rust-lang#107150 - Nilstrieb:thread-local-cleanups, r…
4d0bd4a 
…=cjgillot

`ty::tls` cleanups

Pull it out into a separate file, make the conditional compilation more obvious and give the internal functions better names.

Pulled out of rust-lang#106311

r? cjgillot
@matthiaskrgr matthiaskrgr mentioned this pull request Jan 26, 2023
Closed
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 26, 2023
@matthiaskrgr
Rollup merge of rust-lang#107150 - Nilstrieb:thread-local-cleanups, r…
6ec4b73 
…=cjgillot

`ty::tls` cleanups

Pull it out into a separate file, make the conditional compilation more obvious and give the internal functions better names.

Pulled out of rust-lang#106311

r? cjgillot
This was referenced Jan 26, 2023
Closed
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 26, 2023
@bors
Auto merge of rust-lang#107328 - matthiaskrgr:rollup-lfqwo0o, r=matth…
c62665e 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#106904 (Preserve split DWARF files when building archives.)
 - rust-lang#106971 (Handle diagnostics customization on the fluent side (for one specific diagnostic))
 - rust-lang#106978 (Migrate mir_build's borrow conflicts)
 - rust-lang#107150 (`ty::tls` cleanups)
 - rust-lang#107168 (Use a type-alias-impl-trait in `ObligationForest`)
 - rust-lang#107189 (Encode info for Adt in a single place.)
 - rust-lang#107322 (Custom mir: Add support for some remaining, easy to support constructs)
 - rust-lang#107323 (Disable ConstGoto opt in cleanup blocks)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 70a665a into rust-lang:master Jan 26, 2023
@rustbot rustbot added this to the 1.69.0 milestone Jan 26, 2023
@Noratrieb Noratrieb deleted the thread-local-cleanups branch January 26, 2023 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@cjgillot cjgillot

Labels
perf-regression-triaged The performance regression has been triaged. 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.69.0
Development

Successfully merging this pull request may close these issues.
7 participants
@Noratrieb @rust-timer @bors @rust-log-analyzer @cjgillot @oli-obk @rustbot