Revert of #79968 to test perf regression in #80867

[rylev]

DO NOT MERGE

Revert "Rollup merge of #79968 - bjorn3:better_drop_glue_debuginfo, r=matthewjasper"

This is a revert of #79968 to test whether it was responsible for performance regressions in the rollup #80867.

This reverts commit f90c7f0, reversing
changes made to 5c0f5b6.

r? @ghost

[rylev]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

[bors]

⌛ Trying commit 26be83b with merge 3fb3412359cce85c7e01cdbcee536cfbbbfec74e...

[bors]

☀️ Try build successful - checks-actions
Build commit: 3fb3412359cce85c7e01cdbcee536cfbbbfec74e (3fb3412359cce85c7e01cdbcee536cfbbbfec74e)

[rust-timer]

Queued 3fb3412359cce85c7e01cdbcee536cfbbbfec74e with parent fc9944f, future comparison URL.

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

[rust-timer]

Finished benchmarking try commit (3fb3412359cce85c7e01cdbcee536cfbbbfec74e): 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
@rustbot label: +S-waiting-on-review -S-waiting-on-perf

[bjorn3]

#79968 was indeed the cause of the perf regression. Interestingly, LLVM optimizations are what slowed down and not the linker invocation as I expected.

[Mark-Simulacrum]

I think this is pretty expected? There's probably slightly more work for LLVM/codegen to do as there's more functions and with longer names/debug info attached.

[bjorn3]

There should be exactly as much functions, just with a longer symbol name in some cases.

[rylev]

@bjorn3 what's your opinion on whether the change is worth the perf hit? I'm assuming there's not much we can do to gain that performance back since the desired change is exactly the reason for the slowdown in codegen.

1% is certainly a non-trivial perf hit albeit the lowest amount we usually register as an actual perf regression. I don't have good insight into the amount of utility of having the dropped type's name in the debug output for drop_in_place.

[bjorn3]

I'm assuming there's not much we can do to gain that performance back since the desired change is exactly the reason for the slowdown in codegen.

Yeah, this is LLVM getting slower, not any code we control.

1% is certainly a non-trivial perf hit albeit the lowest amount we usually register as an actual perf regression. I don't have good insight into the amount of utility of having the dropped type's name in the debug output for drop_in_place.

I agree that the perf hit is unfortunate. I do think that this is a useful change. The whole reason to have -Zsymbol-mangling-scheme=v0 is to add the generic parameters for all functions. drop_in_place is a special case where previously the symbol name was completely useless, but -Zsymbol-mangling-scheme=v0 will have an even bigger perf hit. It may be nice to get a perf run for -Zsymbol-mangling-scheme=v0 I guess.

[nagisa]

On one hand nice and useful symbol names help debugging and have benefits in some other use-cases. On the other hand most of the users likely don't care about them at all and would likely rather if, by default, they were unreadable but resulted in faster compile and link times.

Now, the goal of the mangling schemes we utilized so far has always been to provide utility and performance implications were never of much importance in this particular area, so I suggest we continue to focus on utility for the schemes that we currently have. And then a mangling scheme better suited for faster compiles could be considered and implemented as a separate proposal.

[rylev]

I'm going to close this as it seems that the consensus is that the performance degradation is worth getting the better debugging output.