Fix performance backlash of #74762

[ssomers]

A small mistake had a measurable effect on performance. According to benchmarks, this turns #74762 into a petty speedup.

r? @Mark-Simulacrum

[lcnr]

@bors try @rust-timer queue

[bors]

⌛ Trying commit c1c211a with merge 86d42ef9e3d641b4c17c285378d84d332de83d0b...

[bors]

☀️ Try build successful - checks-actions, checks-azure
Build commit: 86d42ef9e3d641b4c17c285378d84d332de83d0b (86d42ef9e3d641b4c17c285378d84d332de83d0b)

[lcnr]

Looks like rust-timer didn't pick this up

@rust-timer build 86d42ef9e3d641b4c17c285378d84d332de83d0b

[rust-timer]

Queued 86d42ef9e3d641b4c17c285378d84d332de83d0b with parent 1d69e3b, future comparison URL.

[rust-timer]

Finished benchmarking try commit (86d42ef9e3d641b4c17c285378d84d332de83d0b): 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

[JohnTitor]

Hm, it doesn't seem to recover the regression fully?

[ssomers]

I have no idea what is being benchmarked here, and since how long, but here is another commit reverting everything that could have an effect.

[mati865]

rust-timer measures performance of rustc compiling various crates. It doesn't run any of the benchmarks from the code.
Perf results in this case mean the changes from this PR made some code marginally easier to compile (it takes less work). Performance of Rust itself doesn't seem affected.

[ssomers]

All I'm saying is that if #74762 caused this, then #68770 must have been much worse. Or in other words, if #74762 caused this, I can easily supply a 1% performance change for the better, simply stripping down a copy of remove_kv_tracking that snubs drain_filter. And I'm not saying that this code duplication is bad, because there is more to be gained if these two are no longer chained together.

[lqd]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion

[bors]

⌛ Trying commit 1794a79 with merge 7dfbeca825bc8cbbb586199f4511d2f8859aacfa...

[bors]

☀️ Try build successful - checks-actions, checks-azure
Build commit: 7dfbeca825bc8cbbb586199f4511d2f8859aacfa (7dfbeca825bc8cbbb586199f4511d2f8859aacfa)

[rust-timer]

Queued 7dfbeca825bc8cbbb586199f4511d2f8859aacfa with parent 0d75c91, future comparison URL.

[rust-timer]

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

[ssomers]

The original regression in #75060 was alarmed about helloworld-check and neither of the commits here change that back. So can I conclude that #74762 was not the culprit? Then in my shallow view, the only other suspect is #75010.

[Mark-Simulacrum]

Looking at the regression, it seems to be down to slightly more queries being run -- all metadata decoding. I'm not sure, but it does seem not implausible that the performance was affected by an extra struct or something odd like that. It's such a minor regression that I'm not too worried about tracking it down precisely -- #75010 seems like a plausible cause too.

[ssomers]

Meanwhile, my small mistake fixed in the first commit here is superseded by #75257, so I think we're done here.