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

libSystem.IO.Compression.Native size increased 15% #39598

Closed
jkotas opened this issue Jul 18, 2020 · 19 comments
Closed

libSystem.IO.Compression.Native size increased 15% #39598

jkotas opened this issue Jul 18, 2020 · 19 comments

Comments

@jkotas
Copy link
Member

jkotas commented Jul 18, 2020

Context: #39281

  • libSystem.IO.Compression.Native.so in .NET 3.1 is 771 KB
  • libSystem.IO.Compression.Native.so in .NET 5 is 900 KB

This is 15% increase even though we have not added any new compression algorithms.

Is this increase expected?

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-System.IO.Compression untriaged New issue has not been triaged by the area owner labels Jul 18, 2020
@jkotas jkotas removed the untriaged New issue has not been triaged by the area owner label Jul 18, 2020
@jkotas jkotas added this to the 5.0.0 milestone Jul 18, 2020
@jkotas jkotas added the tenet-performance Performance related issue label Jul 18, 2020
@vitek-karas
Copy link
Member

vitek-karas commented Jul 20, 2020

The other native binaries from libraries regressed in similar fashion:

netcoreapp3.1 (KB) net5.0 (KB)
libSystem.Security.Cryptography.Native.OpenSsl.so 115 160
libSystem.Net.Security.Native.so 15 19
libSystem.Native.so 64 93
libSystem.IO.Compression.Native.so 771 900
Total 965 1172

That's 207KB / 21% regression.

@ghost
Copy link

ghost commented Jul 21, 2020

Tagging subscribers to this area: @safern, @ViktorHofer
Notify danmosemsft if you want to be subscribed.

@danmoseley
Copy link
Member

@ViktorHofer @jkoritzinsky did anything relevant change in how we build these, since 3.1?

@danmoseley
Copy link
Member

@vitek-karas is it possible to crack the so's with some tool (similar to dumpbin on Windows) to get an idea where the size increase was? Of course it might be across the board, eg., different compilation flags.

@danmoseley
Copy link
Member

OK now I see the linked issue and I'm confused. @jkotas is it the case that @janvorli 's fix fixed the other 3, but not (for some reason) compression? And @vitek-karas's numbers happened to be without that fix?

@jkotas
Copy link
Member Author

jkotas commented Jul 21, 2020

@janvorli fix affected libcoreclr.so and libclrjit.so only. It did not touch the libraries native shims. You should be able to still see the regression in latest nightly builds.

is it possible to crack the so's with some tool

objdump is Linux equivalent of dumpbin.

@am11
Copy link
Member

am11 commented Jul 23, 2020

Stats from macOS (ignoring the lib prefixes additions to all libs in net5 and mutually exclusive binaries), there is only one regression in libhostpolicy.dylib, all the rest of native libraries have gotten size reduction:

Library Name netcoreapp3.1 (KB) net5.0 (KB)
libhostfxr.dylib 400 396
System.IO.Compression.Native.dylib 928 900
System.Native.dylib 92 72
System.Net.Security.Native.dylib 48 16
System.Security.Cryptography.Native.Apple.dylib 80 52
System.Security.Cryptography.Native.OpenSsl.dylib 140 132
libclrjit.dylib 2732 2572
libcoreclr.dylib 7232 4668
libdbgshim.dylib 820 624
libhostpolicy.dylib 344 388
libmscordaccore.dylib 2792 2516
libmscordbi.dylib 2108 1904
Total
17716 14240

@ViktorHofer
Copy link
Member

@ViktorHofer @jkoritzinsky did anything relevant change in how we build these, since 3.1?

Not that I'm aware of. cc @jkoritzinsky @janvorli

@jkoritzinsky
Copy link
Member

Not to my knowledge.

@janvorli
Copy link
Member

janvorli commented Aug 3, 2020

This change might be related:
#685

@janvorli
Copy link
Member

janvorli commented Aug 4, 2020

Hmm, so the #685 is not related. The size before and after that change is the same.

@janvorli
Copy link
Member

janvorli commented Aug 4, 2020

