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

Switch to the new Mono.Unix nuget #5971

Merged
merged 1 commit into from
Jun 10, 2021
Merged

Conversation

grendello
Copy link
Contributor

@grendello grendello commented May 28, 2021

Xamarin.Android uses the Mono's Mono.Posix assembly on Unix machines
to perform tasks not possible with BCL classes, provided so far by the
Mono.Posix.NETStandard nuget. Unfortunately, that nuget doesn't work
with dotnet and so the Mono.Posix code has recently been extracted
from the Mono repository and placed in its own, from which the new
Mono.Unix nuget is built and published.

This commit switches to Mono.Unix in Xamarin.Android and also updates
a number of submodules which likewise migrated from
Mono.Posix.NETStandard to Mono.Unix

Additionally, LibZipSharp is updated as well, since it now uses
Mono.Unix

Mono.Unix no longer uses the older libMonoPosixHelper dynamic
library, replaced by a new libMono.Unix one. This change, however,
broke a number of tests since the Mono.Unix.dll assembly was no longer
able to find its companion native shared library. The reason is that
Mono.Posix.NETStandard used the "standard" helper library which was
also part of the Mono distribution that we use on all the Unix machines.
However, that meant the helper library from Mono.Posix.NETStandard was
never used, instead the Mono copy was loaded and everything worked as
expected.

Mono.Unix's new native helper library, however, must be taken from the
nuget and both Mono and dotnet runtimes must be told where from to load
the library once a p/invoke into it is encountered in managed code. The
native library is copied from the nuget to the referencing application's
output directory and it should be loaded from there. This proved to be
easy for the "legacy" Mono - a simple dllmap configuration is provided
and everything works as it should.

With dotnet however, dllmap doesn't work. dotnet has instead a
number (5 I think) mechanisms to configure where the native libraries
can be found. Unfortunately, the mechanisms either require that a main
executable of the application calls the APIs on entry (e.g. in the
Main) method or that a JSON configuration file is provided for the
application, telling the runtime where the native libraries reside. In
case of Xamarin.Android.Build.Tasks there is no main executable we can
configure, since it works in the msbuild context, providing tasks and
utilities to build Xamarin.Android apps. In such instance, dotnet
could be persuaded to find the libraries by calling one of the 5 APIs.
The problem with this approach, however, is that this action would have
to be performed at every possible entry point to the
Xamarin.Android.Build.Tasks assembly, since any of them could be used
as the first one. While certainly possible, it would be both fragile
and an unnecessary maintenance burden. Instead, a simpler (albeit a bit
kludgy) solution was chosen. The Xamarin.Android.Build.Tasks build
process now takes care of generating a fat (multi-architecture) binary
for macOS hosts (including x86-64 and arm64 architectures) using the
lipo utility, then it copies the resulting binary to the same
directory where Xamarin.Android.Build.Tasks.dll and Mono.Unix.dll
live. The Linux shared library is also copied to the same location.
The dotnet runtime is able to find and load shared libraries that are
in the same directory as the assembly that needs them and everything
works as expected.

@grendello
Copy link
Contributor Author

Note that it uses a branch of xamarin-android-tools, we probably need to update Java.Interop to use that branch as well? Or can it wait till we merge this PR?

@grendello
Copy link
Contributor Author

The many tests fail because dotnet is unable to find libMono.Unix.{so,dylib}:

