Move the creation of an uninitialized NSObject from native to managed, which:
* Removes the need for the mono_object_new Embedding API.
* Removes one location where we write to managed memory from native code (to
write the handle + flags in the uninitialized NSObject).
* Makes things easier for CoreCLR.
Any performance difference will be neglible compared to running the GC, so
there's no compelling reason to use the embedding API.
This makes things a bit easier with CoreCLR, since the new code works there too.
This also required a few changes in delegates.t4 to make code generation for
functions without arguments work correctly.
* Convert the GCHandles interface from 32-bit ints to pointer size types
This involves:
* Stop using some bits of the GCHandle to store extra flags, instead add an extra
field to store those flags.
* Define a INVALID_GCHANDLE constant and use it instead of 0/NULL. This is not
strictly required, but it makes the code more self-documenting.
* Define a GCHandle type (typedef'ed to void*) and change all variables and parameters
to use it instead of guint32.
* Use our own xamarin_gchandle_* methods (with pointer-sized types) that wraps
the mono_gchandle_* embedding API (which uses 32-bit types) everywhere.
* Update managed code (registrars, runtime code, etc) accordingly.
* [runtime] Make debug code compile.
* Fix typo.
* Fix signature of xamarin_create_gchandle.
Co-authored-by: Aaron R Robinson <arobins@microsoft.com>
These methods were already partially using GCHandles, so convert the parameter
using ObjectWrapper to GCHandle, and port the rest of the existing logic to
use the new helper API.
The native methods xamarin_get_[generic_]method_from_token are a bit unusual
in that they return an actual GCHandle. This is for performance reasons, since
in some cases their return value is passed as parameters to other function
calls to managed code, in which case we need the GCHandle. This way we avoid
round-tripping a GCHandle multiple times.
Some of the fixes done for the warnings have breaking changes. Move back
to int and ensure that we do not have any compilation errors (we are
using -Werror).
Fixes: https://github.com/xamarin/xamarin-macios/issues/7509
* [Runtime] Enable the -Wshorten-64-to-32 flag and fix all warnings.
We want to enable the -Wconversion but that will raise too many warning
for a single commit. We are enabiling one by one the flags included in
-Wconversion so that we have smaller diffs.
-Wshorten-64-to-32 adds warnings when there is a implicit conversion that
loses integer precision. We are moving all the 32 to 64 conversions to
use 64. Expecially since most of the code changed is related with sizes,
legths and params counts that are never going to be negative.
Co-Authored-By: Rolf Bjarne Kvinge <rolf@xamarin.com>
* [runtime] Add an inner exception parameter to Runtime.CreateProductException.
This allows us to simplify code by using inner (and outer) exceptions as
a means to provide information instead of passing extra information
around in order to create decent exceptions.
One example is how we pass the selector and method name to the method
that converts from a native id to a managed NSObject instance: passing
this information is not necessary anymore if we can use two exceptions,
one for the failure to convert from an id to a NSObject instance,
wrapped in a second that tells which method/selector call ran into this
conversion problem.
* [runtime] Throw better exceptions when the dynamic registrar can't marshal something.
* [runtime] Throw a better exception when something goes wrong when trying to marshal a return value.
* [runtime] Use inner exceptions to convey failure information instead of trying to create a single exception with all we know.
* Fix merge problem.
* Add support for delegates as return values in protocol members. Fixes#4102.
This required a few changes:
* The generator now emits the DelegateProxy attribute for property getters in
protocol interfaces.
* The generator now emits the DelegateProxy attribute in ProtocolMember
attributes (and the ProtocolMember attribute has been extended with
additional properties for this purpose).
* The generator now emits the BlockProxy attribute for the parameter in
property setters.
* The generator now emits the BlockProxy attribute in ProtocolMember
attributes for property setters.
* The static registrar now emits the metadata token for the
DelegateProxy.DelegateType property into the generated code so that the
DelegateProxy attribute itself isn't needed at runtime. This is required
when the dynamic registrar has been optimized away.
Fixes https://github.com/xamarin/xamarin-macios/issues/4102.
* [tests] Update MX4105 test to expect new warnings.
* Convert it into a MM8027/MT8027 error (with documentation).
* Add information about the selector and managed method that triggered the error.
Now the problem reported in issue #4254 shows up as:
> ObjCRuntime.RuntimeException: Failed to marshal the Objective-C object 0x7f8080412810 (type: UIView). Could not find an existing managed instance for this object, nor was it possible to create a new managed instance (because the type 'UIKit.UIView&' does not have a constructor that takes one IntPtr argument).
> Additional information:
> Selector: popoverController:willRepositionPopoverToRect:inView:
> Method: UIKit.UIPopoverController+_UIPopoverControllerDelegate.WillReposition(UIKit.UIPopoverController, CoreGraphics.CGRect ByRef, UIKit.UIView ByRef)
instead of just:
> ObjCRuntime.RuntimeException: Failed to marshal the Objective-C object 0x7f8080412810 (type: UIView). Could not find an existing managed instance for this object, nor was it possible to create a new managed instance (because the type 'UIKit.UIView&' does not have a constructor that takes one IntPtr argument).
which makes it much easier to understand, track down, and fix/work around,
both for customers and ourselves.
* [static registrar] Optimize creation of delegates for blocks.
Optimize creation of delegates for blocks so that it doesn't require the
dynamic registrar.
This is done by getting the metadata token for the Create method that creates
the delegate, and embed that metadata token in the generated code from the
static registrar.
Also add tests, since this scenario was not covered by tests already.
* [mmptest] Fix test after recent changes.
* [test-libraries] Avoid duplicate symbols.
* [tests] Update according to changes.
This can be reproduced by running monotouch-test with all optimizations in
Debug mode (because some of the exception marshalling tests are only enabled
in debug mode), so add such a configuration to xharness. To avoid bloating PR
builds, this configuration is only enabled when running all tests (either
manually selecting all tests for a PR, or on Wrench, where everything is
always tested).
Mark managed callback functions if they're only used in the dynamic registrar,
and only initialize the corresponding delegate if the dynamic registrar is
available.
This will make it possible for the linker to remove those managed functions if
the dynamic registrar is being removed.
* [linker] Optimize calls to BlockLiteral.SetupBlock to inject the block signature.
Optimize calls to BlockLiteral.SetupBlock[Unsafe] to calculate the block
signature at build time, and inject it into the call site.
This makes block invocations 10-15x faster (I've added tests that asserts at
least an 8x increase).
It's also required in order to be able to remove the dynamic registrar code in
the future (since calculating the block signature at runtime requires the
dynamic registrar).
* [mtouch/mmp] Add support for reporting errors/warnings that point to the code line causing the error/warning.
Add support for reporting errors/warnings that point to the code line causing
the error/warning by adding ErrorHelper overloads that take the exact
instruction to report (previously we defaulted to the first line/instruction
in a method).
* [tests] Add support for asserting filename/linenumber in warning messages.
* Make all methods that manually create BlockLiterals optimizable.
* [tests] Create a BaseOptimizeGeneratedCodeTest test that's included in both XI's and XM's link all test.
* [tests] Add link all test (for both XI and XM) to test the BlockLiteral.SetupBlock optimization.
* [tests] Add mtouch/mmp tests for the BlockLiteral.SetupBlock optimization.
* [tests][linker] Make the base test class abstract, so tests in the base class aren't executed twice.
* [tests][linker] Don't execute linkall-only tests in linksdk.
The optimization tests only apply when the test assembly is linked, and that
only happens in linkall, so exclude those tests in linksdk.
* [tests][mmptest] Update test according to mmp changes.
Fixes these test failures:
1) Failed : Xamarin.MMP.Tests.MMPTests.MM0132("inline-runtime-arch")
The warning 'MM0132: Unknown optimization: 'inline-runtime-arch'. Valid optimizations are: remove-uithread-checks, dead-code-elimination, inline-isdirectbinding, inline-intptr-size.' was not found in the output:
Message #1 did not match:
actual: 'Unknown optimization: 'inline-runtime-arch'. Valid optimizations are: remove-uithread-checks, dead-code-elimination, inline-isdirectbinding, inline-intptr-size, blockliteral-setupblock.'
expected: 'Unknown optimization: 'inline-runtime-arch'. Valid optimizations are: remove-uithread-checks, dead-code-elimination, inline-isdirectbinding, inline-intptr-size.'
Message #2 did not match:
actual: 'Unknown optimization: 'inline-runtime-arch'. Valid optimizations are: remove-uithread-checks, dead-code-elimination, inline-isdirectbinding, inline-intptr-size, blockliteral-setupblock.'
expected: 'Unknown optimization: 'inline-runtime-arch'. Valid optimizations are: remove-uithread-checks, dead-code-elimination, inline-isdirectbinding, inline-intptr-size.'
2) Failed : Xamarin.MMP.Tests.MMPTests.MM0132("foo")
The warning 'MM0132: Unknown optimization: 'foo'. Valid optimizations are: remove-uithread-checks, dead-code-elimination, inline-isdirectbinding, inline-intptr-size.' was not found in the output:
Message #1 did not match:
actual: 'Unknown optimization: 'foo'. Valid optimizations are: remove-uithread-checks, dead-code-elimination, inline-isdirectbinding, inline-intptr-size, blockliteral-setupblock.'
expected: 'Unknown optimization: 'foo'. Valid optimizations are: remove-uithread-checks, dead-code-elimination, inline-isdirectbinding, inline-intptr-size.'
Message #2 did not match:
actual: 'Unknown optimization: 'foo'. Valid optimizations are: remove-uithread-checks, dead-code-elimination, inline-isdirectbinding, inline-intptr-size, blockliteral-setupblock.'
expected: 'Unknown optimization: 'foo'. Valid optimizations are: remove-uithread-checks, dead-code-elimination, inline-isdirectbinding, inline-intptr-size.'
* [tests][linker] Fix typo.
Fixes this test failure:
1) SetupBlock_CustomDelegate (Linker.Shared.BaseOptimizeGeneratedCodeTest.SetupBlock_CustomDelegate)
Counter
Expected: 1
But was: 2
* [registrar] Minor adjustment to error message to match previous (and better) behavior.
Fixes this test failure:
1) Failed : Xamarin.Registrar.GenericType_WithInvalidParameterTypes
The error 'MT4136: The registrar cannot marshal the parameter type 'System.Collections.Generic.List`1<U>' of the parameter 'arg' in the method 'Open`1.Bar(System.Collections.Generic.List`1<U>)'' was not found in the output:
Message #1 did not match:
actual: 'The registrar cannot marshal the parameter type 'System.Collections.Generic.List`1<Foundation.NSObject>' of the parameter 'arg' in the method 'Open`1.Bar(System.Collections.Generic.List`1<U>)''
expected: 'The registrar cannot marshal the parameter type 'System.Collections.Generic.List`1<U>' of the parameter 'arg' in the method 'Open`1.Bar(System.Collections.Generic.List`1<U>)''
* [docs] mmp shows MM errors/warnings.
* [docs] Improve according to reviews.
* [tests] Fix merge failure causing test duplication.
* [runtime] Remove specialized function for getting the Selector/Class handle of a managed Selector/Class instance.
The existing function to get the handle of an INativeObject works just as fine.
* [runtime] Store and use a prebuilt XamMacLauncher.
Some changes in the runtime needs corresponding changes in XamMac.dll, but
since we're not changing XamMac.dll anymore, we can't change the runtime
either.
This means that we must add XamMacLauncher to the list of binaries we copy.
* [runtime] Store and use a prebuilt libxammac*.a as well.
Two Objective-C methods can be named identically as long as one is static and
the other instance.
We must support this since Apple did just this (in the NSItemProviderReading /
NSItemProviderWriting protocols).
We solve it by prepending a '+' or '-' to the selector when hashing it (to
determine selector uniqueness, and to look the method up again at runtime).
Use metadata tokens instead of strings to find types and methods.
This makes the code to find methods more compact (a lot less strings in the
executable, and additionally in most cases a compact representation (32-bit
integer) of the corresponding metadata token and additional information can be
used, which results in less executable code (fewer parameters to methods,
etc)), resulting in smaller executables.
Size savings are around 200kb for dont link apps, and 20-60kb for linked apps
(this obviously varies a lot depending on how much has to registered by the
registrar).
| | Before | After | Diff |
|----------------|--------------:|--------------:|------------------:|
| dontlink/32bit | 102.810.144 | 102.609.456 | -200.688 = -0,20% |
| dontlink/64bit | 107.420.576 | 107.221.792 | -198.784 = -0,19% |
| linksdk/32bit | 40.957.296 | 40.936.864 | -20.432 = -0,05% |
| linksdk/64bit | 43.113.136 | 43.093.936 | -19.200 = -0,04% |
| linkall/32bit | 38.410.032 | 38.348.288 | -61.744 = -0,16% |
| linkall/64bit | 40.315.200 | 40.267.344 | -47.856 = -0,12% |
Additionally I've removed the `lazy_map` dictionary, which we populated at
startup and was used to map between Class instances and the corresponding
managed type's FullName, and instead iterate over a native array of Class ->
metadata token mappings whenever we need to look up the managed type for a
certain Class instance.
This is slightly slower for each type we need to look up (for a non-linked app
there might be a 2000-3000 entries in the native array, which would be
iterated instead of using a hashtable lookup), but it's only done once per
type and there's a significant startup memory improvement.
For a non-linked test app I get the following using the Xamarin profiler:
| | Before | After | Diff |
|-------------------|--------:|--------:|----------------:|
| Memory allocated | 2,8 MB | 2,4 MB | -0,4 MB = -14 % |
| Objects allocated | 43678 | 38463 | -5215 = -12 % |
| Private bytes | 26,6 MB | 24,4 MB | -2,2 MB = -8,3% |
| Working set | 26,6 MB | 24,4 MB | -2,2 MB = -8,3% |
Example code:
public override void ViewDidLoad ()
{
throw new Exception ("USELESS");
}
Current output with unhandled exceptions that are marshaled:
Unhandled Exception:
System.Exception: USELESS
at (wrapper managed-to-native) AppKit.NSApplication:NSApplicationMain (int,string[])
at AppKit.NSApplication.Main (System.String[] args) [0x00041] in /work/maccore/master/xamarin-macios/src/AppKit/NSApplication.cs:98
at UselessExceptions.MainClass.Main (System.String[] args) [0x00007] in /Users/rolf/Downloads/filed-bug-test-cases-master/Xamarin/bxc45742/Main.cs:10
[ERROR] FATAL UNHANDLED EXCEPTION: UselessExceptions.CustomException: USELESS
at (wrapper managed-to-native) AppKit.NSApplication:NSApplicationMain (int,string[])
at AppKit.NSApplication.Main (System.String[] args) [0x00041] in /work/maccore/master/xamarin-macios/src/AppKit/NSApplication.cs:98
at UselessExceptions.MainClass.Main (System.String[] args) [0x00007] in /Users/rolf/Downloads/filed-bug-test-cases-master/Xamarin/bxc45742/Main.cs:10
Note how the managed frame where the exception was thrown does not show up.
This is because we technically catch exceptions when marshaling them, and then
later we call mono_raise_exception to raise the same exception. Unfortunately
that leads to overwriting the initial stack trace, and we end up with a stack
trace that does not include the location where the original exception was
thrown.
So instead of calling mono_raise_exception to rethrow the same exception, we
use ExceptionDispatchInfo, which is able to capture the stack trace for both
the original exception and the new exception, producing the following output:
System.Exception: USELESS
at UselessExceptions.ViewController.ViewDidLoad () [0x0000c] in /Users/rolf/Downloads/filed-bug-test-cases-master/Xamarin/bxc45742/ViewController.cs:27
--- End of stack trace from previous location where exception was thrown ---
at (wrapper managed-to-native) AppKit.NSApplication:NSApplicationMain (int,string[])
at AppKit.NSApplication.Main (System.String[] args) [0x00041] in /work/maccore/master/xamarin-macios/src/AppKit/NSApplication.cs:98
at UselessExceptions.MainClass.Main (System.String[] args) [0x00007] in /Users/rolf/Downloads/filed-bug-test-cases-master/Xamarin/bxc45742/Main.cs:10
[ERROR] FATAL UNHANDLED EXCEPTION: UselessExceptions.CustomException: USELESS
at UselessExceptions.ViewController.ViewDidLoad () [0x0000c] in /Users/rolf/Downloads/filed-bug-test-cases-master/Xamarin/bxc45742/ViewController.cs:27
--- End of stack trace from previous location where exception was thrown ---
at (wrapper managed-to-native) AppKit.NSApplication:NSApplicationMain (int,string[])
at AppKit.NSApplication.Main (System.String[] args) [0x00041] in /work/maccore/master/xamarin-macios/src/AppKit/NSApplication.cs:98
at UselessExceptions.MainClass.Main (System.String[] args) [0x00007] in /Users/rolf/Downloads/filed-bug-test-cases-master/Xamarin/bxc45742/Main.cs:10
Incidently this is how Xamarin.Android does it [1].
[1]: 9387f2fe16https://bugzilla.xamarin.com/show_bug.cgi?id=45742
Rolf Bjarne Kvinge
Manuel de la Pena
Chris Hamons