[Xamarin.Android.Build.Tasks] remove the linksrc directory #3359

jonathanpeppers · 2019-07-16T21:39:46Z

I've removed various MSBuild targets that copy assemblies into the
obj\Debug\linksrc directory, and instead just use
@(ResolvedUserAssemblies) or @(ResolvedAssemblies) directly

Things seem to work, I can build and deploy samples\HelloWorld and
the Xamarin.Forms app in this repo. Let's see what breaks?

The size of tests\Xamarin.Forms-Performance-Integration\Droid\obj\
is improved dramatically:

Before:
184.27 MB
After:
111.74 MB

~73MB savings.

These MSBuild tasks were completely removed:

  3 ms  _CopyMdbFiles                              1 calls
 43 ms  _CopyPdbFiles                              1 calls
127 ms  _CopyIntermediateAssemblies                1 calls

On a slower machine with Windows Defender enabled, these targets are
taking:

  6 ms  _CopyMdbFiles                              1 calls
 95 ms  _CopyPdbFiles                              1 calls
269 ms  _CopyIntermediateAssemblies                1 calls

On the slower machine, the overall time was even more drastic:

Before:
Time Elapsed 00:00:33.41
After:
Time Elapsed 00:00:29.92

With Windows Defender processes using the CPU, the overall build time
is improved by ~3.5 seconds?

Changes for jnimarshalmethod-gen

Because jnimarshalmethod-gen edits assemblies in linksrc in-place,
before the linker, I added an extra jnisrc directory to accommodate
for this. Then when _LinkAssembliesShrink runs, it operates on
assemblies in the jnisrc directory.

We can revisit this if $(AndroidGenerateJniMarshalMethods) ever
becomes default.

jonathanpeppers · 2019-07-18T19:38:23Z

Unfortunately there is a problem here:

jnimarshalmethod-gen.exe operates on assemblies in-place in linksrc
We can't run this after the linker, because the Linker expects it to have been run: https://github.com/xamarin/xamarin-android/blob/05344f8d43293c794f3df2b533b71d339ef031f7/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/MonoDroidMarkStep.cs#L35

With the current implementation of jnimarshalmethod-gen, we need an extra copy of the .NET assemblies on disk: linksrc. Need to think on this...

src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets

Added -r --reference option to add information about processed assemblies references. Also make it possible to use response file with additional command line arguments. The file can be specified by @response_file at the command line. That might help with dotnet/android#3359

