Skip to content

Conditionally call ManagedPeer.Init() #761

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.

Sign up for GitHub

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

Jump to bottom
Merged
jonpryor merged 6 commits into from
Jan 11, 2021
Merged

Conditionally call ManagedPeer.Init() #761

jonpryor merged 6 commits into from
Jan 11, 2021

Conversation

@radekdoulik
Copy link
Member

@radekdoulik radekdoulik commented Dec 14, 2020 •
edited
Loading

Context: dotnet/android#5400
Context: dotnet/android#5444

Driven by XA_JI_EXCLUDE, which will be defined
in XAConfig.props in $(JavaInteropDefineConstants), when
building for XA. The file is generated by XA's xaprep
in bin/Build$(Configuration).

Also clean old XA_INTEGRATION #if's, they are not used anymore.

@radekdoulik
Conditionaly exclude ManagedPeer.Init call
02b19bb 
Driven by `XA_JI_EXCLUDE`, which will be defined
in Xamarin.Android.props, when building for XA.
@radekdoulik
Remove XA_INTEGRATION #if's
29bab19 
They are not used anymore
@jonpryor
Copy link
Contributor

jonpryor commented Dec 18, 2020

Who is setting $(JavaInteropDefineConstants) and what is it set to?

radekdoulik added a commit to radekdoulik/xamarin-android that referenced this pull request Dec 18, 2020
@radekdoulik
[illink] Don't initialize managed peer
aa5140f 
Context: dotnet#5400
Context: dotnet/java-interop#761

Generate `XAConfig.props` file in Java.Interop to define `XA_JI_EXCLUDE`
compilation constant.

The generated file contains properties set for Java.Interop by XA. So
far it is only setting one property, and thus doesn't necessarily be
generated. I still wanted to be written by `xaprep`, plus it might be
handy in future for other configuration.

The currently set `JavaInteropDefineConstants` property is going to be
used to disable `ManagedPeer` initialization.

We don't really use `ManagedPeer` today and it is duplicating
functionality of `ConstructorBuilder` class. The unwanted effect of
having it in JI is that it is pulling in System.Linq.Expressions, which
greatly increase assemblies size after linking.

The size reduction for simple XA app is cca 135kbytes of compressed
size.

    Size difference in bytes ([*1] apk1 only, [*2] apk2 only):
      -          27 assemblies/Mono.Android.dll
      -          30 assemblies/System.Collections.Concurrent.dll
      -         989 assemblies/System.Linq.dll
      -       2,930 assemblies/Java.Interop.dll
      -       4,514 assemblies/System.Collections.dll *1
      -       4,874 assemblies/System.ObjectModel.dll *1
      -       7,987 assemblies/System.Private.CoreLib.dll
      -     115,284 assemblies/System.Linq.Expressions.dll *1
    Summary:
      -     136,635 Assemblies -14.87% (of 918,689)
      -     139,506 Package size difference -1.79% (of 7,780,819)
@radekdoulik radekdoulik mentioned this pull request Dec 18, 2020
Closed
@radekdoulik radekdoulik marked this pull request as ready for review December 18, 2020 16:28
@radekdoulik
Copy link
Member Author

radekdoulik commented Dec 18, 2020

Who is setting $(JavaInteropDefineConstants) and what is it set to?

I updated the PR description with that information.

