Skip to content

[Don't merge] Learn the performance impact of running lints #74718

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

Closed
wants to merge 1 commit into from

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Jul 24, 2020

Related: #74704

This is still running some things, not sure why. I think the lints are being run directly by the passes, not through lint_mod.

warning: hidden lifetime parameters in types are deprecated
  --> ui/lint/reasons.rs:21:29
   |
21 |     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
   |                             ^^^^^^^^^^^^^^- help: indicate the anonymous lifetime: `<'_>`
   |
   = note: explicit anonymous lifetimes aid reasoning about ownership
note: the lint level is defined here
  --> ui/lint/reasons.rs:5:9
   |
5  | #![warn(elided_lifetimes_in_paths,
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^

However most of the lints have gone away.

r? @ghost

This is still running some things, not sure why.

```
warning: hidden lifetime parameters in types are deprecated
  --> ui/lint/reasons.rs:21:29
   |
21 |     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
   |                             ^^^^^^^^^^^^^^- help: indicate the anonymous lifetime: `<'_>`
   |
   = note: explicit anonymous lifetimes aid reasoning about ownership
note: the lint level is defined here
  --> ui/lint/reasons.rs:5:9
   |
5  | #![warn(elided_lifetimes_in_paths,
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^
```

However most of the lints have gone away.
@jyn514
Copy link
Member Author

jyn514 commented Jul 24, 2020

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Collaborator

bors commented Jul 24, 2020

⌛ Trying commit f794bca with merge 4874c68561d0727f5ff895edef9e1e3473b6e389...

@Mark-Simulacrum
Copy link
Member

I would also make sure that these vectors

pub pre_expansion_passes: Vec<Box<dyn Fn() -> EarlyLintPassObject + sync::Send + sync::Sync>>,
pub early_passes: Vec<Box<dyn Fn() -> EarlyLintPassObject + sync::Send + sync::Sync>>,
pub late_passes: Vec<Box<dyn Fn() -> LateLintPassObject + sync::Send + sync::Sync>>,
/// This is unique in that we construct them per-module, so not once.
pub late_module_passes: Vec<Box<dyn Fn() -> LateLintPassObject + sync::Send + sync::Sync>>,
are all empty.

@jyn514
Copy link
Member Author

jyn514 commented Jul 24, 2020

@bors help

@jyn514
Copy link
Member Author

jyn514 commented Jul 24, 2020

@bors cancel

@bors
Copy link
Collaborator

bors commented Jul 24, 2020

☀️ Try build successful - checks-actions, checks-azure
Build commit: 4874c68561d0727f5ff895edef9e1e3473b6e389 (4874c68561d0727f5ff895edef9e1e3473b6e389)

@rust-timer
Copy link
Collaborator

Queued 4874c68561d0727f5ff895edef9e1e3473b6e389 with parent 9008693, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (4874c68561d0727f5ff895edef9e1e3473b6e389): 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

@jyn514
Copy link
Member Author

jyn514 commented Jul 25, 2020

decrease of at most 3.5%, mostly in the 1% range, no increases at all :D

@Mark-Simulacrum
Copy link
Member

Closing, seems like we have an initial assessment at least.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants