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

Use tracing spans in rustc_trait_selection #77792

Merged
merged 1 commit into from
Oct 13, 2020

Conversation

matthewjasper
Copy link
Contributor

Spans are very helpful when debugging this code. It's also hot enough to make a good benchmark.

r? @oli-obk

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 10, 2020
@matthewjasper
Copy link
Contributor Author

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Contributor

bors commented Oct 10, 2020

⌛ Trying commit 33566e3c4d5c128db222179e0fd0ea4ac7deff27 with merge 33f17791eda83acb9da4fee1c76dda7d9a630c7f...

@bors
Copy link
Contributor

bors commented Oct 10, 2020

☀️ Try build successful - checks-actions, checks-azure
Build commit: 33f17791eda83acb9da4fee1c76dda7d9a630c7f (33f17791eda83acb9da4fee1c76dda7d9a630c7f)

@rust-timer
Copy link
Collaborator

Queued 33f17791eda83acb9da4fee1c76dda7d9a630c7f with parent 87b71ed, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (33f17791eda83acb9da4fee1c76dda7d9a630c7f): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never

@matthewjasper
Copy link
Contributor Author

This is up to a 0.8% regression. I'll see if reducing the number of spans being used can help here.

@matthewjasper
Copy link
Contributor Author

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Contributor

bors commented Oct 11, 2020

⌛ Trying commit a555e3e018d4bae90448e51f4ce34bad3da3376d with merge 021a682eb34f6400e8cf300121d1b4a28f6267e3...

@rust-log-analyzer
Copy link
Collaborator

The job dist-x86_64-linux of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
   Compiling rustc_trait_selection v0.0.0 (/checkout/compiler/rustc_trait_selection)
error: expected expression, found `?`
   --> compiler/rustc_trait_selection/src/traits/fulfill.rs:346:38
    |
346 |         debug!("process_obligation", ?obligation, ?obligation.cause);
    |                                      ^ expected expression
error: aborting due to previous error

error: could not compile `rustc_trait_selection`


To learn more, run the command again with --verbose.
warning: build failed, waiting for other jobs to finish...
error: build failed
command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "build" "--target" "x86_64-unknown-linux-gnu" "-Zbinary-dep-depinfo" "-j" "16" "--release" "--locked" "--color" "always" "--features" "jemalloc llvm max_level_info" "--manifest-path" "/checkout/compiler/rustc/Cargo.toml" "--message-format" "json-render-diagnostics"
failed to run: /checkout/obj/build/bootstrap/debug/bootstrap dist --host x86_64-unknown-linux-gnu --target x86_64-unknown-linux-gnu
Build completed unsuccessfully in 0:11:36
== clock drift check ==
  local time: Sun Oct 11 10:53:11 UTC 2020
  local time: Sun Oct 11 10:53:11 UTC 2020
  network time: Sun, 11 Oct 2020 10:53:12 GMT
== end clock drift check ==
##[error]Process completed with exit code 1.
Terminate orphan process: pid (4653) (python)

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @rust-lang/infra. (Feature Requests)

@bors
Copy link
Contributor

bors commented Oct 11, 2020

💔 Test failed - checks-actions

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 11, 2020
@matthewjasper
Copy link
Contributor Author

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Contributor

bors commented Oct 11, 2020

⌛ Trying commit b8d2560 with merge 3cb8bc741b28e59fb753232e99ce1d235fd6c713...

@bors
Copy link
Contributor

bors commented Oct 11, 2020

☀️ Try build successful - checks-actions, checks-azure
Build commit: 3cb8bc741b28e59fb753232e99ce1d235fd6c713 (3cb8bc741b28e59fb753232e99ce1d235fd6c713)

@rust-timer
Copy link
Collaborator

Queued 3cb8bc741b28e59fb753232e99ce1d235fd6c713 with parent 8cc82ee, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (3cb8bc741b28e59fb753232e99ce1d235fd6c713): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never

@oli-obk
Copy link
Contributor

oli-obk commented Oct 12, 2020

@bors r+

looks like we actually got a small perf improvement out of it

@bors
Copy link
Contributor

bors commented Oct 12, 2020

📌 Commit b8d2560 has been approved by oli-obk

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 12, 2020
@bors
Copy link
Contributor

bors commented Oct 12, 2020

⌛ Testing commit b8d2560 with merge abbdec3...

@bors
Copy link
Contributor

bors commented Oct 13, 2020

☀️ Test successful - checks-actions, checks-azure
Approved by: oli-obk
Pushing abbdec3 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 13, 2020
@bors bors merged commit abbdec3 into rust-lang:master Oct 13, 2020
@rustbot rustbot added this to the 1.49.0 milestone Oct 13, 2020
@matthewjasper matthewjasper deleted the instrument-trait-selection branch October 13, 2020 15:56
@Mark-Simulacrum
Copy link
Member

Final results confirm the perf improvement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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.

8 participants