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.

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

[Mono] Add support for DisableRuntimeMarshallingAttribute #61685

Open
4 of 5 tasks
lambdageek opened this issue Nov 16, 2021 · 5 comments
Open
4 of 5 tasks

[Mono] Add support for DisableRuntimeMarshallingAttribute #61685

lambdageek opened this issue Nov 16, 2021 · 5 comments
Assignees
@fanyang-mono
Labels
area-Interop-mono tracking This issue is tracking the completion of other related issues.
Milestone
9.0.0

Comments

@lambdageek
Copy link
Member

lambdageek commented Nov 16, 2021 •
edited by jandupej

The DisableRuntimeMarshallingAttribute is added to a module to indicate that it should:

  • Consider all unmanaged types as blittable
  • Promise that the current assembly does not need the default runtime marshaling behavior (that is responsible for converting strings between encodings, supporting SafeHandle, etc, etc). The idea is that a DllImportGenerator is used to generate those kinds of wrappers, and the runtime is only responsible for dealing with primitive types.

For mono that means:

We can drop the component if every single assembly meets one of these criteria:
* Is also tagged with the DisableRuntimeMarshalingAttribute.
* Uses only PInvokes with blitable types

Tooling consists of the RuntimeMarhsalingScannner which scans assemblies for these criteria.

The user sets RuntimeMarshaling to one of three values: Auto, Disable, and Enable. Auto uses the RuntimeMarshalingScanner to scan all assemblies, and drops the component if all assemblies meet the criteria. Disable forces disabling of runtime marshaling, and the SDK will emit warnings for incompatible assemblies. Enable preserves runtime marshaling, regardless of the results of the RuntimeMarshalingScanner.

Implementation plan:

  • First pass implements the scanner, but this can only implement warnings on an assembly eevel
  • Platform order: wasm, iOS, android
  • Second version uses source analyzer for better errors.
@lambdageek lambdageek added this to the 7.0.0 milestone Nov 16, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Nov 16, 2021
@lambdageek lambdageek mentioned this issue Nov 16, 2021
Closed
@lambdageek lambdageek added tracking This issue is tracking the completion of other related issues. and removed untriaged New issue has not been triaged by the area owner labels Nov 16, 2021
@jkoritzinsky jkoritzinsky mentioned this issue Jan 4, 2022
Merged
@SamMonoRT SamMonoRT assigned naricc and unassigned fanyang-mono Jan 26, 2022
@lambdageek lambdageek mentioned this issue Feb 7, 2022
Closed
@lambdageek
Copy link
Member Author

We should also take advantage of the DISABLE_NONBLITTABLE work that was done as part of #46444 albeit using mono runtime components instead of ifdefs

@naricc naricc mentioned this issue Apr 14, 2022
Closed
This was referenced Apr 28, 2022
Closed
Merged
@naricc naricc mentioned this issue May 11, 2022
Merged
@naricc naricc mentioned this issue Jun 23, 2022
Merged
@lambdageek lambdageek mentioned this issue Jul 25, 2022
Open
8 tasks
@SamMonoRT
Copy link
Member

moving the tracking issue to 8.0.0 -- Based on the design and complexity of actually moving/dropping components and respective size savings we will consider backporting fixes.

@SamMonoRT SamMonoRT modified the milestones: 7.0.0, 8.0.0 Aug 11, 2022
jonpryor added a commit to jonpryor/xamarin-android that referenced this issue Aug 15, 2022
@jonpryor
[Xamarin.Android.Build.Tasks] Use marshal-ilgen component
ae56b4a 
Fixes: dotnet#7249

Context: dotnet/runtime#71203
Context: dotnet/runtime#60639
Context: dotnet/runtime#61685

Mono introduced a new `marshal-ilgen` component in
dotnet/runtime@de32c446 which needs to be used when
*non-blittable type* is required.

`string` marshaling is a form of non-blittable marshaling.

Eventually the idea is that assemblies should migrate to use the
[`DllImport Generator`][0] for non-blittable marshaling purposes,
then [`[assembly:DisableRuntimeMarshallingAttribute]`][1] can be
applied to the assembly, and -- if no assemblies in an Android app
require non-blittable marshaling -- then the `marshal-ilgen`
component can be omitted from the `.apk`, reducing app size.

That's a fair number of `if`s; this won't be happening soon.

In the meantime, we need to start including the `marshal-ilgen`
component in all .NET 7 RC1+ builds so that our existing `DllImport`
declarations continue to work.

