Remove DropArena.

[nnethercote]

Most arena-allocate types that impl Drop get their own TypedArena, but a
few infrequently used ones share a DropArena. This sharing adds complexity
but doesn't help performance or memory usage. Perhaps it was more effective in
the past prior to some other improvements to arenas.

This commit removes DropArena and the sharing of arenas via the few
attribute of the arena_types macro. This change removes over 100 lines of
code and nine uses of unsafe (one of which affects the parallel compiler) and
makes the remaining code easier to read.

[rust-highfive]

r? @jackh726

(rust-highfive has picked a reviewer for you, use r? to override)

[nnethercote]

If all goes well, this won't affect performance at all.

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

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

[bors]

⌛ Trying commit fb80c73 with merge 058077a231e47388f5d3ff792aebd54885ebe43f...

[bors]

☀️ Try build successful - checks-actions
Build commit: 058077a231e47388f5d3ff792aebd54885ebe43f (058077a231e47388f5d3ff792aebd54885ebe43f)

[rust-timer]

Queued 058077a231e47388f5d3ff792aebd54885ebe43f with parent 4205481, future comparison URL.

[rust-timer]

Finished benchmarking commit (058077a231e47388f5d3ff792aebd54885ebe43f): comparison url.

Summary: This change led to moderate relevant mixed results 🤷 in compiler performance.

• Moderate improvement in instruction counts (up to -1.2% on incr-unchanged builds of html5ever)
• Moderate regression in instruction counts (up to 0.7% on full builds of webrender)

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

[nnethercote]

I think those results basically equate to noise.

This change really should have a negligible effect because DropArena is used extremely infrequently... I did some logging and found that building a stage 2 compiler only 75 DropArena allocations were done, which is an incredibly small number. And in webrender (the one benchmark I checked) only 2 DropArena allocations were done.

[Mark-Simulacrum]

I agree that the results are unlikely to be meaningful in practice and/or are worth the cleanup.

@bors r+

[bors]

📌 Commit fb80c73 has been approved by Mark-Simulacrum

[bors]

⌛ Testing commit fb80c73 with merge d914f17...

[bors]

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing d914f17 to master...

[rust-timer]

Finished benchmarking commit (d914f17): comparison url.

Summary: This change led to very large relevant mixed results 🤷 in compiler performance.

• Small improvement in instruction counts (up to -0.3% on incr-patched: println builds of clap-rs)
• Very large regression in instruction counts (up to 6.1% on incr-unchanged builds of inflate)

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

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression

[nnethercote]

Summary: This change led to very large relevant mixed results 🤷 in compiler performance.

As above, we think these changes are just noise. DropArena was barely used, so in theory its removal should have negligible effect on speed and memory use. And in practice:

• only a small number of benchmarks had significant changes;
• if you compare instructions vs. cycles vs. wall-time vs. max-rss there is no discernible pattern to the changes; and
• the pre-merge perf run and the post-merge perf run don't agree at all about which benchmarks were affected.

All of which is consistent with noise, IMO.

[est31]

Yeah it's noise. inflate shows up as slight improvement in the first benchmark, and is now supposedly the biggest regression.