-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Remove char[] allocations from RegistryKey #66918
Conversation
Tagging subscribers to this area: @dotnet/area-microsoft-win32 Issue DetailsReplace char[] allocations with stackalloc'd memory, or ArrayPool as a fallback if the size is too large. I also noticed that the handler for REG_EXPAND_SZ was identical to the handler for REG_SZ save for one block, so I just collapsed the two and added an additional condition to that one block.
|
src/libraries/Microsoft.Win32.Registry/src/Microsoft/Win32/RegistryKey.Windows.cs
Outdated
Show resolved
Hide resolved
src/libraries/Microsoft.Win32.Registry/src/Microsoft/Win32/RegistryKey.Windows.cs
Outdated
Show resolved
Hide resolved
src/libraries/Microsoft.Win32.Registry/src/Microsoft/Win32/RegistryKey.Windows.cs
Outdated
Show resolved
Hide resolved
src/libraries/Microsoft.Win32.Registry/src/Microsoft/Win32/RegistryKey.Windows.cs
Outdated
Show resolved
Hide resolved
src/libraries/Microsoft.Win32.Registry/src/Microsoft/Win32/RegistryKey.Windows.cs
Outdated
Show resolved
Hide resolved
5fdb673
to
32f73e8
Compare
I rewrote InternalGetValueCore. It now starts with a stackalloc'd buffer, and if that's large enough, doesn't make any further calls. It also checks the return value of RegQueryValueEx for any calls it makes, and it removes a bunch of duplicated code. using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;
using Microsoft.Win32;
[MemoryDiagnoser]
public class Program
{
static void Main(string[] args) => BenchmarkSwitcher.FromAssembly(typeof(Program).Assembly).Run(args);
private static readonly RegistryKey s_netFramework = Registry.LocalMachine.OpenSubKey(@"SOFTWARE\Microsoft\.NETFramework");
private static readonly RegistryKey s_hardwareCurrent = Registry.LocalMachine.OpenSubKey(@"System\HardwareConfig\Current");
[Benchmark] public string RegSz() => (string)s_netFramework.GetValue("InstallRoot");
[Benchmark] public int RegDword() => (int)s_netFramework.GetValue("Enable64Bit");
[Benchmark] public string[] RegMultiSz() => (string[])s_hardwareCurrent.GetValue("SystemBiosVersion");
}
|
src/libraries/Microsoft.Win32.Registry/src/Microsoft/Win32/RegistryKey.Windows.cs
Outdated
Show resolved
Hide resolved
Build breaks in CoreLib... |
Ah, I'd searched for this file in *.csproj, forgot *.projitems. |
04857b9
to
36d955d
Compare
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.
Nice!
src/libraries/Common/src/Interop/Windows/Advapi32/Interop.RegQueryValueEx.cs
Outdated
Show resolved
Hide resolved
src/libraries/Microsoft.Win32.Registry/src/Microsoft/Win32/RegistryKey.Windows.cs
Show resolved
Hide resolved
12f8169
to
f3fd756
Compare
097bf82
to
b14f724
Compare
b14f724
to
4b547fa
Compare
@stephentoub are you still working on this one? |
Yes. |
bbbf5d7
to
104caf3
Compare
ee4c949
to
c509ede
Compare
Reduces typical number of syscalls as well as avoids allocation (or uses the ArrayPool otherwise). Also updates Corelib's copy of RegistryKey to match (with things like perf key support removed).
c509ede
to
47a4076
Compare
I believe I finally figured out the issues that were causing failures here; perf keys are special and have undocumented requirements (or at least requirements I couldn't find), like the input buffer has to be zero'd. @jkotas, could you re-review? |
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.
LGTM
Replace char[] allocations with stackalloc'd memory, or ArrayPool as a fallback if the size is too large.
I also noticed that the handler for REG_EXPAND_SZ was identical to the handler for REG_SZ save for one block, so I just collapsed the two and added an additional condition to that one block.