Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ObjWriter in C# #95876

Merged
merged 144 commits into from
Jan 8, 2024
Merged

ObjWriter in C# #95876

merged 144 commits into from
Jan 8, 2024

Conversation

filipnavara
Copy link
Member

@filipnavara filipnavara commented Dec 11, 2023

This is reimplementation of NativeAOT ObjWriter in pure C# instead of depending on LLVM. It implements Mach-O, ELF, COFF object file emitter with DWARF and CodeView debugging information. Only x64 and arm64 targets are implemented to cover officially supported platforms (win-x86 code is present and lightly tested; linux-x86 code is present and incomplete, only serves as a test bed for emitting 32-bit ELF files if we ever need that).

Original object writer code is still present and can be used by setting the DOTNET_USE_LLVM_OBJWRITER=1 environment variable.

Thanks to @am11 for helping with testing and debugging this, @xoofx for making LibObjectFile which helped kickstart this project, @PaulusParssinen for tips about branchless U/SLEB128 size calculation, and all the people on the .NET team who helped push this forward!

Fixes #77178

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Dec 11, 2023
@ghost
Copy link

ghost commented Dec 11, 2023

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

Issue Details

This is reimplementation of NativeAOT ObjWriter in pure C# instead of depending on LLVM. It implements Mach-O, ELF, COFF object file emitter with DWARF and CodeView debugging information. Only x64 and arm64 targets are implemented to cover officially supported platforms (win-x86 code is present and lightly tested; linux-x86 code is present and incomplete, only serves as a test bed for emitting 32-bit ELF files if we ever need that).

Fixes #77178

Author: filipnavara
Assignees: -
Labels:

area-NativeAOT-coreclr

Milestone: -

@NCLnclNCL
Copy link

What is this, it is ilc.exe ??

@filipnavara
Copy link
Member Author

What is this, it is ilc.exe ??

Yes. It replaces the object file format writing of the ILC backend.

@xoofx
Copy link
Member

xoofx commented Dec 13, 2023

On that topic, dunno if ilc is already today using NativeAOT, but I assume that this PR should make it a lot more lightweight for this scenario, correct?

@filipnavara
Copy link
Member Author

On that topic, dunno if ilc is already today using NativeAOT, but I assume that this PR should make it a lot more lightweight for this scenario, correct?

ILC itself is published as NativeAOT executable. It's not compiled with the in-tree compiler though so it takes a little while till the bits propagate through the dependency pipeline into the SDK and back, but it's self-hosted.

It makes it slightly more lightweight in some ways. I expect the memory usage to reduce, although it's not trivial to compare with both the GC and native heap contributing to it. Performance measurements on smoke tests were favorable. It also simplifies the build process and dependency handling, especially for the full .NET source builds.

@am11
Copy link
Member

am11 commented Dec 13, 2023

Yup, ilc is published as a NativeAOT app.

@MichalStrehovsky
Copy link
Member

Is there any code/method left from the prototype that used existing NuGets? (Asking whether we need to add entry to THIRD-PARTY-NOTICES.TXT.)

@filipnavara
Copy link
Member Author

Is there any code/method left from the prototype that used existing NuGets?

The algorithm for SizeOfULEB128 originally comes from https://android.googlesource.com/platform/art/+/ba257bc/runtime/leb128.h#101. It's two lines of code so that would very likely fall into fair use.

The only other thing that indirectly came from 3rd party sources were the constants in ElfNative.cs, MachNative.cs, and DwarfNative.cs. They were mostly taken from the specs where possible but few of them mirror the system headers.

@NCLnclNCL
Copy link

Is there any code/method left from the prototype that used existing NuGets?

The algorithm for SizeOfULEB128 originally comes from https://android.googlesource.com/platform/art/+/ba257bc/runtime/leb128.h#101. It's two lines of code so that would very likely fall into fair use.

The only other thing that indirectly came from 3rd party sources were the constants in ElfNative.cs, MachNative.cs, and DwarfNative.cs. They were mostly taken from the specs where possible but few of them mirror the system headers.

So what about compilation time, is it faster?

@huoyaoyuan
Copy link
Member

So what about compilation time, is it faster?

You can see more information in the linked issue #77178. Yes it's faster.

@jtschuster
Copy link
Member

Should the outputs of this and the LLVM object writer be identical byte for byte? I'm seeing different sizes on the same commit using each object writer for linux-x64.