Update `@(_MonoComponent)` to add `marshal-ilgen`.

In order to facilitate future testing, allow the `marshal-ilgen`
component to be *excluded* if the
`$(_AndroidExcludeMarshalIlgenComponent)` property is True.

[0]: https://github.com/dotnet/runtimelab/tree/feature/DllImportGenerator
[1]: dotnet/runtime#60639
@jonpryor jonpryor mentioned this issue Aug 15, 2022
Merged
@rolfbjarne rolfbjarne mentioned this issue Aug 16, 2022
Open
jonpryor added a commit to dotnet/android that referenced this issue Aug 17, 2022
@jonpryor
[Xamarin.Android.Build.Tasks] Use marshal-ilgen component (#7260)
eab31f7 
Fixes: #7249

Context: dotnet/runtime#71203
Context: dotnet/runtime#60639
Context: dotnet/runtime#61685
Context: #7276

Mono introduced a new `marshal-ilgen` component in
dotnet/runtime@de32c446 which needs to be used when
*non-blittable type* is required.

`string` marshaling is a form of non-blittable marshaling, as is
`Marshal.PtrToStructure()` (?!).

Eventually the idea is that assemblies should migrate to use the
[`DllImport Generator`][0] for non-blittable marshaling purposes,
then [`[assembly:DisableRuntimeMarshallingAttribute]`][1] can be
applied to the assembly, and -- if no assemblies in an Android app
require non-blittable marshaling -- then the `marshal-ilgen`
component can be omitted from the `.apk`, reducing app size.

That's a fair number of `if`s; this won't be happening soon.

In the meantime, we need to start including the `marshal-ilgen`
component in all .NET 7 RC1+ builds so that our existing
`[DllImport]` declarations continue to work.  Failure to do so
results in runtime errors:

	mono-rt : [ERROR] FATAL UNHANDLED EXCEPTION: System.InvalidProgramException: Invalid IL code in (wrapper managed-to-native) object:wrapper_native_0x7725af156f10 (intptr): IL_0012: calli     0x00000003
	mono-rt :    at System.Runtime.InteropServices.Marshal.PtrToStructureHelper(IntPtr , Object , Boolean )
	mono-rt :    at System.Runtime.InteropServices.Marshal.PtrToStructure(IntPtr , Type )
	mono-rt :    at Java.Interop.JniRuntime.CreateInvoker(IntPtr )
	mono-rt :    at Java.Interop.JniRuntime..ctor(CreationOptions )
	mono-rt :    at Android.Runtime.AndroidRuntime..ctor(IntPtr , IntPtr , Boolean , IntPtr , IntPtr , Boolean )
	mono-rt :    at Android.Runtime.JNIEnv.Initialize(JnienvInitializeArgs* )

Update `@(_MonoComponent)` to add `marshal-ilgen`.

In order to facilitate future testing, allow the `marshal-ilgen`
component to be *excluded* if the
`$(_AndroidExcludeMarshalIlgenComponent)` property is True.

[0]: https://github.com/dotnet/runtimelab/tree/feature/DllImportGenerator
[1]: dotnet/runtime#60639
@jandupej jandupej mentioned this issue Jul 13, 2023
Closed
@jandupej jandupej modified the milestones: 8.0.0, 9.0.0 Jul 13, 2023
@jandupej
Copy link
Member

Moving the entire issue to net9.0. Created a separate issue for the iOS implementation, which should be included in net8.0

@jandupej jandupej mentioned this issue Jul 14, 2023
Merged
@jandupej
Copy link
Member

jandupej commented Oct 2, 2023

@simonrozsival @lambdageek Are we ready for implementing this also on Android? cc: @SamMonoRT

@lambdageek
Copy link
Member Author

lambdageek commented Oct 2, 2023 •
edited

@simonrozsival @lambdageek Are we ready for implementing this also on Android? cc: @SamMonoRT

I think we should check what is the state of android bindings, if they use LibraryImport (or possibly DllImport with only blittlable arguments) extensively enough that doing the work is profitable.

Android is also a pretty dynamic environment, do we know they don't generate non-blittable pinvokes at runtime?

@SamMonoRT SamMonoRT assigned fanyang-mono and unassigned jandupej Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@fanyang-mono fanyang-mono

Labels
area-Interop-mono tracking This issue is tracking the completion of other related issues.
Projects
None yet
Milestone
9.0.0
Development

No branches or pull requests
5 participants
@lambdageek @naricc @SamMonoRT @fanyang-mono @jandupej