More precise CPU instruction set specification for ASP.NET composite #47207
Conversation
Based on offline feedback I have modified build of the ASP.NET composite image to explicitly specify the following instruction set extensions per Tanner's suggestion: avx2 bmi bmi2 lzcnt movbe fma With this change, the composite image is about 500 KB longer - before the change its length was 33_392_640 B, after the change it's 33_832_960. As we're now working with the partial composite that reduced the previous publishing size by about 30~40 MB, hopefully this shouldn't be much of a concern. Thanks Tomas
trylek
added
the
area-infrastructure
|
Hey @dotnet/aspnet-build, looks like this PR is something you want to take a look at. |
|
LGTM! |
|
Hmm, apparently I'll need to conditionally filter the CPU set extensions per architecture, I'll fix this in the next commit. |
| @@ -489,6 +489,7 @@ This package is an internal implementation of the .NET Core SDK and is not meant | |||
| <ManagedAssetsFullPath>$(RuntimePackageRoot)$(ManagedAssetsPackagePath)</ManagedAssetsFullPath> | |||
| <NativeAssetsFullPath>$(RuntimePackageRoot)$(NativeAssetsPackagePath)</NativeAssetsFullPath> | |||
| <PartialCompositeAssemblyListPath>$(MSBuildThisFileDirectory)\PartialCompositeAssemblyList.txt</PartialCompositeAssemblyListPath> | |||
| <InstructionSetSupport Condition="'$(TargetArchitecture)' == 'x64' or '$(TargetArchitecture)' == 'x86'">+avx2,+bmi,+bmi2,+lzcnt,+movbe,+fma</InstructionSetSupport> | |||
There was a problem hiding this comment.
Why these instead of the simpler +x86-x64-v3 that is supported and means the same thing: https://github.com/dotnet/runtime/blob/main/src/coreclr/tools/Common/JitInterface/CorInfoInstructionSet.cs#L990
If you're going to specify them explicitly, then you're missing +popcnt.
There was a problem hiding this comment.
Thanks Tanner for pointing that out, I have updated the specification in the 4th commit based on your suggestion.
ghost
added this to the
| @@ -489,6 +489,7 @@ This package is an internal implementation of the .NET Core SDK and is not meant | |||
| <ManagedAssetsFullPath>$(RuntimePackageRoot)$(ManagedAssetsPackagePath)</ManagedAssetsFullPath> | |||
| <NativeAssetsFullPath>$(RuntimePackageRoot)$(NativeAssetsPackagePath)</NativeAssetsFullPath> | |||
| <PartialCompositeAssemblyListPath>$(MSBuildThisFileDirectory)\PartialCompositeAssemblyList.txt</PartialCompositeAssemblyListPath> | |||
| <InstructionSetSupport Condition="'$(TargetArchitecture)' == 'x64' or '$(TargetArchitecture)' == 'x86'">+x86-x64-v3</InstructionSetSupport> | |||
|
would be good to know the size differences with this change. |
|
Hi @mangod9. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context. |
|
@mangod9 - I believe I have documented the size diff in the PR description, please let me know if more data is needed. For arm64, I have no idea, the instruction set extensions you pointed out on the Teams thread based on Tanner's suggestions all seem to be Intel-specific. |
|
Hi @trylek. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context. |
|
Ah sorry missed that section in the description. Looks reasonable. Thanks |
|
Hi @mangod9. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context. |
Based on offline feedback I have modified build of the ASP.NET composite image to explicitly specify the following instruction set extensions per Tanner's suggestion:
avx2 bmi bmi2 lzcnt movbe fma
With this change, the composite image is about 500 KB longer - before the change its length was 33_392_640 B, after the change it's 33_832_960. As we're now working with the partial composite that reduced the previous publishing size by about 30~40 MB, hopefully this shouldn't be much of a concern.
Thanks
Tomas
/cc @dotnet/crossgen-contrib