"/Users/runner/work/1/s/bin/TestRelease/temp/BuildAfterAddingNuget/UnnamedProject.csproj" (Build;SignAndroidPackage target) (1:7) ->
(_CompileJava target) -> 
  Xamarin.Android.Common.targets(1725,3): error XAJVC7000: System.DllNotFoundException: Unable to load shared library 'Mono.Unix' or one of its dependencies. In order to help diagnose loading problems, consider setting the DYLD_PRINT_LIBRARIES environment variable: dlopen(libMono.Unix, 1): image not found [/Users/runner/work/1/s/bin/TestRelease/temp/BuildAfterAddingNuget/UnnamedProject.csproj]
  Xamarin.Android.Common.targets(1725,3): error XAJVC7000:    at Mono.Unix.Native.Syscall.lstat(String file_name, Stat& buf) [/Users/runner/work/1/s/bin/TestRelease/temp/BuildAfterAddingNuget/UnnamedProject.csproj]
  Xamarin.Android.Common.targets(1725,3): error XAJVC7000:    at Xamarin.Tools.Zip.Utilities.StatFile(String path, Boolean followSymlinks, Stat& sbuf) in /Users/runner/work/1/s/LibZipSharp/Xamarin.Tools.Zip/Utilities.Unix.cs:line 59 [/Users/runner/work/1/s/bin/TestRelease/temp/BuildAfterAddingNuget/UnnamedProject.csproj]
  Xamarin.Android.Common.targets(1725,3): error XAJVC7000:    at Xamarin.Tools.Zip.Utilities.GetFileType(String path, Boolean followSymlinks, FilePermissions& fileType) in /Users/runner/work/1/s/LibZipSharp/Xamarin.Tools.Zip/Utilities.Unix.cs:line 148 [/Users/runner/work/1/s/bin/TestRelease/temp/BuildAfterAddingNuget/UnnamedProject.csproj]
  Xamarin.Android.Common.targets(1725,3): error XAJVC7000:    at Xamarin.Tools.Zip.UnixPlatformServices.IsFileOfType(String path, FilePermissions mode, Boolean& result) in /Users/runner/work/1/s/LibZipSharp/Xamarin.Tools.Zip/UnixPlatformServices.cs:line 429 [/Users/runner/work/1/s/bin/TestRelease/temp/BuildAfterAddingNuget/UnnamedProject.csproj]
  Xamarin.Android.Common.targets(1725,3): error XAJVC7000:    at Xamarin.Tools.Zip.UnixPlatformServices.IsDirectory(ZipArchive archive, String path, Boolean& result) in /Users/runner/work/1/s/LibZipSharp/Xamarin.Tools.Zip/UnixPlatformServices.cs:line 41 [/Users/runner/work/1/s/bin/TestRelease/temp/BuildAfterAddingNuget/UnnamedProject.csproj]
  Xamarin.Android.Common.targets(1725,3): error XAJVC7000:    at Xamarin.Tools.Zip.PlatformServices.<>c__DisplayClass9_0.<IsDirectory>b__0(IPlatformServices services) in /Users/runner/work/1/s/LibZipSharp/Xamarin.Tools.Zip/PlatformServices.cs:line 74 [/Users/runner/work/1/s/bin/TestRelease/temp/BuildAfterAddingNuget/UnnamedProject.csproj]
  Xamarin.Android.Common.targets(1725,3): error XAJVC7000:    at Xamarin.Tools.Zip.PlatformServices.CallServices(Func`2 code) in /Users/runner/work/1/s/LibZipSharp/Xamarin.Tools.Zip/PlatformServices.cs:line 166 [/Users/runner/work/1/s/bin/TestRelease/temp/BuildAfterAddingNuget/UnnamedProject.csproj]
  Xamarin.Android.Common.targets(1725,3): error XAJVC7000:    at Xamarin.Tools.Zip.PlatformServices.IsDirectory(ZipArchive archive, String path) in /Users/runner/work/1/s/LibZipSharp/Xamarin.Tools.Zip/PlatformServices.cs:line 74 [/Users/runner/work/1/s/bin/TestRelease/temp/BuildAfterAddingNuget/UnnamedProject.csproj]
  Xamarin.Android.Common.targets(1725,3): error XAJVC7000:    at Xamarin.Tools.Zip.ZipArchive.AddFile(String sourcePath, String archivePath, EntryPermissions permissions, CompressionMethod compressionMethod, Boolean overwriteExisting) in /Users/runner/work/1/s/LibZipSharp/Xamarin.Tools.Zip/ZipArchive.cs:line 398 [/Users/runner/work/1/s/bin/TestRelease/temp/BuildAfterAddingNuget/UnnamedProject.csproj]
  Xamarin.Android.Common.targets(1725,3): error XAJVC7000:    at Xamarin.Android.Tasks.ZipArchiveEx.AddFileAndFlush(String filename, Int64 fileLength, String archiveFileName, CompressionMethod compressionMethod) in Xamarin.Android.Build.Tasks:token 0x6001743+0xe [/Users/runner/work/1/s/bin/TestRelease/temp/BuildAfterAddingNuget/UnnamedProject.csproj]
  Xamarin.Android.Common.targets(1725,3): error XAJVC7000:    at Xamarin.Android.Tasks.ZipArchiveEx.AddFiles(String folder, String folderInArchive, CompressionMethod method) in Xamarin.Android.Build.Tasks:token 0x6001744+0x0 [/Users/runner/work/1/s/bin/TestRelease/temp/BuildAfterAddingNuget/UnnamedProject.csproj]
  Xamarin.Android.Common.targets(1725,3): error XAJVC7000:    at Xamarin.Android.Tasks.ZipArchiveEx.AddDirectory(String folder, String folderInArchive, CompressionMethod method) in Xamarin.Android.Build.Tasks:token 0x6001746+0x94 [/Users/runner/work/1/s/bin/TestRelease/temp/BuildAfterAddingNuget/UnnamedProject.csproj]
  Xamarin.Android.Common.targets(1725,3): error XAJVC7000:    at Xamarin.Android.Tasks.Javac.RunTask() in Xamarin.Android.Build.Tasks:token 0x6001134+0x46 [/Users/runner/work/1/s/bin/TestRelease/temp/BuildAfterAddingNuget/UnnamedProject.csproj]
  Xamarin.Android.Common.targets(1725,3): error XAJVC7000:    at Microsoft.Android.Build.Tasks.AndroidToolTask.Execute() in /Users/builder/azdo/_work/1/s/xamarin-android/external/xamarin-android-tools/src/Microsoft.Android.Build.BaseTasks/AndroidToolTask.cs:line 15 [/Users/runner/work/1/s/bin/TestRelease/temp/BuildAfterAddingNuget/UnnamedProject.csproj]

And there doesn't seem to be a good way to make dotnet locate the required shared libraries. Here's a summary of the options available to make it work and problems that are associated with it. I'm going to paste here what I wrote on Discord, should the link no longer work:

hello gentle people, I'm running into the situation described here dotnet/msbuild#1887
however, AssemblyDependencyResolver doesn't appear to be the right answer (at least for me)
in Xamarin.Android we package quite a number of assemblies in a directory outside msbuild root
one of these is the new Mono.Unix (a.k.a. Mono.Posix) assembly which depends on a native helper
the native helper is bundled into Mono.Unix nuget and when we generate Xamarin.Android net6 workflow nuget, the runtimes are packaged along with the rest

https://gist.github.com/grendello/5a1f796b17cff10ba7f7d0e8ba0c0310#file-gistfile1-txt-L184-L186 - this is the tree of the nuget
runtimes are here: https://gist.github.com/grendello/5a1f796b17cff10ba7f7d0e8ba0c0310#file-gistfile1-txt-L217-L223
and the assembly which uses Mono.Unix.dll is here https://gist.github.com/grendello/5a1f796b17cff10ba7f7d0e8ba0c0310#file-gistfile1-txt-L255
both Mono.Unix.dll and Xamarin.Android.Build.Tasks.dll have a plethora of possible entry points, which makes AssemblyDependencyResolver quite awkward to use - each of the entry points would have to also be a point where AssemblyDependencyResolver is initialized

one of my thoughts was to use runtimeconfig.json with appropriate APP_NI_PATHS property in there but that rises two questions:

  1. where to put runtimeconfig.json so that the main msbuild process reads it (and where it doesn't conflict with other potential runtimeconfig.json files other packages/apps want to place there)
  2. What path to put in APP_NI_PATHS - the config file would probably need to be generated on installation time, as I doubt APP_NI_PATHS supports paths relative to path of the assembly which depends on the native libraries
    in mono/mono there's a beautiful mechanism of dllmap, which supports paths relative to the assembly for this particular purpose (that's the Mono.Unix.dll.config in the above gist (https://github.com/mono/mono.posix/blob/main/src/Mono.Unix/Mono.Unix.dll.config for reference what's inside it), but dllmap was deemed unnecessary in dotnet, alas, and here we are...

Xamarin.Android requires Mono.Unix to work, because its predecessor Mono.Posix.NETStandard doesn't work with dotnet, and this is something that prevents us from building Xamarin.Android with dotnet
there's also https://docs.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.nativelibrary.setdllimportresolver?view=net-5.0 this mechanism, but the problem with this one is similar to AssemblyDependencyResolver in that it must be initialized by the first entry point to the assembly depending on native libraries (or the main application executable, which doesn't really exist in any practical shape here)

@grendello grendello force-pushed the new-mono.posix branch 3 times, most recently from b5737fc to ea3075b Compare June 1, 2021 06:48
@dellis1972
Copy link
Contributor

This looks like a much simpler approach :D 👍 👍 on and have a 🦖 too cos 🦖 make everything better :D

@grendello grendello changed the title [WIP] Use the new Mono.Unix nuget Switch to the new Mono.Unix nuget Jun 8, 2021
@grendello grendello marked this pull request as ready for review June 8, 2021 16:06
@grendello grendello requested a review from directhex as a code owner June 8, 2021 16:06
@jonpryor
Copy link
Member

jonpryor commented Jun 8, 2021

monodroid diff: https://github.com/xamarin/monodroid/compare/76c04cd15eca7afc269a6d26296e9d2db6f79be2...1f2ce156245ef1bf63ec8014882d283b3224216b

What's potentially odd is that the Legacy .apk size increased by ~86KB: https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=4849982&view=ms.vss-test-web.build-test-results-tab&runId=22423592&resultId=100032&paneView=debug

Size difference in bytes ([*1] apk1 only, [*2] apk2 only):
  +       5,848 lib/arm64-v8a/libmonodroid.so
  +         132 assemblies/mscorlib.dll
  +          60 classes.dex
  +          39 assemblies/Java.Interop.dll
  +          19 assemblies/System.Core.dll
  +          14 assemblies/UnnamedProject.dll
  +           7 assemblies/System.dll
  +           1 assemblies/Mono.Android.dll
  -      12,592 lib/arm64-v8a/libmonosgen-2.0.so
Summary:
  +           0 Other entries 0.00% (of 58,022)
  +          60 Dalvik executables 0.02% (of 316,700)
  +         212 Assemblies 0.02% (of 1,123,258)
  -       6,744 Shared libraries -0.10% (of 6,724,872)
  +      86,016 Package size difference 2.17% (of 3,962,580)

I believe that "Package size difference" means that the new .apk is bigger, but it's not specifically attributable to anything in particular. Which is troubling; this implies that the .apkdesc file won't help us.

My guess is that the .apkdesc files track the uncompressed contents of entries within the .apk. Why would the new .apk be bigger? Because the files aren't being compressed as much as they were before.

I need to figure out how to validate this guess.

@jonpryor
Copy link
Member

jonpryor commented Jun 9, 2021

Verification? I installed current main, 11.3.99.58: https://artprodscussu1.artifacts.visualstudio.com/A011b8bdf-6d56-4f87-be0d-0092136884d9/0bdbc590-a062-4c3f-b0f6-9383f67865ee/_apis/artifact/cGlwZWxpbmVhcnRpZmFjdDovL2RldmRpdi9wcm9qZWN0SWQvMGJkYmM1OTAtYTA2Mi00YzNmLWIwZjYtOTM4M2Y2Nzg2NWVlL2J1aWxkSWQvNDg0OTg4Mi9hcnRpZmFjdE5hbWUvaW5zdGFsbGVycy11bnNpZ25lZA2/content?format=file&subPath=%2Fxamarin.android-11.3.99.58.pkg

I then built Xamarin.Forms-Performance-Integration:

% cd tests/Xamarin.Forms-Performance-Integration/Droid
% msbuild /restore /t:SignAndroidPackage /p:Configuration=Release

I also downloaded the (hopefully) equivalent .apk from this PR build: https://artprodscussu1.artifacts.visualstudio.com/A011b8bdf-6d56-4f87-be0d-0092136884d9/0bdbc590-a062-4c3f-b0f6-9383f67865ee/_apis/artifact/cGlwZWxpbmVhcnRpZmFjdDovL2RldmRpdi9wcm9qZWN0SWQvMGJkYmM1OTAtYTA2Mi00YzNmLWIwZjYtOTM4M2Y2Nzg2NWVlL2J1aWxkSWQvNDg0OTk4Mi9hcnRpZmFjdE5hbWUvVGVzdCtSZXN1bHRzKy0rQVBLcytMZWdhY3krLSttYWNPUys1/content?format=file&subPath=%2FDefault%2FXamarin.Forms_Performance_Integration-Signed.apk

"Success", apkdiff is kinda/sorta "similar":

% ~/.dotnet/tools/apkdiff ../../../bin/TestRelease/Xamarin.Forms_Performance_Integration-Signed.apk ~/Downloads/Xamarin.Forms_Performance_Integration-Signed.ap
Size difference in bytes ([*1] apk1 only, [*2] apk2 only):
  +          52 assemblies/Mono.Android.dll
  +           4 assemblies/Xamarin.AndroidX.Lifecycle.LiveData.Core.dll
  +           4 assemblies/System.Drawing.Common.dll
…
  -           1 assemblies/Plugin.Connectivity.Abstractions.dll
  -           1 assemblies/Xamarin.AndroidX.CardView.dll
  -           1 assemblies/Xamarin.Forms.Core.dll
  -           1 assemblies/Xamarin.Forms.Platform.Android.dll
…
Summary:
  +           0 Other entries 0.00% (of 901,874)
  +           0 Dalvik executables 0.00% (of 2,467,280)
  -           4 Assemblies -0.00% (of 5,584,243)
  +           0 Shared libraries 0.00% (of 13,661,104)
  +     192,512 Package size difference 1.50% (of 12,809,568)

The 195KB "Package size difference" is not explained by any entry differences. This looks "similar enough".

What about the .apk contents?

At this point I cast about for tools which will tell me offsets of data, on the assumption that the "extra size" may be due to inserted padding between entries, or something. I come across zipdetails, and find a difference, though I don't understand it:

# zipdetails -v ../../../bin/TestRelease/Xamarin.Forms_Performance_Integration-Signed.apk # main
…
09C7EA 0DF07F ...         PAYLOAD

17B869 000004 50 4B 03 04 LOCAL HEADER #239     04034B50
17B86D 000001 0A          Extract Zip Spec      0A '1.0'
17B86E 000001 00          Extract OS            00 'MS-DOS'
17B86F 000002 00 00       General Purpose Flag  0000
17B871 000002 00 00       Compression Method    0000 'Stored'
17B873 000004 22 9E C8 52 Last Mod Time         52C89E22 'Tue Jun  8 19:49:04 2021'
17B877 000004 8F FD 75 09 CRC                   0975FD8F
17B87B 000004 DE C0 01 00 Compressed Length     0001C0DE
17B87F 000004 DE C0 01 00 Uncompressed Length   0001C0DE
17B883 000002 3A 00       Filename Length       003A
17B885 000002 0B 00       Extra Length          000B
17B887 00003A 61 73 73 65 Filename              'assemblies/Xamarin.Forms.Performance
              6D 62 6C 69                       .Integration.Droid.dll'
…
17B8C1 000002 55 54       Extra ID #0001        5455 'UT: Extended Timestamp'
17B8C3 000002 05 00         Length              0005
17B8C5 000001 01            Flags               '01 mod'
17B8C6 000004 F0 01 C0 60   Mod Time            60C001F0 'Tue Jun  8 19:49:04 2021'
17B8CA 01C0DE ...         PAYLOAD
1979A8 000002 4B 50       UNEXPECTED PADDING    KP


Unexpecded END at offset 001979AA, value 04030000
Done
# zipdetails -v ~/Downloads/Xamarin.Forms_Performance_Integration-Signed.apk # PR#5971
…
09C7EA 0E9870 ...         PAYLOAD

18605A 000004 50 4B 03 04 LOCAL HEADER #239     04034B50
18605E 000001 0A          Extract Zip Spec      0A '1.0'
18605F 000001 00          Extract OS            00 'MS-DOS'
186060 000002 00 00       General Purpose Flag  0000
186062 000002 00 00       Compression Method    0000 'Stored'
186064 000004 31 B4 C8 52 Last Mod Time         52C8B431 'Tue Jun  8 22:33:34 2021'
186068 000004 F7 1E 46 16 CRC                   16461EF7
18606C 000004 D8 C0 01 00 Compressed Length     0001C0D8
186070 000004 D8 C0 01 00 Uncompressed Length   0001C0D8
186074 000002 3A 00       Filename Length       003A
186076 000002 0A 00       Extra Length          000A
186078 00003A 61 73 73 65 Filename              'assemblies/Xamarin.Forms.Performance
              6D 62 6C 69                       .Integration.Droid.dll'
…
1860B2 000002 55 54       Extra ID #0001        5455 'UT: Extended Timestamp'
1860B4 000002 05 00         Length              0005
1860B6 000001 01            Flags               '01 mod'
1860B7 000004 3C F0 BF 60   Mod Time            60BFF03C 'Tue Jun  8 18:33:32 2021'
1860BB 01C0D8 ...         PAYLOAD
1A2193 000001 03          UNEXPECTED PADDING    .


Unexpecded END at offset 001A2194, value 0400504B
Done

It appears, via zipdetails, that everything is "similar enough" (except for timestamp) up through offset 09C7EA. In zipdetails -v output:

   If the "-v" option is present, column 1 is expanded to include
   o    The offset from the start of the file in hex.
   o    The length of the filed in hex.
   o    A hex dump of the bytes in field in the order they are stored in the zip file.

This is where things confuse me: the length of the field [sic] at 09C7EA is 0x0DF07F for main, 0x0E9870 for this PR. What is the payload at 09C7EA? It's classes.dex:

09C7B8 000004 50 4B 03 04 LOCAL HEADER #238     04034B50
09C7BC 000001 14          Extract Zip Spec      14 '2.0'
09C7BD 000001 00          Extract OS            00 'MS-DOS'
09C7BE 000002 02 00       General Purpose Flag  0002
                          [Bits 1-2]            2 'Fast Compression'
09C7C0 000002 08 00       Compression Method    0008 'Deflated'
09C7C2 000004 22 9E C8 52 Last Mod Time         52C89E22 'Tue Jun  8 19:49:04 2021'
09C7C6 000004 F7 86 DA 4F CRC                   4FDA86F7
09C7CA 000004 7F F0 0D 00 Compressed Length     000DF07F
09C7CE 000004 D0 A5 25 00 Uncompressed Length   0025A5D0
09C7D2 000002 0B 00       Filename Length       000B
09C7D4 000002 09 00       Extra Length          0009
09C7D6 00000B 63 6C 61 73 Filename              'classes.dex'
              73 65 73 2E
              64 65 78
09C7E1 000002 55 54       Extra ID #0001        5455 'UT: Extended Timestamp'
09C7E3 000002 05 00         Length              0005
09C7E5 000001 01            Flags               '01 mod'
09C7E6 000004 EF 01 C0 60   Mod Time            60C001EF 'Tue Jun  8 19:49:03 2021'

Yes classes.dex has the same size, as per unzip -l:

# main
% unzip -lv ../../../bin/TestRelease/Xamarin.Forms_Performance_Integration-Signed.apk | grep classes.dex
 2467280  Defl:X   913535  63% 06-08-2021 19:49 4fda86f7  classes.dex

# pr#5971
% unzip -lv ~/Downloads/Xamarin.Forms_Performance_Integration-Signed.apk | grep classes.dex
 2467280  Defl:X   956528  61% 06-08-2021 22:33 4fda86f7  classes.dex

…the same uncompressed size, but different compressed sizes! I think we're onto something here: the compressed size of classes.dex is ~42KB larger with PR#5971 than main.

Is classes.dex any different?

# main
% unzip ../../../bin/TestRelease/Xamarin.Forms_Performance_Integration-Signed.apk classes.dex
% mv classes.dex classes-main.dex

# pr#5971
% unzip ~/Downloads/Xamarin.Forms_Performance_Integration-Signed.apk classes.dex

% diff classes-main.dex classes.dex
# no difference

It looks like the new libZipSharp compresses classes.dex less well than the previous version.

@grendello
Copy link
Contributor Author

It seems the difference in classes.dex compressed size stems from different deflate algorithm configuration between the "old" zlib (as used by the previous versions of LibZipSharp) and zlib-ng used by the new version of LibZipSharp. The default compression level for both libraries is 6, however in case of zlib-ng that is a "medium" compression, while for zlib it's "slow" (that is better). This zlib-ng issue also explains it a bit. It would appear that if we care more about the compression ratio rather than compression speed, we could use a higher compression level with the new LibZipSharp (it should still be faster than zlib on supported CPUs).

@grendello
Copy link
Contributor Author

So, I checked whether using compression level 9 would make a difference and it did not. classes.dex simply compresses worse with zlib-ng than with zlib. So the question is - do we want to take the size hit on this (and potentially similar) files in favor of speed, or do we want to revert LibZipSharp back to the standard zlib and get the better compression back?

@grendello
Copy link
Contributor Author

I will create a new version of LibZipSharp with some further tweaks to see whether we can get the old compression ratio back with zlib-ng

grendello added a commit to dotnet/android-libzipsharp that referenced this pull request Jun 9, 2021
Context: dotnet/android#5971 (comment)

Also, attempt to tweak `zlib-ng` to use the best compression possible,
by sacrificing speed.
grendello added a commit to dotnet/android-libzipsharp that referenced this pull request Jun 9, 2021
Context: dotnet/android#5971 (comment)

Also, attempt to tweak `zlib-ng` to use the best compression possible,
by sacrificing speed.
@jonpryor
Copy link
Member

jonpryor commented Jun 9, 2021

@grendello asked:

do we want to take the size hit on this (and potentially similar) files in favor of speed, or do we want to revert LibZipSharp back to the standard zlib and get the better compression back?

I'm not sure.

A few additional things that worry me:

  1. The compressed size difference for classes.dex is 42KB. The entire .apk is 195KB. It's not just classes.dex.
  2. Is libZipSharp used to separately compress assemblies as well? Native libraries?
  3. It "feels like" the size increase will be somewhat proportional to the size of classes.dex and assemblies. This isn't just a "oh, classes.dex is 42KB larger", it'll be a "classes.dex is X% larger". That really starts getting into "unknown" territory.

Back on (2), if I run a "side-by-side" comparison of unzip -lv output of the .apk files, I see that many of the assemblies have different sizes (main / pr):

  114910  Stored   114910   0% 06-08-2021 19:49 0975fd8f  assemblies/Xamarin.Forms.Performance.Integration.Droid.dll
  114904  Stored   114904   0% 06-08-2021 22:33 16461ef7  assemblies/Xamarin.Forms.Performance.Integration.Droid.dll

Xamarin.Forms.Performance.Integration.Droid.dll is smaller (by 6 bytes). Yay. Spot-checking, most of the assemblies are slightly smaller, actually.

Then we hit native libs (main vs pr):

  135288  Defl:X    19751  85% 06-08-2021 19:49 831e5ce4  lib/armeabi-v7a/libxamarin-app.so
  135288  Defl:X    20265  85% 06-08-2021 22:33 2cca4267  lib/armeabi-v7a/libxamarin-app.so
…
 4456716  Defl:X  1775200  60% 06-08-2021 19:49 ff4870fc  lib/armeabi-v7a/libmonosgen-2.0.so
 4456716  Defl:X  1832692  59% 06-08-2021 22:33 ff4870fc  lib/armeabi-v7a/libmonosgen-2.0.so

libxamarin-app.so is 514 bytes larger now. libmonosgen-2.0.so is 57.5KB larger.

The more I look, the more worried I am about how entries are being compressed. Returning to standard zlib increasingly looks preferrable.

Copy link
Contributor

@dellis1972 dellis1972 left a comment

Choose a reason for hiding this comment

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

This looks good once we figure out the zip issue.

grendello added a commit to dotnet/android-libzipsharp that referenced this pull request Jun 9, 2021
Context: dotnet/android#5971 (comment)
Context: dotnet/android#5971 (comment)
Context: zlib-ng/zlib-ng#850
Context: https://github.com/zlib-ng/zlib-ng/wiki/Deflate-config-comparison

`zlib-ng` is mostly about speed and efficiency, however it achieves its
goal by sometimes sacrificing the compression ratio which then affects
Xamarin.Android, the main LibZipSharp consumer.

Add a new `zlib` submodule and switch to using `zlib` by default instead
of `zlib-ng`, while retaining `zlib-ng` as an option.
grendello added a commit to dotnet/android-libzipsharp that referenced this pull request Jun 10, 2021
Context: dotnet/android#5971 (comment)
Context: dotnet/android#5971 (comment)
Context: zlib-ng/zlib-ng#850
Context: https://github.com/zlib-ng/zlib-ng/wiki/Deflate-config-comparison

`zlib-ng` is mostly about speed and efficiency, however it achieves its
goal by sometimes sacrificing the compression ratio which then affects
Xamarin.Android, the main LibZipSharp consumer.

Add a new `zlib` submodule and switch to using `zlib` by default instead
of `zlib-ng`, while retaining `zlib-ng` as an option.
grendello added a commit to dotnet/android-libzipsharp that referenced this pull request Jun 10, 2021
Context: dotnet/android#5971 (comment)
Context: dotnet/android#5971 (comment)
Context: zlib-ng/zlib-ng#850
Context: https://github.com/zlib-ng/zlib-ng/wiki/Deflate-config-comparison

`zlib-ng` is mostly about speed and efficiency, however it achieves its
goal by sometimes sacrificing the compression ratio which then affects
Xamarin.Android, the main LibZipSharp consumer.

Add a new `zlib` submodule and switch to using `zlib` by default instead
of `zlib-ng`, while retaining `zlib-ng` as an option.
`Xamarin.Android` uses the Mono's Mono.Posix assembly on Unix machines
to perform tasks not possible with BCL classes, provided so far by the
`Mono.Posix.NETStandard` nuget.  Unfortunately, that nuget doesn't work
with `dotnet` and so the `Mono.Posix` code has recently been extracted
from the Mono repository and placed in its own, from which the new
`Mono.Unix` nuget is built and published.

This commit switches to `Mono.Unix` in Xamarin.Android and also updates
a number of submodules which likewise migrated from
`Mono.Posix.NETStandard` to `Mono.Unix`

Additionally, LibZipSharp is updated as well, since it now uses
`Mono.Unix`

`Mono.Unix` no longer uses the older `libMonoPosixHelper` dynamic
library, replaced by a new `libMono.Unix` one.  This change, however,
broke a number of tests since the `Mono.Unix.dll` assembly was no longer
able to find its companion native shared library.  The reason is that
`Mono.Posix.NETStandard` used the "standard" helper library which was
also part of the Mono distribution that we use on all the Unix machines.
However, that meant the helper library from `Mono.Posix.NETStandard` was
never used, instead the Mono copy was loaded and everything worked as
expected.

`Mono.Unix`'s new native helper library, however, must be taken from the
nuget and both Mono and dotnet runtimes must be told where from to load
the library once a p/invoke into it is encountered in managed code.  The
native library is copied from the nuget to the referencing application's
output directory and it should be loaded from there.  This proved to be
easy for the "legacy" Mono - a simple `dllmap` configuration is provided
and everything works as it should.

With `dotnet` however, dllmap doesn't work.  `dotnet` has instead a
number (5 I think) mechanisms to configure where the native libraries
can be found.  Unfortunately, the mechanisms either require that a main
executable of the application calls the APIs on entry (e.g. in the
`Main`) method or that a JSON configuration file is provided for the
application, telling the runtime where the native libraries reside.  In
case of `Xamarin.Android.Build.Tasks` there is no main executable we can
configure, since it works in the msbuild context, providing tasks and
utilities to build Xamarin.Android apps.  In such instance, `dotnet`
could be persuaded to find the libraries by calling one of the 5 APIs.
The problem with this approach, however, is that this action would have
to be performed at **every** possible entry point to the
`Xamarin.Android.Build.Tasks` assembly, since any of them could be used
as the first one.  While certainly possible, it would be both fragile
and an unnecessary maintenance burden.  Instead, a simpler (albeit a bit
kludgy) solution was chosen.  The `Xamarin.Android.Build.Tasks` build
process now takes care of generating a fat (multi-architecture) binary
for macOS hosts (including `x86-64` and `arm64` architectures) using the
`lipo` utility, then it copies the resulting binary to the same
directory where `Xamarin.Android.Build.Tasks.dll` and `Mono.Unix.dll`
live.  The Linux shared library is also copied to the same location.
The `dotnet` runtime is able to find and load shared libraries that are
in the same directory as the assembly that needs them and everything
works as expected.
@jonpryor
Copy link
Member

Use Mono.Unix NuGet package

Changes: https://github.com/xamarin/monodroid/compare/76c04cd15eca7afc269a6d26296e9d2db6f79be2...1f2ce156245ef1bf63ec8014882d283b3224216b

  * xamarin/monodroid@1f2ce1562: [tools/msbuild] only run _GetPrimaryCpuAbi for Fast Dev (#1208)
  * xamarin/monodroid@691310ede: Bump android-sdk-installer to use Mono.Unix (#1207)
  * xamarin/monodroid@48843fcb2: [tools/msbuild] <GetPrimaryCpuAbi/> selects backup RIDs for .NET 6 (#1205)
  * xamarin/monodroid@4af48f54b: [tools/fastdev] Add error checking when writing data to disk. (#1204)
  * xamarin/monodroid@65b7b2dd4: [tools/fastdev] Rework Unix timestamp calculation code in xamarin.find. (#1202)
  * xamarin/monodroid@401f170e9: [build] fix `dotnet tool install` command (#1203)
  * xamarin/monodroid@a879b250b: Bump to xamarin/xamarin-android@032d840, xamarin/androidtools@355d015 (#1199)
  * xamarin/monodroid@9f9ee378c: [tools/msbuild] Missing translations for XA0135 (#1198)

Changes: https://github.com/xamarin/xamarin-android-tools/compare/683f37508b56c76c24b3287a5687743438625341...49936d60ce808d706f9e4c1ace71c2fe9c604733

  * xamarin/xamarin-android-tools@49936d6: Merge pull request #124 from xamarin/update-libzipsharp
  * xamarin/xamarin-android-tools@ef78dfc: Bump LibZipSharp to 2.0.0-alpha6
  * xamarin/xamarin-android-tools@e3d708c: [BaseTasks] fix `\`-delimited paths on macOS (#122)
  * xamarin/xamarin-android-tools@bdcf899: Reference the new Mono.Unix nuget (#123)
  * xamarin/xamarin-android-tools@90d7621: [BaseTasks] add ABI detection for RIDs (#121)
  * xamarin/xamarin-android-tools@79e3b97: [JdkInfo] handle invalid XML from /usr/libexec/java_home (#120)
  * xamarin/xamarin-android-tools@81519fe: Add SECURITY.md (#119)


Xamarin.Android uses the Mono's Mono.Posix assembly on Unix machines
to perform tasks not possible with BCL classes, provided by the
[`Mono.Posix.NETStandard` NuGet][0].  The `Mono.Posix` source has been
extracted into the [mono/mono.posix repo][1], which is used to build
the new [`Mono.Unix` NuGet package][2].

Update the xamarin/xamarin-android repo -- and various dependencies --
to use the `Mono.Unix` package instead of `Mono.Posix.NETStandard`.
This includes the [`Xamarin.LibZipSharp` NuGet][3], as of
xamarin/LibZipSharp@cf5e33c6.

`Mono.Unix` no longer uses the older `libMonoPosixHelper` dynamic
library, replaced by a new `libMono.Unix` native library.
Unfortunately, this change broke a number of tests since the
`Mono.Unix.dll` assembly was no longer able to find its companion
native shared library.  While the `Mono.Posix.NETStandard` NuGet
package provides the `libMonoPosixHelper` native library, in practice
the *actual* `libMonoPosixHelper` that was used was the "system"
library included with the system mono.

`Mono.Unix`'s new native helper library, however, must be taken from
the NuGet and both the Mono and dotnet runtimes must be told where to
load the library from once a P/Invoke into `Mono.Unix` is encountered
in managed code.  The native library is copied from the NuGet to the
referencing application's output directory and it should be loaded from
there.  This proved to be easy for the "legacy" Mono: a simple
[`dllmap` configuration][4] and everything works as it should.

With `dotnet` however, dllmap doesn't work.  `dotnet` has instead a
number of mechanisms to configure where the native libraries can be
found (5 I think).  Unfortunately, the mechanisms either require that
a main executable of the application calls the APIs on entry (e.g. in
the `Main()` method) or that a JSON configuration file is provided for
the application, telling the runtime where the native libraries reside.
In case of `Xamarin.Android.Build.Tasks` there is no main executable we
can configure, since it works in the MSBuild context, providing tasks
and utilities to build Xamarin.Android apps.  In this instance, `dotnet`
could be persuaded to find the libraries by calling one of the 5 APIs.
The problem with this approach, however, is that this action would have
to be performed at **every** possible entry point to the
`Xamarin.Android.Build.Tasks` assembly, since any of them could be used
as the first one.  While certainly possible, it would be both fragile
and an unnecessary maintenance burden.  Instead, a simpler (albeit a
bit kludgy) solution was chosen: the `src/Xamarin.Android.Build.Tasks`
build process now takes care of generating a fat (multi-architecture)
binary for macOS hosts (including `x86-64` and `arm64` architectures)
using the `lipo` utility, then it copies the resulting binary to the
same directory where `Xamarin.Android.Build.Tasks.dll` and
`Mono.Unix.dll` live.  The Linux shared library is also copied to the
same location.  The `dotnet` runtime is able to find and load shared
libraries that are in the same directory as the assembly that needs
them and everything works as expected.

[0]: https://www.nuget.org/packages/Mono.Posix.NETStandard/5.20.1-preview
[1]: https://github.com/mono/mono.posix
[2]: https://www.nuget.org/packages/Mono.Unix/
[3]: https://www.nuget.org/packages/Xamarin.LibZipSharp
[4]: https://www.mono-project.com/docs/advanced/pinvoke/dllmap/

@jonpryor jonpryor merged commit ceca993 into dotnet:main Jun 10, 2021
@grendello grendello deleted the new-mono.posix branch June 10, 2021 18:57
@github-actions github-actions bot locked and limited conversation to collaborators Jan 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants