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.

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

Add a managed static registrar. Fixes #17324. #18268

Merged
merged 68 commits into from May 19, 2023
Merged

Conversation

rolfbjarne
Copy link
Member

@rolfbjarne rolfbjarne commented May 11, 2023

Go grab a ☕️ or a 🫖 first, this is a rather big PR 😉

Add a new version of the static registrar (called the managed static
registrar), which most notably doesn't use metadata tokens (because NativeAOT
doesn't support metadata tokens). In addition, the new registrar also takes
advantage of new features in both C# and the runtime, in order to be more
performant.

I won't go into detail about everything here, because it would be rather long,
but I've added documentation for the new registrar (the first commit, so start
reviewing there).

Fixes #17324.


This PR is best reviewed commit-by-commit. Note that each commit has a
descriptive commit message that describes that change.

@github-actions

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

There are no functional changes here, just code simplification.
…iguration.Report method.

Since LinkerConfiguration.Report uses the trimmer's API to report warnings and errors.
@vs-mobiletools-engineering-service2

This comment has been minimized.

rolfbjarne and others added 9 commits May 11, 2023 12:21
…turn exceptions.

Without having to throw them.
…r on.

There are no functional changes here, just refactoring to make code easier to re-use.
…r on.

There are no functional changes here, just refactoring to make code easier to re-use.
…r on.

There are no functional changes here, just refactoring to make code easier to re-use.
This new mode is still considered a 'Static' registrar mode, it's just a variation of it.
This way we can ask the linker to inline the Runtime.IsManagedStaticRegistrar property, and remove any dead code paths.
@github-actions

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

rolfbjarne and others added 4 commits May 11, 2023 13:10
…egistrar

The managed static registrar will add code to the processed assemblies, which means
it must run before the trimmer sweeps unused code.

This means we have to split the current registrar logic in two:

1. First we process all the assemblies.
2. Then we write out the results.

When not using the managed static registrar, these two steps happens right after
oneanother (like they do now), while when using the managed static registrar, the
processing is done before the trimmer sweeps (where we'll also generate all the new
IL code), and then the generated native code will be done at the end of the build
process (like for the old-school static registrar).
…ed static registrar step can access them.

There are no functional changes here, just refactoring to make code easier to re-use.
Comparing -1 to 0xFFFFFFFF doesn't get the right result otherwise.
…ve-C method signatures.

This is required when generating a cast of a function pointer to an Objective-C method
signature (which the managed static registrar does).
rolfbjarne and others added 9 commits May 18, 2023 11:54
…ime.GetNSObject

We can't call the generic Runtime.GetNSObject<T> method when we don't know the final
type of the returned object, which happens for open types. However, we must always
call the generic Runtime.GetNSObject<T> when we know the final type, so that we have
the correct object type on the evaluation stack.

Fix this by properly detecting open types and only calling Runtime.GetNSObject in
that case.
…voke.

Keep better track of whether we're calling MethodBase.Invoke to call the target method,
which allows us to detect certain error scenarios (and show a corresponding error).
… 'object' reference is left on the stack.

Keep track of when we called MethodBase.Invoke and an 'object' reference is left
on the stack, so that we can cast to the appropiate type later on when we know how
the value is converted to the corresponding native type.

This fixes a few cases of the managed static registrar creating unverifiable IL.
…ypes.

We can't create an instance of a generic type in the UnmanagedCallersOnly trampoline,
so simplify the code to not even try, and instead just throw the final exception.
@github-actions
Copy link
Contributor

⚠️ Your code has been reformatted. ⚠️

If this is not desired, add the actions-disable-autoformat label, and revert the reformatting commit.

If files unrelated to your change were modified, try reverting the reformatting commit + merging with the target branch (and push those changes).

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2
Copy link
Collaborator

📚 [PR Build] Artifacts 📚

Packages generated

View packages

Pipeline on Agent
Hash: [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

✅ API diff for current PR / commit

Legacy Xamarin (No breaking changes)
  • iOS (no change detected)
  • tvOS (no change detected)
  • watchOS (no change detected)
  • macOS (no change detected)
.NET (No breaking changes)

✅ API diff vs stable

Legacy Xamarin (No breaking changes)
.NET (No breaking changes)
Legacy Xamarin (stable) vs .NET

ℹ️ Generator diff

Generator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes)

Pipeline on Agent
Hash: 65590ba8c0d82b49de7aa7312720fd7b7c42f09d [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

💻 [PR Build] Tests on macOS M1 - Mac Big Sur (11.5) passed 💻

All tests on macOS M1 - Mac Big Sur (11.5) passed.

Pipeline on Agent
Hash: [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

💻 [PR Build] Tests on macOS M1 - Mac Ventura (13.0) passed 💻

All tests on macOS M1 - Mac Ventura (13.0) passed.

Pipeline on Agent
Hash: [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

💻 [CI Build] Windows Integration Tests passed 💻

All Windows Integration Tests passed.

Pipeline on Agent
Hash: 65590ba8c0d82b49de7aa7312720fd7b7c42f09d [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

🚀 [CI Build] Test results 🚀

Test results

✅ All tests passed on VSTS: simulator tests.

🎉 All 235 tests passed 🎉

Tests counts

✅ bcl: All 69 tests passed. Html Report (VSDrops) Download
✅ cecil: All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests: All 1 tests passed. Html Report (VSDrops) Download
✅ fsharp: All 7 tests passed. Html Report (VSDrops) Download
✅ framework: All 8 tests passed. Html Report (VSDrops) Download
✅ generator: All 2 tests passed. Html Report (VSDrops) Download
✅ interdependent_binding_projects: All 7 tests passed. Html Report (VSDrops) Download
✅ install_source: All 1 tests passed. Html Report (VSDrops) Download
✅ introspection: All 8 tests passed. Html Report (VSDrops) Download
✅ linker: All 65 tests passed. Html Report (VSDrops) Download
✅ mac_binding_project: All 1 tests passed. Html Report (VSDrops) Download
✅ mmp: All 2 tests passed. Html Report (VSDrops) Download
✅ mononative: All 12 tests passed. Html Report (VSDrops) Download
✅ monotouch: All 35 tests passed. Html Report (VSDrops) Download
✅ msbuild: All 2 tests passed. Html Report (VSDrops) Download
✅ mtouch: All 1 tests passed. Html Report (VSDrops) Download
✅ xammac: All 3 tests passed. Html Report (VSDrops) Download
✅ xcframework: All 8 tests passed. Html Report (VSDrops) Download
✅ xtro: All 2 tests passed. Html Report (VSDrops) Download

Pipeline on Agent
Hash: 65590ba8c0d82b49de7aa7312720fd7b7c42f09d [PR build]

Copy link
Contributor

@tj-devel709 tj-devel709 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Things LGTM! Thank you for taking the time to split things by commit, helped make things more clear as reviewing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make the static registrar work with NativeAOT