I've found that the reason for the growth here is the compiler. Compiling native shared libraries with clang 3.9 gets the same size as in .NET Core 3.1.
Comparing the 3.1 and 5.0 builds, I can see that especially the Brotli functions sizes have grown significantly:

3.1
0000000000009040 l     F .text  000000000000104b              CreateBackwardReferencesNH2
000000000000a090 l     F .text  00000000000013df              CreateBackwardReferencesNH3
0000000000016d80 l     F .text  0000000000001ca0              CreateBackwardReferencesNH35
000000000000b470 l     F .text  0000000000001521              CreateBackwardReferencesNH4
0000000000010500 l     F .text  0000000000001c58              CreateBackwardReferencesNH40
0000000000012160 l     F .text  0000000000001d42              CreateBackwardReferencesNH41
0000000000013eb0 l     F .text  0000000000001ed0              CreateBackwardReferencesNH42
000000000000c9a0 l     F .text  0000000000001e29              CreateBackwardReferencesNH5
0000000000015d80 l     F .text  0000000000000ff9              CreateBackwardReferencesNH54
0000000000018a20 l     F .text  0000000000001939              CreateBackwardReferencesNH55
000000000000e7d0 l     F .text  0000000000001d2a              CreateBackwardReferencesNH6
000000000001a360 l     F .text  0000000000002af9              CreateBackwardReferencesNH65

5.0
000000000000a710 l     F .text  00000000000016d1              CreateBackwardReferencesNH2
000000000000bdf0 l     F .text  0000000000001e2b              CreateBackwardReferencesNH3
00000000000249a0 l     F .text  0000000000002983              CreateBackwardReferencesNH35
000000000000dc20 l     F .text  0000000000003332              CreateBackwardReferencesNH4
0000000000017850 l     F .text  000000000000439d              CreateBackwardReferencesNH40
000000000001bbf0 l     F .text  0000000000003094              CreateBackwardReferencesNH41
000000000001ec90 l     F .text  00000000000031db              CreateBackwardReferencesNH42
0000000000010f60 l     F .text  000000000000340e              CreateBackwardReferencesNH5
0000000000021e70 l     F .text  0000000000002b21              CreateBackwardReferencesNH54
0000000000027330 l     F .text  0000000000003702              CreateBackwardReferencesNH55
0000000000014370 l     F .text  00000000000034dc              CreateBackwardReferencesNH6
000000000002aa40 l     F .text  00000000000045ba              CreateBackwardReferencesNH65

My guess is that clang 9 that we use to build .NET 5.0 unrolls loops in a more aggressive manner.

@jkotas
Copy link
Member Author

jkotas commented Aug 4, 2020

I think we should measure the Brotli performance with and without the loop unrolling, and disable the loop unrolling optimizations if they do not make material difference.

I have noticed that clang loop unrolling optimizations are very code-bloating and I have doubts about their effectiveness. We may want to disable them for coreclr as well. I suspect they are one of the contributors for why coreclr binaries are so much bigger on Linux.

@Anipik
Copy link
Contributor

Anipik commented Aug 11, 2020

@janvorli would u be able to take a lead on this one ?

@janvorli
Copy link
Member

I've measured performance with and without loop unrolling using the Brotli benchmarks. The impact of not unrolling was significant, between 6..9% for half of the benchmarks in the suite. All the results can be seen in the following gist:
https://gist.github.com/janvorli/d4b10e4a10a7adf1e79847e8a0351152

@janvorli
Copy link
Member

I've also looked at the other native shared libraries produced by the libraries build. Interestingly a significant part of the regression is caused by symbols. In 3.1, we were stripping all unneeded symbols, in 5.0, after unification of the stripping for all the three formerly separated repos, we are stripping only debugging symbols.

@ViktorHofer
Copy link
Member

Will be fixed by #41039.

@danmoseley
Copy link
Member

@janvorli is this bug fixed now? Unless you're waiting on #41986 ? Maybe we can close this?

@janvorli
Copy link
Member

Yes, it is fixed, I have not realized I have not put the issue closing tag on the PR fixing it.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

9 participants