I've removed various MSBuild targets that copy assemblies into the `obj\Debug\linksrc` directory, and instead just use `@(ResolvedUserAssemblies)` or `@(ResolvedAssemblies)` where appropriate. Things seem to *work*, I can build and deploy `samples\HelloWorld` and the Xamarin.Forms app in this repo. Let's see what breaks? The size of `tests\Xamarin.Forms-Performance-Integration\Droid\obj\` is improved dramatically: Before: 184.27 MB After: 111.74 MB ~73MB savings. These MSBuild tasks were completely removed: 3 ms _CopyMdbFiles 1 calls 43 ms _CopyPdbFiles 1 calls 127 ms _CopyIntermediateAssemblies 1 calls I will need to retest this on a slower machine where I'm seeing Windows Defender have an impact on build times. ~~ Changes for jnimarshalmethod-gen ~~ Because `jnimarshalmethod-gen` edits assemblies in `linksrc` in-place, before the linker, I added an extra `jnisrc` directory to accommodate for this. The new behavior is: * `jnimarshalmethod-gen` outputs assemblies to `jnisrc`, it outputs a subset of `@(ResolvedAssemblies)`. * `_LinkAssembliesShrink` uses files form `jnisrc` if they exist and `@(ResolvedAssemblies)` otherwise. I also use the new `-r` switch, so any needed assemblies can be loaded.

jonathanpeppers · 2019-08-20T16:28:46Z

@radekdoulik after I've switched this around to use -o. I'm getting:

 Unhandled Exception:
  System.UnauthorizedAccessException: Access to the path "/Library/Frameworks/Mono.framework/External/xbuild-frameworks/MonoAndroid/v10.0/Mono.Android.dll" is denied.

https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=2956832&view=logs&jobId=8556562a-ae5f-5bd1-7c4d-bf1af4b6f1e1&taskId=5076e147-fc66-561e-6c69-3aa777afefc5&lineStart=754&lineEnd=790&colStart=3&colEnd=138

This only happens because our tests on Azure DevOps are running against a system install.

Is jnimarshalmethod-gen.exe opening this file with write access?

radekdoulik · 2019-08-20T17:13:17Z

Indeed, we were opening the assemblies with ReadWrite reader parameters even when using -o. It should be now fixed by dotnet/java-interop@d39699f

jonathanpeppers · 2019-08-21T13:08:49Z

@radekdoulik got past that one, thanks.

There is a new error though 😞:

EXEC : error : jnimarshalmethod-gen: Unable to process assembly '/Library/Frameworks/Mono.framework/External/xbuild-frameworks/MonoAndroid/v10.0/Mono.Android.dll' [/Users/vsts/agent/2.155.1/work/1/s/src/Mono.Android/Test/Mono.Android-Tests.csproj]
  Access to the path "/Library/Frameworks/Mono.framework/External/xbuild-frameworks/MonoAndroid/v10.0/Mono.Android-JniMarshalMethods.dll" is denied.
  System.UnauthorizedAccessException: Access to the path "/Library/Frameworks/Mono.framework/External/xbuild-frameworks/MonoAndroid/v10.0/Mono.Android-JniMarshalMethods.dll" is denied.
    at System.IO.FileStream..ctor (System.String path, System.IO.FileMode mode, System.IO.FileAccess access, System.IO.FileShare share, System.Int32 bufferSize, System.Boolean anonymous, System.IO.FileOptions options) [0x001b7] in <b047284b444b4c74b59771ad4346b34a>:0 
    at System.IO.FileStream..ctor (System.String path, System.IO.FileMode mode, System.IO.FileAccess access, System.IO.FileShare share, System.Int32 bufferSize, System.Boolean isAsync, System.Boolean anonymous) [0x00000] in <b047284b444b4c74b59771ad4346b34a>:0 
    at System.IO.FileStream..ctor (System.String path, System.IO.FileMode mode, System.IO.FileAccess access) [0x00000] in <b047284b444b4c74b59771ad4346b34a>:0 
    at (wrapper remoting-invoke-with-check) System.IO.FileStream..ctor(string,System.IO.FileMode,System.IO.FileAccess)
    at System.Reflection.Emit.ModuleBuilder.Save () [0x001e5] in <b047284b444b4c74b59771ad4346b34a>:0 
    at System.Reflection.Emit.AssemblyBuilder.Save (System.String assemblyFileName, System.Reflection.PortableExecutableKinds portableExecutableKind, System.Reflection.ImageFileMachine imageFileMachine) [0x0022b] in <b047284b444b4c74b59771ad4346b34a>:0 
    at System.Reflection.Emit.AssemblyBuilder.Save (System.String assemblyFileName) [0x00000] in <b047284b444b4c74b59771ad4346b34a>:0 
    at Xamarin.Android.Tools.JniMarshalMethodGenerator.App.CreateMarshalMethodAssembly (System.String path) [0x0043d] in <3859ef09ce2c46b7b24c5a9fe9afd6b4>:0 
    at Xamarin.Android.Tools.JniMarshalMethodGenerator.App.ProcessAssemblies (System.Collections.Generic.List`1[T] assemblies) [0x001b0] in <3859ef09ce2c46b7b24c5a9fe9afd6b4>:0

Is this Mono.Android-JniMarshalMethods.dll an assembly generated that should be going into the -o directory? I need also make sure it makes it into the linker step and the app?

radekdoulik · 2019-08-21T13:25:33Z

Is this Mono.Android-JniMarshalMethods.dll an assembly generated that should be going into the -o directory? I need also make sure it makes it into the linker step and the app?

Definitely, it should be generated in the directory specified by -o. I will fix it as well.

The assembly itself is temporary.

radekdoulik · 2019-08-21T13:48:25Z

dotnet/java-interop#483 should fix it.

Changes: dotnet/java-interop@60e85b0...8ed9677 Fixes for `-o` in `jnimarshalmethod-gen.exe`.

jonathanpeppers · 2019-08-22T13:23:04Z

Ok, this actually looks green now. Jenkins looks like what we see on master.

mjo151 · 2019-11-21T17:32:10Z

We just upgraded to VS 2019 and this change appears to have broken obfuscation with our Xamarin.Android builds. We were performing obfuscation in the AfterBuild target, then copying our obfuscated assemblies into the linksrc directory after the _CopyIntermediateAssemblies target. Due to this change, this no longer works. Can you provide suggestions on which build target we should use to perform obfuscation? Also, where does the linker look for assemblies (e.g. where should be copy our obfuscated assemblies)?

jonathanpeppers · 2019-11-21T22:10:26Z

@mjo151 the convention in MSBuild is that anything prefixed with an underscore is private. Meaning it might change from release to release. (MSBuild is all global variables otherwise)

https://github.com/xamarin/xamarin-android/blob/master/Documentation/guides/MSBuildBestPractices.md#naming

