[Proposal] Trimming friendly registrar #19342
Conversation
simonrozsival
added
the
request-for-comments
|
@vitek-karas @jkotas FYI |
| partial class CGRect { | ||
| [UnmanagedCallersOnly(EntryPoint = "_registrar__CoreGraphics_CGRect__CreateManagedInstance")] | ||
| public static IntPtr CreateManagedInstance(IntPtr handle) | ||
| => GCHandle.Alloc (new CGRect (handle, owns: false)).ToIntPtr (); | ||
| } |
There was a problem hiding this comment.
There should be no need to handle C-style structs.
There was a problem hiding this comment.
I've just now ran into an issue with SecCertificate while working on a PoC. It's fine when it's converted directly in the managed registrar callback (thanks to #18706), but when it's an element of an NSArray, the call to NSArray.ArrayFromHandle won't work without some special case for the native object.
| - code generator can produce errors and warnings while editing the source files for better DX | ||
| - source generators are more future-proof than Mono.Cecil | ||
| - **Cons**: | ||
| - the generated code has to be distributed with runtimepack/nupkg |
There was a problem hiding this comment.
A few more cons:
- Can only generate valid C# code. Generating IL directly opens up a few more possibilities (which we already take advantage of iirc).
- The registrar is now a build-time option (when building the app), and the default is different between simulator and device. Supporting a different registrar than the source-generated code (i.e. not use the source-generated code) would likely complicate things more. Another option is to (eventually) make this the single registrar, but that would impose additional requirements (fast at runtime + low memory consumption at runtime + fast at build time is not easy...)
- If we find an issue in the source-generated code, we'll have to convince library authors to update their packages. This is a hard problem, in fact many won't do it. In the past we've actually developed tools for library consumers to pre-process their third-party libraries before they use them in their projects (and this was easier when they would just reference the assembly instead of referencing a NuGet).
- The above also applies to improvements: in the past any improvement to the static registrar would immediately apply to all third-party libraries. With the source generator approach library authors will have to rebuild their projects to get any improvements (and they'll have to multi-target if they want to support older .NET versions).
- It'll be harder to innovate. Right now the managed static registrar can use internal API from the platform assembly, and if needed we can just remove that internal API if we find a better solution. With a source generator we'll have to make all consumed API public, and keep maintaining it.
- Testing will be harder: we'll have to add test cases for each iteration of the source-generated code, to make sure it works with future versions of our product.
I think the best solution is option B, but doing option A for prototyping/expedience in the beginning makes sense.
| - It is unclear what would be the file size and startup or runtime performance implications of these changes. | ||
| - We will need to generate less managed code but on the other hand we'll generate more Objective-C code. | ||
| - There will be more transitions between managed and native code. Does it matter too much in the context of NativeAOT | ||
| though? Will it be worse than performing linear search in the lookup tables that we do today? |
There was a problem hiding this comment.
This is likely app-dependant: a linear search in a small table is quick, while in a bigger table it gets quite slow. This new registrar has constant speed, and there are probably optimizations that can be made too if we find bottlenecks.
| using the same ideas that are outlined in this document. | ||
| - This claim needs validation though. Are there some obvious cases which would be hard or obviously inefficient | ||
| to implement this way? | ||
| - It is unclear what would be the file size and startup or runtime performance implications of these changes. |
There was a problem hiding this comment.
Note that there are memory implications too: there's no virtual memory on iOS, so keeping dirty memory (i.e. memory that can't be discared because it exists on disk) low is important. For some app extensions there's a dirty memory limit of only 10mb, and we want to use as little as possible of that. This was one driving factor for some design decisions in the static registrar (a constant table in Objective-C generated at build time does not consume any dirty memory, while a dictionary maintained at runtime does).
| ### Risks | ||
|
|
||
| - We still need the current setup for all the cases which are unsupported by this scenario (inner dev loop, support | ||
| for existing codebases that rely on the static registrar). The maintenance cost might not decrease, but instead |
There was a problem hiding this comment.
I believe it shouldn't be too much of a burden to have (yet) another registrar, especially if we can share at least some of the existing code. Once any new registrars are deemed better/feature complete, we can deprecate/remove any old ones.
There was a problem hiding this comment.
I believe it shouldn't be too much of a burden to have (yet) another registrar
Would this still be true if we choose to implement the new registrar with option B - independently to custom linker steps?
There was a problem hiding this comment.
There are two very distinct parts of the registrar: figure out what needs to be registered, and then doing the actual registration. The first part is currently shared between all registrars (for the dynamic registrar this happens at runtime inside Microsoft.iOS.dll, for the static registrars this happens as a custom linker step, so in very different execution environments). The second part is very different between the dynamic and the static registrars, and this hasn't been much of a problem in the past, so I expect it won't be for a third registrar either. Once again: a good test suite makes this much easier.
There was a problem hiding this comment.
I see, thanks for the explanation!
| ### Option B: Pre-trimming + post-trimming MSBuild tasks | ||
|
|
||
| We would modify the DLLs directly before the code is AOTed. Instead of ILLink and custom linker steps, we would create | ||
| an independent MSBuild task to perform the weaving. This would include generating the `__Registrar_Callback__` nested | ||
| classes with UCO endpoints, proxy classes for generic types, and generating any `[DynamicDependency]` or other attributes | ||
| to express all inverse dependencies. At the same time, we would generate the Objective-C code and any information about | ||
| native dependencies (frameworks, static libraries, other generated Objective-C source files). |
There was a problem hiding this comment.
If we go with the option B and we have a dedicated task which uses Mono.Cecil to load/process/write the assemblies, would we still need or have a dependency on ILLink?
I am just thinking if in this case we would have multiple build tasks needing to load/process/write assemblies which would affect the build time.
There was a problem hiding this comment.
The goal is specifically to get rid of the ILLink step altogether. We might bring some of the code from the custom linker steps into the pre-processing task, but we shouldn't definitely run all three tools - the new generator task, ILLink, and then ILC.
There was a problem hiding this comment.
Thanks for the clarification!
| - generated code isn't distributed with runtimepack/nupkg | ||
| - it would be an invisible change for libraries authors - they won't need to rebuild their apps for .NET 9 | ||
| - when we generate IL directly, we have control over visibility of certain members and nested types if needed | ||
| - we can reuse some code from the current codebase if we implement the pre-trimming task using Mono.Cecil |
There was a problem hiding this comment.
I think we should try to use System.Reflection.Metadata for this. It is a bit more work (it doesn't have an in-memory model for everything), but we can leverage code we have in other tools (crossgen, ilc, ...) to fill in the gaps. The main advantages of it are:
- It's faster than Cecil (lot less allocation, generally designed for speed)
- It is thread safe, so if built properly the whole tool can be multi-threaded (unlike illink) and as such increase the speed a lot
- It is maintained along with Roslyn (it's used by Roslyn to write the compiled assemblies), so new language/runtime features will be implemented in it "for us". Unlike Mono.Cecil which we have to fix every time
| - source generators are _synchronous_ and doing many IO operations will slow them down, causing lagging in the editor | ||
| - its's not yet clear how to correctly generate the `[DynamicDependency]` attributes and if the inverse dependency | ||
| attributes could be added to .NET | ||
|
|
There was a problem hiding this comment.
I would also be worried about the cost of using pinvoke/reverse pinvoke. Have you measured the difference compared to the existing solution?
There was a problem hiding this comment.
Not yet. I still need to finish a PoC and make these measurements.
| on module initializers or class and instance constructors to emulate similar behaviour today. | ||
|
|
||
| Unfortunately, `DynamicDependency` implies use of reflection. Therefore, the IL Compiler emits unnecessary metadata. | ||
| Ideally, we would have some additional attribute to express "static" or "implicit" dependency. It would also help if we |
There was a problem hiding this comment.
I'm confused by this - if the member in question is accessed via direct reference, then there should be no need for DynamicDependency. If it's accessed via reflection, then we need the reflection metadata so DynamicDependency is correct. Is there some other way the member can be accessed?
There was a problem hiding this comment.
I see. I think I'll drop the part about reflection completely.
The reason we need those DynamicDependency attributes is to emulate the Preserve attributes (as described in the next section). We're only using direct references, no reflection at all. DynamicDependency seems like an overkill but there isn't any other similar tool AFAIK.
There was a problem hiding this comment.
We can maybe discuss this offline - I would be interested in the particulars, since if there are direct references, then there should be no need for DynamicDependency, but this is a very special scenario, so it's likely I don't get some important detail.
There was a problem hiding this comment.
Yes, definitely. It's also quite possible that there is an obvious solution here that I am missing 🤷♂️
There was a problem hiding this comment.
From our offline discussion:
There is actually no hard managed code reference to the UCOs in question - they only have callers from Objective-C. That's why we need DynamicDependency on them to preserve them through the managed trimming process. The platform may be able to remove the unused ones (if we don't add exports for them), but any additional managed world dependencies from them would still be kept around.
I've been looking into ways how we could get rid of custom linker steps in the build process (#17693) and I want to share my ideas. Feedback is appreciated!
cc @rolfbjarne @ivanpovazan