Skip to content

Set callbacks globally #67614

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Dec 29, 2019
Merged

Conversation

Mark-Simulacrum
Copy link
Member

@Mark-Simulacrum Mark-Simulacrum commented Dec 25, 2019

This sets the callbacks from syntax and rustc_errors just once, utilizing static (rather than thread-local) storage.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 25, 2019
@Mark-Simulacrum
Copy link
Member Author

I guess so. Maybe put the call in run_compiler_in_existing_thread_pool since there's 2 spawn_thread_pool impls

I opted not to do this for now, I think duplicating a single function call isn't too bad.

If we put it in spawn_thread_pool before spawning the threads we could make it use Relaxed atomics.

I don't feel confident enough to say either way; regardless, the load on an Atomic should not be hot code I think -- the value isn't changing, and we only issue ~1 store per program to the address. Either way in a dummy example (https://rust.godbolt.org/z/jE2XJj) changing the load to Relaxed instead of SeqCst didn't seem to have any effect on the generated assembly.

Copy link
Contributor

@Centril Centril left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nits

The callbacks have precisely two states: the default, and the one
present throughout almost all of the rustc run (the filled in value
which has access to TyCtxt).

We used to store this as a thread local, and reset it on each thread to
the non-default value. But this is somewhat wasteful, since there is no
reason to set it globally -- while the callbacks themselves access TLS,
they do not do so in a manner that fails in when we do not have TLS to
work with.
@Zoxc
Copy link
Contributor

Zoxc commented Dec 25, 2019

r=me when CI passes

@Mark-Simulacrum
Copy link
Member Author

@bors r=Zoxc

@bors
Copy link
Collaborator

bors commented Dec 25, 2019

📌 Commit 4dcc627 has been approved by Zoxc

@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 Dec 25, 2019
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Dec 26, 2019
…=Zoxc

Set callbacks globally

This sets the callbacks from syntax and rustc_errors just once, utilizing static (rather than thread-local) storage.
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Dec 26, 2019
…=Zoxc

Set callbacks globally

This sets the callbacks from syntax and rustc_errors just once, utilizing static (rather than thread-local) storage.
bors added a commit that referenced this pull request Dec 26, 2019
Rollup of 12 pull requests

Successful merges:

 - #67112 (Refactor expression parsing thoroughly)
 - #67192 (Various const eval and pattern matching ICE fixes)
 - #67287 (typeck: note other end-point when checking range pats)
 - #67459 (prune ill-conceived BTreeMap iter_mut assertion and test its mutability)
 - #67576 (reuse `capacity` variable in slice::repeat)
 - #67602 (Use issue = "none" instead of "0" in intrinsics)
 - #67614 (Set callbacks globally)
 - #67617 (Remove `compiler_builtins_lib` documentation)
 - #67629 (Remove redundant link texts)
 - #67632 (Convert collapsed to shortcut reference links)
 - #67633 (Update .mailmap)
 - #67635 (Document safety of Path casting)

Failed merges:

r? @ghost
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Dec 27, 2019
…=Zoxc

Set callbacks globally

This sets the callbacks from syntax and rustc_errors just once, utilizing static (rather than thread-local) storage.
@bors
Copy link
Collaborator

bors commented Dec 29, 2019

⌛ Testing commit 4dcc627 with merge 774a4bd...

bors added a commit that referenced this pull request Dec 29, 2019
Set callbacks globally

This sets the callbacks from syntax and rustc_errors just once, utilizing static (rather than thread-local) storage.
@bors
Copy link
Collaborator

bors commented Dec 29, 2019

☀️ Test successful - checks-azure
Approved by: Zoxc
Pushing 774a4bd to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 29, 2019
@bors bors merged commit 4dcc627 into rust-lang:master Dec 29, 2019
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants