Skip to content

Replace usages of Marshal.AllocHGlobal with malloc in libraries #54468

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.

Sign up for GitHub

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

Closed
reflectronic wants to merge 63 commits into from
Closed

Replace usages of Marshal.AllocHGlobal with malloc in libraries #54468

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
63 commits
Select commit Hold shift + click to select a range
10f15f7
System.Private.CoreLib
reflectronic Jun 20, 2021
61340eb
System.Net.Security
reflectronic Jun 20, 2021
96d410b
System.Net.NameResolution
reflectronic Jun 20, 2021
f93fe74
System.Net.NetworkInformation
reflectronic Jun 20, 2021
6badbdd
System.Net.Quic
reflectronic Jun 20, 2021
d2b561f
System.Net.HttpListener
reflectronic Jun 20, 2021
38824c5
nint casts
reflectronic Jun 20, 2021
8ec610e
System.Text.Encoding.CodePages
reflectronic Jun 20, 2021
2db0e86
intptr casts
reflectronic Jun 20, 2021
658bf85
System.Speech
reflectronic Jun 20, 2021
0930248
System.ServiceProcess.ServiceController
reflectronic Jun 20, 2021
89ded43
NativeMemoryHelper in System.Speech
reflectronic Jun 20, 2021
340735a
NativeMemoryHelper System.Text.Encoding.CodePages
reflectronic Jun 20, 2021
7038fca
Crypto
reflectronic Jun 20, 2021
453f15e
System.Reflection.Metadata
reflectronic Jun 20, 2021
325b20d
System.Net.Http.WinHttpHandler
reflectronic Jun 20, 2021
36c48a4
System.Management
reflectronic Jun 20, 2021
09d0361
System.Diagnostics.EventLog
reflectronic Jun 20, 2021
190fb6b
System.Drawing.Common
reflectronic Jun 20, 2021
673925a
System.DirectoryServices.Protocols
reflectronic Jun 20, 2021
29e37a5
System.DirectoryServices.AccountManagement
reflectronic Jun 20, 2021
2510792
System.DirectoryServices
reflectronic Jun 20, 2021
935a77a
System.Data.Odbc
reflectronic Jun 20, 2021
68a9dc7
System.Diagnostics.PerformanceCounter
reflectronic Jun 20, 2021
443c362
more System.Drawing.Common
reflectronic Jun 20, 2021
5dd3cf6
more System.Private.CoreLib
reflectronic Jun 21, 2021
8783e8c
Fix build break
reflectronic Jun 21, 2021
3184c15
Fix additional embarrassing mistakes
reflectronic Jun 21, 2021
6a0746a
Fix mistake
reflectronic Jun 21, 2021
59cfa9b
fixing
reflectronic Jun 21, 2021
f8d77c2
fix project files
reflectronic Jun 21, 2021
8bc626d
powershell doesn't know how to preserve BOM
reflectronic Jun 21, 2021
d52bb61
fix project files
reflectronic Jun 23, 2021
aa86b70
does SRM csproj have BOM?
reflectronic Jun 23, 2021
e6b6e8e
more csproj fixes
reflectronic Jun 23, 2021
f0fb97a
one last time
reflectronic Jun 23, 2021
3423d98
Some changes
reflectronic Jun 23, 2021
b5dd0da
revert System.DirectoryServices.Protocols
reflectronic Jun 30, 2021
8a1f781
revert System.DirectoryServices.AccountManagement
reflectronic Jun 30, 2021
6fe4d7c
revert System.DirectoryServices
reflectronic Jun 30, 2021
c2275fc
fix corelib and other stuff
reflectronic Jun 30, 2021
068248c
revert System.Drawing.Common
reflectronic Jun 30, 2021
b875528
fix SafeNativeMemoryHandle
reflectronic Jun 30, 2021
f89bc04
Sysem.Text.Encoding.CodePages safenativememoryhandle
reflectronic Jun 30, 2021
2a961d3
fixxxx
reflectronic Jun 30, 2021
1b0f51a
do it again
reflectronic Jun 30, 2021
af272a8
Improve crypto
reflectronic Jun 30, 2021
a2042c6
EventLog and other stuff
reflectronic Jul 14, 2021
6e57302
misc other updates
reflectronic Jul 14, 2021
db1ee0c
Back out System.ServiceProcess
reflectronic Jul 14, 2021
e79d136
final fix perhaps
reflectronic Jul 14, 2021
b7c3292
Merge branch 'main' into replace-allochglobal
reflectronic Jul 14, 2021
7156b1e
source.dot.net lied to me
reflectronic Jul 14, 2021
3ea5947
Update MsQuicStream.cs
reflectronic Jul 14, 2021
fc6147a
fix
reflectronic Jul 14, 2021
1b3e561
Compile errors
reflectronic Jul 30, 2021
1f60dbf
Another fix
reflectronic Jul 30, 2021
852508b
Fix
reflectronic Jul 30, 2021
332e3aa
add unsafe
reflectronic Jul 30, 2021
de3481e
revert ldap interop
reflectronic Jul 30, 2021
8d3fbdd
temp change
reflectronic Jul 30, 2021
c31c674
improvement
reflectronic Jul 30, 2021
8a12cec
add unsafe
reflectronic Jul 30, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
NativeMemoryHelper in System.Speech
  • Loading branch information
