Remove redundant imports for making benchmarking results more representative of #117772.

[surechen]

For making benchmarking results more representative of rust-117772.

[surechen]

r? @Kobzol

[Kobzol]

Hi :) I'm not sure if I understand the motivation of this PR. What do you mean by "making results more representative"? We usually don't modify the benchmarks here to make individual PRs have less performance impact.

[surechen]

Hi :) I'm not sure if I understand the motivation of this PR. What do you mean by "making results more representative"? We usually don't modify the benchmarks here to make individual PRs have less performance impact.

Hello, thank you.
My understanding is:
The PR(#117772) I'm submitting to rustc detects more redundant imports. After that PR is merged, these imports in compile-benchmarks should also be deleted because they will cause warnings. When currently doing performance testing, some crates containing redudant imports have performance regressions due to lint errors. This cannot reflect the real performance of check_unused pass itself. Therefore, Petrochenkov suggested in review #117772 that I delete these imports in advance.

[nnethercote]

I agree with @Kobzol: we generally avoid changing the benchmarks as much as possible. If the compiler got slower because it's doing more checking, then it got slower and we shouldn't hide that.

[Kobzol]

Indeed, our benchmarks probably generate new warnings because of compiler changes all the time, but that's not a reason to change the benchmarks. We try to only modify them when they actually stop compiling, e.g. because of some soundness bugs being fixed in backwards-incompatible way.

[Kobzol]

@surechen Could you please point me to the comment made by petrochenkov? I couldn't find it.

[surechen]

@surechen Could you please point me to the comment made by petrochenkov? I couldn't find it.

Ok. here:

rust-lang/rust#117772 (comment)

[petrochenkov]

More relevant comment - rust-lang/rust#117772 (comment).

If the benchmarks are not supposed to be changed, then we'll need to just acknowledge where the perf regressions in rust-lang/rust#117772 come from and mark them as triaged.
I just wanted to make sure that the regressions are indeed from reporting new lints and not from the check_unused pass.

[Kobzol]

If the benchmarks are not supposed to be changed, then we'll need to just acknowledge where the perf regressions in rust-lang/rust#117772 come from and mark them as triaged.
I just wanted to make sure that the regressions are indeed from reporting new lints and not from the check_unused pass.

I see. So, I'm perfectly fine with triaging the regressions from that PR, it is doing more work after all.

In addition to that, if we want to examine the performance of the PR in more detail, I would suggest running rustc-perf locally with the original benchmarks and with the modified benchmarks, and comparing the difference. We can't easily modify the set of benchmarks on CI easily to do this test on the benchmark server.

[surechen]

If the benchmarks are not supposed to be changed, then we'll need to just acknowledge where the perf regressions in rust-lang/rust#117772 come from and mark them as triaged.
I just wanted to make sure that the regressions are indeed from reporting new lints and not from the check_unused pass.

I see. So, I'm perfectly fine with triaging the regressions from that PR, it is doing more work after all.

In addition to that, if we want to examine the performance of the PR in more detail, I would suggest running rustc-perf locally with the original benchmarks and with the modified benchmarks, and comparing the difference. We can't easily modify the set of benchmarks on CI easily to do this test on the benchmark server.

If the benchmarks are not supposed to be changed, then we'll need to just acknowledge where the perf regressions in rust-lang/rust#117772 come from and mark them as triaged.
I just wanted to make sure that the regressions are indeed from reporting new lints and not from the check_unused pass.

I see. So, I'm perfectly fine with triaging the regressions from that PR, it is doing more work after all.

In addition to that, if we want to examine the performance of the PR in more detail, I would suggest running rustc-perf locally with the original benchmarks and with the modified benchmarks, and comparing the difference. We can't easily modify the set of benchmarks on CI easily to do this test on the benchmark server.

Hi, petrochenkov, Kobzol , nnethercote
Thank you very much.
I will close this PR.