-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Replace Marshal.AllocHGlobal with NativeMemory.Alloc in cross-platform code #120630
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
base: main
Are you sure you want to change the base?
Conversation
…m code Co-authored-by: stephentoub <[email protected]>
Co-authored-by: stephentoub <[email protected]>
Co-authored-by: stephentoub <[email protected]>
…foImpl.cs Co-authored-by: stephentoub <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR replaces Marshal.AllocHGlobal
/FreeHGlobal
and Marshal.AllocCoTaskMem
/FreeCoTaskMem
with NativeMemory.Alloc
/Free
in cross-platform code to improve performance and consistency.
Key changes:
- JavaScript interop modernization: Updates 9 allocation sites in JavaScript marshaling code for arrays and buffers
- CoreCLR JIT interface optimization: Replaces legacy allocation APIs with standard malloc-based allocation in 5 methods
- Cross-platform consistency: Eliminates platform-specific Windows allocation behavior where not needed
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
JSMarshalerArgument.String.cs | Replaces Marshal allocation calls with NativeMemory for string array marshaling |
JSMarshalerArgument.Object.cs | Updates object array marshaling to use NativeMemory allocation |
JSMarshalerArgument.JSObject.cs | Modernizes JSObject array allocation from Marshal to NativeMemory |
JSMarshalerArgument.Int32.cs | Replaces Marshal calls with NativeMemory for int array marshaling and adds unsafe modifiers |
JSMarshalerArgument.Double.cs | Updates double array marshaling to use NativeMemory and adds unsafe modifiers |
JSMarshalerArgument.Byte.cs | Modernizes byte array allocation from Marshal to NativeMemory and adds unsafe modifiers |
JSProxyContext.cs | Replaces Marshal.FreeHGlobal with NativeMemory.Free for promise holder cleanup |
JSHostImplementation.cs | Updates signature buffer allocation from Marshal to NativeMemory |
JSHostImplementation.Types.cs | Replaces Marshal allocation with NativeMemory for promise holder state |
JSFunctionBinding.cs | Updates async post buffer allocations to use NativeMemory |
CorInfoImpl.cs | Replaces Marshal.AllocCoTaskMem/FreeCoTaskMem and AllocHGlobal/FreeHGlobal with NativeMemory in JIT interface methods |
...nteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/JSFunctionBinding.cs
Show resolved
Hide resolved
…ovider and ThunkGenerator Co-authored-by: jkotas <[email protected]>
src/coreclr/tools/Common/JitInterface/ThunkGenerator/Program.cs
Outdated
Show resolved
Hide resolved
...ropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/JSHostImplementation.cs
Show resolved
Hide resolved
Co-authored-by: jkotas <[email protected]>
src/coreclr/tools/Common/JitInterface/ThunkGenerator/Program.cs
Outdated
Show resolved
Hide resolved
src/coreclr/tools/Common/JitInterface/ThunkGenerator/Program.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: jkotas <[email protected]>
WASM issues here are known |
Summary
Successfully audited and replaced
Marshal.AllocHGlobal
withNativeMemory.Alloc
in cross-platform code where LocalAlloc behavior is not required.Changes Made:
System.Runtime.InteropServices.JavaScript (WASM/browser): 9 allocations + 10 frees
CoreCLR JIT interface (CorInfoImpl.cs): 3 allocations + 5 frees
JitConfigProvider.cs: 2 allocations (replaced AllocCoTaskMem)
ThunkGenerator/Program.cs: 1 allocation (replaced AllocCoTaskMem)
CorInfoImpl_generated.cs: 1 allocation (replaced AllocCoTaskMem)
Total: 16 allocations and 15 frees replaced with
NativeMemory
callsCode Quality Improvements:
sizeof(void*)
instead ofsizeof(IntPtr)
for pointer-sized allocationsExcluded from Changes:
Verification:
NativeMemory.Alloc
/Free
which wrapsmalloc
/free
instead ofLocalAlloc
/CoTaskMemAlloc
Fixes #54297
Original prompt
Fixes #54297
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.