@reflectronic
reflectronic committed Jun 20, 2021
commit 89ded43085029b66a623aab5314a481fbddafc41
33 changes: 27 additions & 6 deletions src/libraries/Common/src/System/NativeMemoryHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,32 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Runtime.InteropServices;

namespace System
{
internal static class NativeMemoryHelper
internal static unsafe class NativeMemoryHelper
Copy link
Member

@jkotas jkotas Jun 21, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

This should be called NativeMemory and only included for pre-.NET 6 targets. We do not want to have the wrapper on .NET 6+.

Copy link
Contributor Author

@reflectronic reflectronic Jun 21, 2021

Choose a reason for hiding this comment

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

That can work for Alloc and Free, but it also has some replacements for Marshal.StringToHGlobalUni that would have to stay on a type like this. Is that okay?

Copy link
Member

@jkotas jkotas Jun 21, 2021

Choose a reason for hiding this comment

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

I would avoid touching StringToHGlobalUni in this PR. If it is useful to have NativeMemory variant of StringToHGlobalUni and similar APIs, we should introduce a proper public API for it first. Duplicating NativeMemory variant of StringToHGlobalUni via private helper is not an improvement over using StringToHGlobalUni directly.

Copy link
Member

@tannergooding tannergooding Jun 21, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

This might be worth profiling more and its probably worth opening a proposal.

On my box, a benchmark which does simply the following, over a 260 character string:

var ptr = Marshal.StringToHGlobalUni(s);
Marshal.FreeHGlobal(ptr);

Compared to an equivalent API using NativeMarshal.Alloc/Free under the covers:

BenchmarkDotNet=v0.13.0, OS=Windows 10.0.19043.1052 (21H1/May2021Update)
AMD Ryzen 9 5950X, 1 CPU, 32 logical and 16 physical cores
.NET SDK=6.0.100-preview.5.21302.13
  [Host]     : .NET 6.0.0 (6.0.21.30105), X64 RyuJIT
  DefaultJob : .NET 6.0.0 (6.0.21.30105), X64 RyuJIT

|  Method |     Mean |    Error |   StdDev |
|-------- |---------:|---------:|---------:|
|   Alloc | 41.92 ns | 0.051 ns | 0.045 ns |
| HGlobal | 47.37 ns | 0.071 ns | 0.066 ns |

Of course, in comparison to the GC transition and any work the call itself does, this is probably not that meaningful but ~10% overhead can add up.

Copy link
Member

@tannergooding tannergooding Jun 21, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