@filipnavara
Copy link
Member Author

Should the outputs of this and the LLVM object writer be identical byte for byte?

TL;DR: No.

There are differences in the order of sections, string table order, temporary local symbols in symbol table, DWARF relocations (.debug_info -> .debug_info relocations were not correctly generated in the original ObjWriter), and DWARF compile unit headers.

Throughout the development I used a mode where I matched up the output more closely but it’s not an efficient thing to do and it was only useful for the initial debugging.

I switched later on to comparing the object files semantically through readelf, objdump, dwarfdump and other tools.

Aside from debugging information the linked output should be byte for byte identical.

@TIHan
Copy link
Contributor

TIHan commented Dec 15, 2023

This is wonderful @filipnavara

@MichalStrehovsky MichalStrehovsky merged commit abffa8f into dotnet:main Jan 8, 2024
110 checks passed
@am11
Copy link
Member

am11 commented Jan 9, 2024

This is available in latest daily build of installer: 9.0.100-alpha.1.24059.1

@filipnavara, just noticed that on win-x64, the object size is smaller with managed implementation, but final executable size regresses.

> Set-Alias -Name dotnet9 -Value ~/.dotnet9/dotnet
> dotnet9 --version
9.0.100-alpha.1.24059.1

> dotnet9 new webapiaot -n web1
> cd web1

# managed
> dotnet9 publish -c Release -o dist
> wsl du -b dist/web1.exe obj/Release/net9.0/win-x64/native/web1.obj

9504768 dist/web1.exe
51874490        obj/Release/net9.0/win-x64/native/web1.obj

# native
> rm -r -fo obj
> $env:DOTNET_USE_LLVM_OBJWRITER="1"
> dotnet9 publish -c Release -o dist2
> wsl du -b dist2/web1.exe obj/Release/net9.0/win-x64/native/web1.obj

9241088 dist2/web1.exe
55142434        obj/Release/net9.0/win-x64/native/web1.obj

worth tracking?

@NCLnclNCL
Copy link

This is available in latest daily build of installer: 9.0.100-alpha.1.24059.1

@filipnavara, just noticed that on win-x64, the object size is smaller with managed implementation, but final executable size regresses.

> Set-Alias -Name dotnet9 -Value ~/.dotnet9/dotnet
> dotnet9 --version
9.0.100-alpha.1.24059.1

> dotnet9 new webapiaot -n web1
> cd web1

# managed
> dotnet9 publish -c Release -o dist
> wsl du -b dist/web1.exe obj/Release/net9.0/win-x64/native/web1.obj

9504768 dist/web1.exe
51874490        obj/Release/net9.0/win-x64/native/web1.obj

# native
> rm -r -fo obj
> $env:DOTNET_USE_LLVM_OBJWRITER="1"
> dotnet9 publish -c Release -o dist2
> wsl du -b dist2/web1.exe obj/Release/net9.0/win-x64/native/web1.obj

9241088 dist2/web1.exe
55142434        obj/Release/net9.0/win-x64/native/web1.obj

worth tracking?

What advantages does it have over llvm?

@filipnavara
Copy link
Member Author

worth tracking?

Definitely. Can you file an issue please? I'll look into it.

@filipnavara
Copy link
Member Author

What advantages does it have over llvm?

Faster compilation speeds at lower memory usage.

@NCLnclNCL
Copy link

worth tracking?

Definitely. Can you file an issue please? I'll look into it.

i saw newest version of ilc.exe is 9.0.0-alpha.1.24058.11 and it had objwriter.dll

@filipnavara
Copy link
Member Author

i saw newest version of ilc.exe is 9.0.0-alpha.1.24058.11 and it had objwriter.dll

There's still a DOTNET_USE_LLVM_OBJWRITER switch to use the old ObjWriter. It's an escape hatch that will likely be removed before final release.

@eerhardt
Copy link
Member

@filipnavara, just noticed that on win-x64, the object size is smaller with managed implementation, but final executable size regresses.

We are seeing this in ASP.NET NativeAOT benchmarks. Not only win-x64 (~2%), but for both linux-x64 and for linux-arm64 (~4-6%). (Percent size regressions are for app size + symbols.)

@filipnavara
Copy link
Member Author

We are seeing this in ASP.NET NativeAOT benchmarks. Not only win-x64 (~2%), but for both linux-x64 and for linux-arm64 (~4-6%). (Percent size regressions are for app size + symbols.)

