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

Add a simple size test #81517

Merged
merged 4 commits into from
Feb 7, 2023
Merged

Conversation

MichalStrehovsky
Copy link
Member

Resolves #80165.

The size of Hello World is currently 1.8 MB. This is achieved by avoiding any non-field reflection in the codepaths that are part of a hello world. If reflection comes into picture, the size jumps by several 100 kB because suddently we need a lot of code to support that. This is a smoke test to detect those situations. We're getting proper size testing in the dotnet/performance repo with trend histories, etc., but the invariant for Hello World is easy to check for and nice to gate commits on.

I chose the HwIntrinsics tests as a "victim" to host this because I don't want to introduce a new test for this.

Cc @dotnet/ilc-contrib

The size of Hello World is currently 1.8 MB. This is achieved by avoiding any non-field reflection in the codepaths that are part of a hello world. If reflection comes into picture, the size jumps by several 100 kB because suddently we need a lot of code to support that. This is a smoke test to detect those situations. We're getting proper size testing in the dotnet/performance repo with trend histories, etc., but the invariant for Hello World is easy to check for and nice to gate commits on.
@ghost
Copy link

ghost commented Feb 2, 2023

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

Issue Details

Resolves #80165.

The size of Hello World is currently 1.8 MB. This is achieved by avoiding any non-field reflection in the codepaths that are part of a hello world. If reflection comes into picture, the size jumps by several 100 kB because suddently we need a lot of code to support that. This is a smoke test to detect those situations. We're getting proper size testing in the dotnet/performance repo with trend histories, etc., but the invariant for Hello World is easy to check for and nice to gate commits on.

I chose the HwIntrinsics tests as a "victim" to host this because I don't want to introduce a new test for this.

Cc @dotnet/ilc-contrib

Author: MichalStrehovsky
Assignees: -
Labels:

area-NativeAOT-coreclr

Milestone: -

@MichalStrehovsky
Copy link
Member Author

Hmm, the test infra is producing a 3.6 MB executable for ~Hello World with StripSymbols on Linux.

On my machine with Ubuntu 18.04, this is slightly under 1.9 MB.

A difference I see is that the test infra environment chose llvm-objcopy to do stripping whereas on my Ubuntu it chose objcopy.

@am11 did you see similar differences when you were working on this?

I guess we can just give the test more budget on Linux.

@am11
Copy link
Member

am11 commented Feb 2, 2023

Just like RyuJIT, the older versions of compiler toolchains (both llvm and gcc) are less efficient; llvm lesser efficient than gcc.

The only reason we have CentOS 7 around with old glibc, libstdc++ and llvm 9 is to enhance the official (portable) linux-x64 binary compatibility (glibc is backward-compatible). For size tests, I think we should use a modern platform https://github.com/dotnet/versions/blob/841708954543e7dffb8250e0561d06f753fdb97c/build-info/docker/image-info.dotnet-dotnet-buildtools-prereqs-docker-production.json#L2412 onward (ubuntu 22.04 with clang 14 or 15 in cross containers).

A difference I see is that the test infra environment chose llvm-objcopy to do stripping whereas on my Ubuntu it chose objcopy.

Yup, llvm-objcopy wasn't that smart in v9, but in later versions, it is. If changing containers for smoke tests is not an option, then <ObjCopyName>objcopy will do the trick (override prop to force binutils' objcopy).

@ivanpovazan
Copy link
Member

FWIW: we have noticed something similar in difference between XCode toolchain versions, more particularly Xcode 14.1.0 vs Xcode 13.3.0, when tracking iOS application size with MonoAOT reported here: #79879

@MichalStrehovsky
Copy link
Member Author

Yup, llvm-objcopy wasn't that smart in v9, but in later versions, it is. If changing containers for smoke tests is not an option, then <ObjCopyName>objcopy will do the trick (override prop to force binutils' objcopy).

Thanks, giving it a shot!

@MichalStrehovsky
Copy link
Member Author

Looks like it did the trick. Digging in the CI logs, I found:

****************************************************
* Size test                                        *
* Size of the executable is   1,928 kB             *
****************************************************

if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
{
lowerBound = 2 * Meg; // 2 MB
upperBound = 4 * Meg; // 4 MB
Copy link
Member

@am11 am11 Feb 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Linux window is 1.5-2.0 MB. Should we shrink this to 3.0-3.5 MB? (it is currently somewhere between 3.0 to 3.1 on osx-arm64: 3217960 bytes / 3.06 Mebibyte with v8.0.0-preview.2.23102.7)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The OSX test will likely cross the 3.0 threshold downward soon. No point dealing with that - this is approximated by the Linux size test. OSX size is not in our 8.0 release criteria.

Copy link
Contributor

@tlakollo tlakollo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! :D

@MichalStrehovsky MichalStrehovsky merged commit b39d6a6 into dotnet:main Feb 7, 2023
@MichalStrehovsky MichalStrehovsky deleted the locksize branch February 7, 2023 02:48
@ghost ghost locked as resolved and limited conversation to collaborators Mar 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bringing down the size of Hello World under 2 MB
4 participants