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

Move Marshal memory allocation methods into CoreLib shared partition #41911

Merged
merged 2 commits into from
Sep 8, 2020

Conversation

jkotas
Copy link
Member

@jkotas jkotas commented Sep 5, 2020

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

@jkotas
Copy link
Member Author

jkotas commented Sep 5, 2020

Depends on #41910

@vargaz
Copy link
Contributor

vargaz commented Sep 5, 2020

The mono changes look ok to me.

@CoffeeFlux
Copy link
Contributor

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.

@jkotas jkotas removed the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Sep 7, 2020
@jkotas
Copy link
Member Author

jkotas commented Sep 7, 2020

I don't know why it's labeled no-merge

I was improving tests in other PR that was just merged. Once the CI is green, this can be merged too.

@jkotas jkotas merged commit d81b066 into dotnet:master Sep 8, 2020
@jkotas jkotas deleted the malloc branch September 8, 2020 04:51
@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
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.

7 participants