Are the win-x64 numbers with #96686? That solved the regression in winapiaot that was reported by @am11 and I believe it should solve the reported regressions in the ASP.NET on win-x64 too.

Do you have any hints on what to build to repro the size regression on linux-x64/arm64? I can take a look this week.

@filipnavara
Copy link
Member Author

Percent size regressions are for app size + symbols

Note that on Linux specifically there's a regression in symbol size in DWARF format inside the .o file because correct relocations are emitted for the .debug_info section. Depending on how the sizes are calculated that would be a possible explanation. Technically fixing the relocations is a bug fix.

@eerhardt
Copy link
Member

It looks like the win-x64 did go back down after a few builds, but not all the way back:

image

image

image

Do you have any hints on what to build to repro the size regression on linux-x64/arm64?

dotnet publish /p:PublishAot=true from https:/aspnet/Benchmarks/tree/main/src/BenchmarksApps/BasicMinimalApi (using the latest runtime versions).

@agocke
Copy link
Member

agocke commented Jan 10, 2024

Isn't the .debug_info section removed after stripping?

@filipnavara
Copy link
Member Author

Isn't the .debug_info section removed after stripping?

It is. I specifically mentioned that because of the "+ symbols". I am not sure how the benchmarks are setup to deal with the symbols so depending on the configuration it may have some effect.

@eerhardt
Copy link
Member

We explicitly pass /p:StripSymbols=true because that wasn't the default when we first made the benchmarks. You can see the full crank command here:

https:/aspnet/Benchmarks/blob/5e7b7eba2a9710eb79fba7a50f3ed6646fc5547c/scenarios/goldilocks.benchmarks.yml#L15-L21

https:/aspnet/Benchmarks/blob/5e7b7eba2a9710eb79fba7a50f3ed6646fc5547c/scenarios/goldilocks.benchmarks.yml#L60-L66

@filipnavara

This comment was marked as outdated.

@filipnavara
Copy link
Member Author

filipnavara commented Jan 10, 2024

win-x64 / .NET SDK 9.0.100-alpha.1.24060.16
Running dotnet publish /p:PublishAot=true /p:StripSymbols=true /p:EnableRequestDelegateGenerator=true (as suggested in BuildingLocally.md). Changing the TargetFramework in .csproj to net9.0.

With DOTNET_USE_LLVM_OBJWRITER=1 (legacy ObjWriter):
BasicMinimalApi.obj 58,848,928
BasicMinimalApi.exe 9,441,792
BasicMinimalApi.pdb 50,319,360

With DOTNET_USE_LLVM_OBJWRITER=0 (new ObjWriter):
BasicMinimalApi.obj 57,694,649
BasicMinimalApi.exe 9,441,792
BasicMinimalApi.pdb 49,434,624

In conclusion, I don't see a size regression on win-x64 with the latest SDK build downloaded from dotnet/installer. The executable size and .pdb size is entirely identical. This is with just flipping the ObjWriter code path and no other unrelated changes. The size is 9.00 MiB which is consistent with the dashboard size. Perhaps the additional regression from 8.81 MiB to 9.00 MiB is caused by some other change?

@filipnavara
Copy link
Member Author

linux-x64 / .NET SDK 9.0.100-alpha.1.24060.24
Running dotnet publish /p:PublishAot=true /p:StripSymbols=true /p:EnableRequestDelegateGenerator=true (as suggested in BuildingLocally.md). Changing the TargetFramework in .csproj to net9.0.

With DOTNET_USE_LLVM_OBJWRITER=1 (legacy ObjWriter):
-rwxr-xr-x 1 navara navara 10543248 Jan 10 22:18 BasicMinimalApi
-rwxr-xr-x 1 navara navara 22841240 Jan 10 22:18 BasicMinimalApi.dbg
-rw-r--r-- 1 navara navara 58250008 Jan 10 22:18 BasicMinimalApi.o

With DOTNET_USE_LLVM_OBJWRITER=0 (new ObjWriter):
-rwxr-xr-x 1 navara navara 10543248 Jan 10 22:16 BasicMinimalApi
-rwxr-xr-x 1 navara navara 23158256 Jan 10 22:16 BasicMinimalApi.dbg
-rw-r--r-- 1 navara navara 61157625 Jan 10 22:15 BasicMinimalApi.o

