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

[MONO] Move Marshal-ilgen into a component #75542

Merged
merged 17 commits into from
Nov 15, 2022

Conversation

naricc
Copy link
Contributor

@naricc naricc commented Sep 13, 2022

Move marhsal-ilgen into a component, ultimately saving approximately 100kb in the runtime when the component is not needed. This addresses issues seen on iOS last time this was attempted, and fixes a race condition.

I am still doing manual testing on xamarin-ios and xamarin-android, but think this is ready for review.

@ghost ghost assigned naricc Sep 13, 2022
@naricc naricc force-pushed the naricc/marshal-component branch 2 times, most recently from 9587a95 to f84b181 Compare October 17, 2022 19:47
@naricc naricc changed the title [MONO][DRAFT] Move Marshal-ilgen into a component [MONO] Move Marshal-ilgen into a component Oct 17, 2022
@naricc naricc marked this pull request as ready for review October 17, 2022 20:07
@lambdageek
Copy link
Member

@naricc you need to resolve the merge conflicts before the CI pipelines will run

@naricc
Copy link
Contributor Author

naricc commented Nov 2, 2022

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

1 similar comment
@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@naricc
Copy link
Contributor Author

naricc commented Nov 3, 2022

/azp run runtime-extra-platforms

@naricc
Copy link
Contributor Author

naricc commented Nov 3, 2022

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

1 similar comment
@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@naricc
Copy link
Contributor Author

naricc commented Nov 14, 2022

/azp run runtime-extra-platforms

@naricc
Copy link
Contributor Author

naricc commented Nov 14, 2022

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

1 similar comment
@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@naricc
Copy link
Contributor Author

naricc commented Nov 15, 2022

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@naricc
Copy link
Contributor Author

naricc commented Nov 15, 2022

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@lambdageek lambdageek left a comment

Choose a reason for hiding this comment

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

LGTM. I'm excited that we finally have a chance to clean things up here

src/mono/mono/metadata/marshal.h Show resolved Hide resolved
@lambdageek

This comment was marked as resolved.

@naricc
Copy link
Contributor Author

naricc commented Nov 15, 2022

/azp run runtime-wasm

@naricc
Copy link
Contributor Author

naricc commented Nov 15, 2022

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

1 similar comment
@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@naricc naricc merged commit 3cfd6cf into dotnet:main Nov 15, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Dec 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants