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.

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

Make sure that queries have predictable symbol names. #50964

Merged
merged 1 commit into from May 24, 2018

Conversation

michaelwoerister
Copy link
Member

@michaelwoerister michaelwoerister commented May 22, 2018

Some recent refactorings led to query names not showing up in the corresponding symbol names. perf-focus and manual profiling have been broken by this. This PR makes sure that query providers always get their own symbol and that that symbol has a predictable name.

Since this adds #[inline(never)] to a function that wraps the provider call, let's check if this does not regress performance before merging.

r? @nikomatsakis

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 22, 2018
@michaelwoerister
Copy link
Member Author

@bors try

@bors
Copy link
Contributor

bors commented May 22, 2018

⌛ Trying commit 599b79e with merge c5477dd...

bors added a commit that referenced this pull request May 22, 2018
Make sure that queries have predictable symbol names.

Some recent refactorings led to queriy names not showing up in the corresponding symbol names. [perf-focus](https://github.com/nikomatsakis/perf-focus) and manual profiling have been broken by this. This PR makes sure that query providers always get their own symbol and that that symbol has a predictable name.

Since this adds `#[inline(never)]` to a function that wraps the provider call, let's check if this does not regress performance before merging.

r? @nikomatsakis
@kennytm kennytm added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 22, 2018
@bors
Copy link
Contributor

bors commented May 22, 2018

☀️ Test successful - status-travis
State: approved= try=True

@michaelwoerister
Copy link
Member Author

@Mark-Simulacrum, could I get a perf-run, please?

@nikomatsakis
Copy link
Contributor

r=me presuming perf is fine (I'd be quite shocked to see otherwise though)

@michaelwoerister
Copy link
Member Author

Performance does not seem to be affected.

@bors r=nikomatsakis

@bors
Copy link
Contributor

bors commented May 23, 2018

📌 Commit 599b79e has been approved by nikomatsakis

@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 May 23, 2018
@michaelwoerister michaelwoerister removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 23, 2018
@michaelwoerister
Copy link
Member Author

@bors rollup

kennytm added a commit to kennytm/rust that referenced this pull request May 24, 2018
…, r=nikomatsakis

Make sure that queries have predictable symbol names.

Some recent refactorings led to query names not showing up in the corresponding symbol names. [perf-focus](https://github.com/nikomatsakis/perf-focus) and manual profiling have been broken by this. This PR makes sure that query providers always get their own symbol and that that symbol has a predictable name.

Since this adds `#[inline(never)]` to a function that wraps the provider call, let's check if this does not regress performance before merging.

r? @nikomatsakis
bors added a commit that referenced this pull request May 24, 2018
Rollup of 9 pull requests

Successful merges:

 - #50864 (Add NetBSD/arm target specs)
 - #50956 (rust-gdb: work around the re-used -d argument in cgdb)
 - #50964 (Make sure that queries have predictable symbol names.)
 - #50965 (Update LLVM to pull in another wasm fix)
 - #50972 (Add -Z no-parallel-llvm flag)
 - #50979 (Fix span for type-only arguments)
 - #50981 (Shrink `LiveNode`.)
 - #50995 (move type out of unsafe block)
 - #51011 ( rustdoc: hide macro export statements from docs)

Failed merges:
@bors bors merged commit 599b79e into rust-lang:master May 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

None yet

5 participants