rustdoc: Put GenericArg::Type into a Box

[GuillaumeGomez]

This is a small experiment to see how it'll impact performance (I think it'll increase memory usage a bit). I don't think it'll be worthwhile to merge it.

r? @camelid

[rust-highfive]

Some changes occurred in clean/types.rs.

cc @camelid

[GuillaumeGomez]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

[bors]

⌛ Trying commit f2a2cd2834bd0686f539fea10d0fe5dc6c66afc2 with merge 92df98fcbbbfd0cceadbaec668e157c484de8eb8...

[petrochenkov]

@GuillaumeGomez
Meta: could you mark PR names with the code are that they touch, like "rustdoc: Put GenericArg::Type into a Box"?
Right now from the title it may look like the PR changes AST or HIR.

[GuillaumeGomez]

I'm so sorry I forgot again... >< I'll add it here but sorry for the useless noise...

[bors]

☀️ Try build successful - checks-actions
Build commit: 92df98fcbbbfd0cceadbaec668e157c484de8eb8 (92df98fcbbbfd0cceadbaec668e157c484de8eb8)

[rust-timer]

Queued 92df98fcbbbfd0cceadbaec668e157c484de8eb8 with parent f8f1751, future comparison URL.

[rust-timer]

Finished benchmarking commit (92df98fcbbbfd0cceadbaec668e157c484de8eb8): comparison url.

Summary: This benchmark run shows 29 relevant regressions 😿 to instruction counts.

• Average relevant regression: 0.5%
• Largest regression in instruction counts: 0.6% on full builds of projection-caching doc

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf +perf-regression

[GuillaumeGomez]

Surprisingly enough, the memory usage is actually being reduced:

But the performance is bit less good:

So should we keep this change or not?

[camelid]

Hmm, the instruction count regressions are higher than I'd like ... but the memory improvement does seem nice. Ideally, we would try to shrink Type instead of just boxing it—though that's quite difficult. I'm ambivalent about this change; I'm not sure whether to merge it or close it.

[GuillaumeGomez]

I'll leave it up to you. We can still take a look at Type in the meantime. ;)

Actually I'll take a look at Type in any case.

[camelid]

Yeah, I've been meaning to get back to trying to shrink Type for a while now. If you want to look into it, can you try to get rid of https://doc.rust-lang.org/nightly/nightly-rustc/rustdoc/clean/types/enum.Type.html#variant.QPath.field.self_def_id? That's a hack that should be removed and takes up decent memory space.

[bors]

☔ The latest upstream changes (presumably #94009) made this pull request unmergeable. Please resolve the merge conflicts.

[GuillaumeGomez]

So what should we do here?

[camelid]

My current thinking is the perf improvements aren't worth the regressions and increased code complexity (having to box and unbox). So I propose closing this PR.

[GuillaumeGomez]

Agreed!