Deduplicate LSDA data in the object writer

[MichalStrehovsky]

Extracting a piece of #87045 that I had to revert in that PR. Native linkers don't like when LSDA is in a COMDAT so fold these in the object writer instead. Seems to save about 1.2% in the Stage1 app. Obviously Unix only.

Cc @dotnet/ilc-contrib

[ghost]

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Extracting a piece of #87045 that I had to revert in that PR. Native linkers don't like when LSDA is in a COMDAT so fold these in the object writer instead. Seems to save about 1.2% in the Stage1 app. Obviously Unix only.

I only tested it with the Stage1 app so opening as draft so that I don't embarrass myself if the CI is all red.

Author:	MichalStrehovsky
Assignees:	-
Labels:	area-NativeAOT-coreclr
Milestone:	-

[filipnavara]

This can be folded into the previous if on line 217. It was originally split to support prepending additional data in the LSDA section but that's solved in a different way for ARM32 now.

[MichalStrehovsky]

@filipnavara what's the preferred way to exclude this for Mach-O? Mach-O seems to have opinions (or bugs) about this so I'd disable this there.

[MichalStrehovsky]

/azp run runtime-nativeaot-outerloop

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[filipnavara]

what's the preferred way to exclude this for Mach-O?

No strong preferences. I would probably use the same pattern as UseFrameNames. Maybe we can even key it off the same property since I assume the underlying reason for the failure is similar.

[MichalStrehovsky]

what's the preferred way to exclude this for Mach-O?

No strong preferences. I would probably use the same pattern as UseFrameNames. Maybe we can even key it off the same property since I assume the underlying reason for the failure is similar.

From a quick look at https://opensource.apple.com/source/ld64/ld64-609/src/ld/parsers/macho_relocatable_file.cpp and the assert in question (cfiStartsArray[i] != cfiStartsArray[i-1]), it looks like ld64 puts LSDAs in the same collection as FDEs and then we hit an assert that is supposed to guard FDEs are unique. The FDEs are unique. LSDAs are not...

		// scan for FDEs claming the same function
		for(uint32_t i=1; i < cfiStartsArrayCount; ++i) {
			assert( cfiStartsArray[i] != cfiStartsArray[i-1] );
		}

UseFrameNames sounds good. It can just mean "Apple weirdness".

[MichalStrehovsky]

/azp run runtime-nativeaot-outerloop

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[am11]

Seems to save about 1.2% in the Stage1 app

👍
For awareness, would it make sense to add a size test (next to dwarfdump one) so every time some observable reduction/regression is introduced, we see it in the CI and adjust the limits accordingly?

AssertInRange(new FileInfo(Environment.ProcessPath).Length, GetLowerUpperLimitsForCurrentOS())

[filipnavara]

👍 For awareness, would it make sense to add a size test (next to dwarfdump one) so every time some observable reduction/regression is introduced, we see it in the CI and adjust the limits accordingly?

It’s hard to separate noise and improvements on this. There’s a dashboard that tracks it over time - https://aka.ms/aspnet/nativeaot/benchmarks - and possibly a system that auto-files issues on large changes.

[MichalStrehovsky]

Seems to save about 1.2% in the Stage1 app

👍 For awareness, would it make sense to add a size test (next to dwarfdump one) so every time some observable reduction/regression is introduced, we see it in the CI and adjust the limits accordingly?

AssertInRange(new FileInfo(Environment.ProcessPath).Length, GetLowerUpperLimitsForCurrentOS())

We have a "is size in range" test that has a big range that prevents the most egregious regressions (like if someone drags in the reflection stack into a hello world):

runtime/src/tests/nativeaot/SmokeTests/HardwareIntrinsics/Program.cs

Lines 17 to 31 in 85b5eab

	Console.WriteLine("****************************************************");
	Console.WriteLine("* Size test *");
	long fileSize = new System.IO.FileInfo(Environment.ProcessPath).Length;
	Console.WriteLine($"* Size of the executable is {fileSize / 1024,7:n0} kB *");
	Console.WriteLine("****************************************************");
	long lowerBound, upperBound;
	lowerBound = 1300 * 1024; // ~1.3 MB
	upperBound = 1750 * 1024; // ~1.75 MB
	if (fileSize < lowerBound || fileSize > upperBound)
	{
	Console.WriteLine($"BUG: File size is not in the expected range ({lowerBound} to {upperBound} bytes). Did a libraries change regress size of Hello World?");
	return 1;
	}

And like Filip said, the rest is checked elsewhere.

[jkotas]

Thanks

[eerhardt]

I can confirm we are seeing 1.3% and 1.7% size on disk reduction in Stage1 and Stage2 AOT apps in our benchmarks. (3% if you include symbol size).

dotnet/aspnetcore#53906
dotnet/aspnetcore#53907
dotnet/aspnetcore#53908
dotnet/aspnetcore#53909