jonpryor
jonpryor reviewed Jan 7, 2021
View reviewed changes
jonpryor
jonpryor reviewed Jan 7, 2021
View reviewed changes
@radekdoulik
Remove forgotten import
6c7de91
@radekdoulik
Move the import of XAConfig.props
75f86fe 
...to a place whete `$(Configuration) is already set
@radekdoulik radekdoulik requested a review from jonpryor January 8, 2021 20:31
@jonpryor jonpryor merged commit a77aa33 into dotnet:master Jan 11, 2021
radekdoulik added a commit to radekdoulik/xamarin-android that referenced this pull request Jan 12, 2021
@radekdoulik
[illink] Don't initialize managed peer
a8eabeb 
Context: dotnet#5400
Context: dotnet/java-interop#761

Generate `XAConfig.props` file in Java.Interop to define `XA_JI_EXCLUDE`
compilation constant.

The generated file contains properties set for Java.Interop by XA. So
far it is only setting one property, and thus doesn't necessarily be
generated. I still wanted to be written by `xaprep`, plus it might be
handy in future for other configuration.

The currently set `JavaInteropDefineConstants` property is going to be
used to disable `ManagedPeer` initialization.

We don't really use `ManagedPeer` today and it is duplicating
functionality of `ConstructorBuilder` class. The unwanted effect of
having it in JI is that it is pulling in System.Linq.Expressions, which
greatly increase assemblies size after linking.

The size reduction for simple XA app is cca 135kbytes of compressed
size.

    Size difference in bytes ([*1] apk1 only, [*2] apk2 only):
      -          27 assemblies/Mono.Android.dll
      -          30 assemblies/System.Collections.Concurrent.dll
      -         989 assemblies/System.Linq.dll
      -       2,930 assemblies/Java.Interop.dll
      -       4,514 assemblies/System.Collections.dll *1
      -       4,874 assemblies/System.ObjectModel.dll *1
      -       7,987 assemblies/System.Private.CoreLib.dll
      -     115,284 assemblies/System.Linq.Expressions.dll *1
    Summary:
      -     136,635 Assemblies -14.87% (of 918,689)
      -     139,506 Package size difference -1.79% (of 7,780,819)
radekdoulik added a commit to radekdoulik/xamarin-android that referenced this pull request Jan 12, 2021
@radekdoulik
[illink] Don't initialize managed peer
02cd1ff 
Context: dotnet#5400
Context: dotnet/java-interop#761

Generate `XAConfig.props` file in Java.Interop to define `XA_JI_EXCLUDE`
compilation constant.

The generated file contains properties set for Java.Interop by XA. So
far it is only setting one property, and thus doesn't necessarily be
generated. I still wanted to be written by `xaprep`, plus it might be
handy in future for other configuration.

The currently set `JavaInteropDefineConstants` property is going to be
used to disable `ManagedPeer` initialization.

We don't really use `ManagedPeer` today and it is duplicating
functionality of `ConstructorBuilder` class. The unwanted effect of
having it in JI is that it is pulling in System.Linq.Expressions, which
greatly increase assemblies size after linking.

The size reduction for simple XA app is cca 135kbytes of compressed
size.

    Size difference in bytes ([*1] apk1 only, [*2] apk2 only):
      -          27 assemblies/Mono.Android.dll
      -          30 assemblies/System.Collections.Concurrent.dll
      -         989 assemblies/System.Linq.dll
      -       2,930 assemblies/Java.Interop.dll
      -       4,514 assemblies/System.Collections.dll *1
      -       4,874 assemblies/System.ObjectModel.dll *1
      -       7,987 assemblies/System.Private.CoreLib.dll
      -     115,284 assemblies/System.Linq.Expressions.dll *1
    Summary:
      -     136,635 Assemblies -14.87% (of 918,689)
      -     139,506 Package size difference -1.79% (of 7,780,819)
radekdoulik added a commit to radekdoulik/xamarin-android that referenced this pull request Jan 15, 2021
@radekdoulik
[illink] Don't initialize managed peer
eef95d9 
Context: dotnet#5400
Context: dotnet/java-interop#761

Generate `XAConfig.props` file in Java.Interop to define `XA_JI_EXCLUDE`
compilation constant.

The generated file contains properties set for Java.Interop by XA. So
far it is only setting one property, and thus doesn't necessarily be
generated. I still wanted to be written by `xaprep`, plus it might be
handy in future for other configuration.

The currently set `JavaInteropDefineConstants` property is going to be
used to disable `ManagedPeer` initialization.

We don't really use `ManagedPeer` today and it is duplicating
functionality of `ConstructorBuilder` class. The unwanted effect of
having it in JI is that it is pulling in System.Linq.Expressions, which
greatly increase assemblies size after linking.

The size reduction for simple XA app is cca 135kbytes of compressed
size.

    Size difference in bytes ([*1] apk1 only, [*2] apk2 only):
      -          27 assemblies/Mono.Android.dll
      -          30 assemblies/System.Collections.Concurrent.dll
      -         989 assemblies/System.Linq.dll
      -       2,930 assemblies/Java.Interop.dll
      -       4,514 assemblies/System.Collections.dll *1
      -       4,874 assemblies/System.ObjectModel.dll *1
      -       7,987 assemblies/System.Private.CoreLib.dll
      -     115,284 assemblies/System.Linq.Expressions.dll *1
    Summary:
      -     136,635 Assemblies -14.87% (of 918,689)
      -     139,506 Package size difference -1.79% (of 7,780,819)
@jpobst jpobst added this to the 11.2 (16.10 / 8.10) milestone Feb 2, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jpobst jpobst Awaiting requested review from jpobst

@jonpryor jonpryor Awaiting requested review from jonpryor

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

11.3 (16.10 / 8.10)

Development

Successfully merging this pull request may close these issues.

3 participants

@radekdoulik @jonpryor @jpobst