-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Move Marshal memory allocation methods into CoreLib shared partition #41911
Conversation
Depends on #41910 |
The mono changes look ok to me. |
src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.Unix.cs
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.Unix.cs
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.Unix.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.Unix.cs
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.Unix.cs
Show resolved
Hide resolved
The mono/mono mirror PR appears to be passing all required lanes: mono/mono#20353 I don't have context for this PR and so I don't know why it's labeled no-merge, but whenever it's ready the Mono side looks good and I can merge it. |
I was improving tests in other PR that was just merged. Once the CI is green, this can be merged too. |
In addition to benefits from better sharing between runtimes, this change avoids unnecessary layers of wrappers on Unix and makes these methods significantly faster on average for small sizes. For example:
for (int i = 0; i < 100000000; i++) Marshal.FreeHGlobal(Marshal.AllocHGlobal(100));
Before this change: 5430ms
After this change: 2030ms