Again, there's not regression in executable size by just switching the ObjWriter code. I manually verified the sections of the .o files to make sure the correct ObjWriter was used (a quirk makes the name of the hydrated section prefixed with . when the new ObjWriter is used and unprefixed with the old ObjWriter; this is easy to check with readelf --sections). As expected the size of the .o file is slightly bigger due to the additional .debug_info relocations. There's a 1.3% size increase of the .dbg DWARF symbol file. While notable it does not look like the cause of the reported regression.

@filipnavara
Copy link
Member Author

For posterity, the data above for both of the platforms were produced with this Benchmarks commit.

@MichalStrehovsky
Copy link
Member

I have seen a regression when using LLVM linker (after linking) in the eh_frames section. I didn't look into details but the difference in the executable looks like this:

image

I don't know what linker ends up the official benchmark using.

@eerhardt
Copy link
Member

I can get the mstat files between the 2 benchmark runs, if that helps anything? Are there other files that would help identify the regression?

@filipnavara
Copy link
Member Author

I don't know what linker ends up the official benchmark using.

You may be onto something here. I am testing on Ubuntu 22.04. The binlog file shows -fuse-ld=bfd so I assume that means the binutils linker.

The relevant versions on the system are:

  • ld: GNU ld (GNU Binutils for Ubuntu) 2.38
  • ld.lld: Ubuntu LLD 14.0.0 (compatible with GNU linkers)

I'll try the LLVM linker and specifically check the .eh_frame section and its padding.

@filipnavara
Copy link
Member Author

linux-x64, same as above, just with lld linker:

DOTNET_USE_LLVM_OBJWRITER=0:
-rwxr-xr-x 1 navara navara 10598384 Jan 10 22:46 BasicMinimalApi
-rwxr-xr-x 1 navara navara 23679200 Jan 10 22:46 BasicMinimalApi.dbg

DOTNET_USE_LLVM_OBJWRITER=1:
-rwxr-xr-x 1 navara navara 10598376 Jan 10 22:47 BasicMinimalApi
-rwxr-xr-x 1 navara navara 23869464 Jan 10 22:47 BasicMinimalApi.dbg

There's difference to the size from the BFD linker but the difference between the old/new ObjWriter is only 8 bytes.

@eerhardt
Copy link
Member

One thing I forgot to mention is that the app as checked in is targeting net8.0. You need to change the TFM to net9.0. crank does that for us automatically.

@filipnavara
Copy link
Member Author

One thing I forgot to mention is that the app as checked in is targeting net8.0. You need to change the TFM to net9.0.

Yeah, I figured that out after the first set of bogus numbers in #95876 (comment).

I have seen a regression when using LLVM linker (after linking) in the eh_frames section.

Seems like the specification say to align to address size, which in our case would be 4-bytes, and we always align to 8-bytes. I'll need to check this thoroughly. I'd expect to see this difference between the local builds with DOTNET_USE_LLVM_OBJWRITER=0/1 and I never noticed it before.

@eerhardt
Copy link
Member

eerhardt commented Jan 10, 2024

Getting the mstat files helped here. Looks like there was some changes in LINQ that might be causing the regression, and I just saw this "ObjWriter" change with comments saying there was a size regression.

image

Here are the mstat files for anyone who is curious:

BasicMinimalApi.zip

Sorry for the false alarm here, @filipnavara.

UPDATE: I believe the regression is from #96570 (comment).

@filipnavara
Copy link
Member Author

Sorry for the false alarm here, @filipnavara.

No worries... I am happy for the extra scrutiny of this PR.

It actually helped bring a light to the .eh_frame over-alignment issue. I'm now 95% convinced that the issue is real and in this particular case contributes up to ~53,736 bytes to the object file size (before linking). Thanks @MichalStrehovsky! I'll submit a PR to fix this.

@am11
Copy link
Member

am11 commented Jan 11, 2024

.NET SDK 9.0.100-alpha.1.24060.24

Installer build is out for this SDK as well. Just replaced ~/.dotnet9 with new bits and it fixed the win-x64 regression (smaller .obj and .pdb, identical .exe). Thanks for the quick fix! :)

-fuse-ld=bfd/lld

nit: prefer using -p:LinkerFlavor=bfd/lld, it does a little more than setting fuse-ld.

@filipnavara filipnavara deleted the objwriter3 branch January 24, 2024 19:07
@github-actions github-actions bot locked and limited conversation to collaborators Feb 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-NativeAOT-coreclr community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Discussion: ObjWriter in C#