  • To clarify, I don't think anything needs to be done in this PR, just calling out there is a measurable difference here and so it might be worth opening a proposal because of that. In more extreme cases, many Alloc calls can be up to 20x faster than many LocalAlloc calls.

{
public static unsafe IntPtr Alloc(int size)
public static IntPtr Alloc(int byteCount)
{
#if NET6_0_OR_GREATER
return (nint)NativeMemory.Alloc((uint)size);
return (nint)NativeMemory.Alloc((uint)byteCount);
#else
return Marshal.AllocHGlobal(size);
return Marshal.AllocHGlobal(byteCount);
#endif
}

public static unsafe IntPtr AllocStringUnicode(string? s)
public static IntPtr Realloc(IntPtr pointer, int byteCount)
{
#if NET6_0_OR_GREATER
return (nint)NativeMemory.Realloc((void*)(nint)pointer, (uint)byteCount);
#else
return Marshal.ReAllocHGlobal(pointer, (nint)byteCount);
#endif
}

public static IntPtr AllocStringUnicode(string? s)
{
if (s is null)
{
Expand All @@ -29,7 +39,10 @@ public static unsafe IntPtr AllocStringUnicode(string? s)
// Overflow checking
if (byteCount < s.Length)
{
#if NET6_0_OR_GREATER
[StackTraceHidden]
#endif
[DoesNotReturn]
static void ThrowArgumentOutOfRangeException(string argument)
{
throw new ArgumentOutOfRangeException(argument);
Expand All @@ -44,13 +57,21 @@ static void ThrowArgumentOutOfRangeException(string argument)
char* memory = (char*)(nint)Marshal.AllocHGlobal(byteCount);
#endif

#if NET6_0_OR_GREATER
s.CopyTo(new Span<char>(memory, s.Length));
#else
// Avoid pulling in System.Memory for netstandard2.0 targets.
fixed (char* str = s)
{
Buffer.MemoryCopy(str, memory, byteCount, s.Length * 2);
}
#endif
memory[s.Length] = '\0';
return (nint)memory;
}


public static unsafe void Free(IntPtr pointer)
public static void Free(IntPtr pointer)
{
#if NET6_0_OR_GREATER
NativeMemory.Free((void*)(nint)pointer);
Expand Down
18 changes: 3 additions & 15 deletions src/libraries/System.Speech/src/Internal/NativeMemorySafeHandle.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,19 +39,11 @@ internal unsafe IntPtr Buffer(int size)
{
if (_bufferSize == 0)
{
#if NET6_0_OR_GREATER
SetHandle((nint)NativeMemory.Alloc((uint)size));
#else
SetHandle(Marshal.AllocHGlobal(size));
#endif
SetHandle(NativeMemoryHelper.Alloc(size));
}
else
{
#if NET6_0_OR_GREATER
SetHandle((nint)NativeMemory.Realloc((void*)(nint)handle, (uint)size));
#else
SetHandle(Marshal.ReAllocHGlobal(handle, (IntPtr)size));
#endif
SetHandle(NativeMemoryHelper.Realloc(handle, size));
}

GC.AddMemoryPressure(size - _bufferSize);
Expand Down Expand Up @@ -84,11 +76,7 @@ protected override unsafe bool ReleaseHandle()
if (handle != IntPtr.Zero)
{
_bufferSize = 0;
#if NET6_0_OR_GREATER
NativeMemory.Free((void*)(nint)handle);
#else
Marshal.FreeHGlobal(handle);
#endif
NativeMemoryHelper.Free(handle);
handle = IntPtr.Zero;

return true;
Expand Down
3 changes: 3 additions & 0 deletions src/libraries/System.Speech/src/System.Speech.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
<NoWarn Condition="'$(TargetFramework)' == '$(NetCoreAppCurrent)-windows'">$(NoWarn);IDE0079</NoWarn>
</PropertyGroup>
<ItemGroup Condition="'$(TargetsWindows)' == 'true'">
<Compile Include="$(CommonPath)System\NativeMemoryHelper.cs"
Link="Common\System\NativeMemoryHelper.cs" />
<Compile Include="SR.cs" />
<Compile Include="SRID.cs" />
<Compile Include="AudioFormat\AudioFormatConverter.cs" />
Expand Down Expand Up @@ -235,6 +237,7 @@
<Reference Include="System.Diagnostics.Debug" />
<Reference Include="System.Diagnostics.TraceSource" />
<Reference Include="System.IO.FileSystem" />
<Reference Include="System.Memory" />
<Reference Include="System.Net.Primitives" />
<Reference Include="System.Net.Requests" />
<Reference Include="System.Net.WebClient" />
Expand Down