However, you can easily copy the files to a directory yourself and fix up the @(ResolvedAssemblies) and @(ResolvedUserAssemblies) item groups, we had to do a similar workaround for AndroidX.Migration: xamarin/XamarinAndroidXMigration#20

You can check if $(MonoAndroidLinkerInputDir) is blank to determine if it is a newer Xamarin.Android version where linksrc is gone.

jonathanpeppers force-pushed the linksrc-is-dead branch from 835f0d0 to 9bb1589 Compare July 17, 2019 20:37

jonathanpeppers force-pushed the linksrc-is-dead branch 2 times, most recently from d72750e to 04b1770 Compare August 1, 2019 20:47

jonathanpeppers force-pushed the linksrc-is-dead branch from 04b1770 to cfdaa89 Compare August 7, 2019 21:48

jonathanpeppers marked this pull request as ready for review August 7, 2019 21:49

jonathanpeppers requested review from dellis1972 and radekdoulik as code owners August 7, 2019 21:49

jonathanpeppers force-pushed the linksrc-is-dead branch 2 times, most recently from 707e52f to 47b4bf2 Compare August 9, 2019 02:15

dellis1972 reviewed Aug 12, 2019

View reviewed changes

src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets Show resolved Hide resolved

radekdoulik reviewed Aug 12, 2019

View reviewed changes

src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets Show resolved Hide resolved

jonathanpeppers force-pushed the linksrc-is-dead branch from 532db4e to 371e135 Compare August 12, 2019 16:24

radekdoulik reviewed Aug 12, 2019

View reviewed changes

src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets Outdated Show resolved Hide resolved

radekdoulik mentioned this pull request Aug 13, 2019

[jnimarshalmethod-gen] New reference option and response file dotnet/java-interop#465

Merged

jonathanpeppers force-pushed the linksrc-is-dead branch from 371e135 to 199b868 Compare August 19, 2019 20:22

Whoops

917e5c9

jonathanpeppers requested a review from jonpryor as a code owner August 20, 2019 17:14

Bump to xamarin/java.interop/master@8ed9677

8150730

Changes: dotnet/java-interop@60e85b0...8ed9677 Fixes for `-o` in `jnimarshalmethod-gen.exe`.

jonathanpeppers force-pushed the linksrc-is-dead branch from 8040bff to 8150730 Compare August 21, 2019 16:32

jonathanpeppers requested a review from radekdoulik August 22, 2019 13:23

jonathanpeppers requested a review from dellis1972 August 22, 2019 13:23

dellis1972 approved these changes Aug 22, 2019

View reviewed changes

dellis1972 merged commit 2ea31e6 into dotnet:master Aug 22, 2019

brendanzagaeski added this to the In preview: 10.1 (d16-4) milestone Sep 25, 2019

jonathanpeppers deleted the linksrc-is-dead branch November 21, 2019 22:04

github-actions bot locked and limited conversation to collaborators Jan 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Xamarin.Android.Build.Tasks] remove the linksrc directory #3359

[Xamarin.Android.Build.Tasks] remove the linksrc directory #3359

Uh oh!

jonathanpeppers commented Jul 16, 2019 •

edited

Loading

Uh oh!

jonathanpeppers commented Jul 18, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jonathanpeppers commented Aug 20, 2019 •

edited

Loading

Uh oh!

radekdoulik commented Aug 20, 2019

Uh oh!

jonathanpeppers commented Aug 21, 2019

Uh oh!

radekdoulik commented Aug 21, 2019

Uh oh!

radekdoulik commented Aug 21, 2019

Uh oh!

jonathanpeppers commented Aug 22, 2019

Uh oh!

mjo151 commented Nov 21, 2019 •

edited

Loading

Uh oh!

jonathanpeppers commented Nov 21, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

[Xamarin.Android.Build.Tasks] remove the linksrc directory #3359

[Xamarin.Android.Build.Tasks] remove the linksrc directory #3359

Uh oh!

Conversation

jonathanpeppers commented Jul 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes for jnimarshalmethod-gen

Uh oh!

jonathanpeppers commented Jul 18, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jonathanpeppers commented Aug 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

radekdoulik commented Aug 20, 2019

Uh oh!

jonathanpeppers commented Aug 21, 2019

Uh oh!

radekdoulik commented Aug 21, 2019

Uh oh!

radekdoulik commented Aug 21, 2019

Uh oh!

jonathanpeppers commented Aug 22, 2019

Uh oh!

mjo151 commented Nov 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonathanpeppers commented Nov 21, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jonathanpeppers commented Jul 16, 2019 •

edited

Loading

jonathanpeppers commented Aug 20, 2019 •

edited

Loading

mjo151 commented Nov 21, 2019 •

edited

Loading