-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Speed up Guid.NewGuid() on Linux #123540
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?
Speed up Guid.NewGuid() on Linux #123540
Conversation
… to offset generation costs
|
@reedz please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement ( “Agreement” ) is agreed to by the party signing below ( “You” ), 1. Definitions. “Code” means the computer software code, whether in human-readable or machine-executable form, “Project” means any of the projects owned or managed by .NET Foundation and offered under a license “Submit” is the act of uploading, submitting, transmitting, or distributing code or other content to any “Submission” means the Code and any other copyrightable material Submitted by You, including any 2. Your Submission. You must agree to the terms of this Agreement before making a Submission to any 3. Originality of Work. You represent that each of Your Submissions is entirely Your 4. Your Employer. References to “employer” in this Agreement include Your employer or anyone else 5. Licenses. a. Copyright License. You grant .NET Foundation, and those who receive the Submission directly b. Patent License. You grant .NET Foundation, and those who receive the Submission directly or c. Other Rights Reserved. Each party reserves all rights not expressly granted in this Agreement. 6. Representations and Warranties. You represent that You are legally entitled to grant the above 7. Notice to .NET Foundation. You agree to notify .NET Foundation in writing of any facts or 8. Information about Submissions. You agree that contributions to Projects and information about 9. Governing Law/Jurisdiction. This Agreement is governed by the laws of the State of Washington, and 10. Entire Agreement/Assignment. This Agreement is the entire agreement between the parties, and .NET Foundation dedicates this Contribution License Agreement to the public domain according to the Creative Commons CC0 1. |
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 optimizes Guid.NewGuid() on Unix, primarily targeting Linux, by reducing syscall overhead and reusing entropy to significantly improve performance while maintaining cryptographic-strength randomness.
Changes:
- Teach the native random infrastructure to use
getrandom()(when available) instead of/dev/urandom, reducing per-call overhead for cryptographically secure random bytes. - Add CMake configuration and header wiring to detect and expose the availability of
getrandom. - Reimplement
Guid.NewGuid()on Unix to use a per-thread cache of GUIDs filled in batches via a single secure-random call, including logic to correctly set RFC 4122 version and variant bits for each cached GUID.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/native/minipal/random.c |
Adds a HAVE_GETRANDOM-guarded implementation of minipal_get_cryptographically_secure_random_bytes using the getrandom() syscall, looping until the requested buffer is fully populated and handling EINTR, to provide a faster secure RNG backend than /dev/urandom. |
src/native/minipal/minipalconfig.h.in |
Introduces the HAVE_GETRANDOM configuration macro so native code can be conditionally compiled against getrandom() support. |
src/native/minipal/configure.cmake |
Extends the CMake configuration to probe for getrandom in sys/random.h, defining HAVE_GETRANDOM when present so the new native path in random.c is enabled on supported Unix platforms. |
src/libraries/System.Private.CoreLib/src/System/Guid.Unix.cs |
Replaces the per-call secure RNG usage with a [ThreadStatic] cache of 64 GUIDs, filled via a single Interop.GetCryptographicallySecureRandomBytes call and post-processed to set version/variant bits, and updates NewGuid() to draw from this cache (with a WASI-specific non-cached fallback). |
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using System.Runtime.CompilerServices; | ||
| using System.Runtime.InteropServices; |
Copilot
AI
Jan 23, 2026
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.
System.Runtime.InteropServices does not appear to be used in this file anymore; consider removing this using to avoid an unnecessary import and keep the file consistent with the usual style of only including needed namespaces.
| using System.Runtime.InteropServices; |
|
Managed side cache of secure RNG doesn't sound secure. If NewGuid becomes a bottleneck there are other strategies to make it fast |
| public partial struct Guid | ||
| { | ||
| // Batch size for cached GUID generation. Chosen to balance memory usage vs syscall overhead. | ||
| // 64 GUIDs = 1024 bytes, which provides good amortization of the syscall cost while |
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.
1kB per thread cache just for GUIDs feels very expensive.
src/native/minipal/random.c
Outdated
| NTSTATUS status = BCryptGenRandom(NULL, buffer, (ULONG)bufferLength, BCRYPT_USE_SYSTEM_PREFERRED_RNG); | ||
| return BCRYPT_SUCCESS(status) ? 0 : -1; | ||
| #elif HAVE_GETRANDOM | ||
| // Use getrandom() syscall which is faster than reading from /dev/urandom |
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.
This part of the change looks good.
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.
Would you mind reverting the contentious cache and focus this PR on just using getrandom when it is available?
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.
Should we fall back to /dev/urandom for some errors? In particular, I was thinking should consider handling ENOSYS and EPERM. Some container environments or seccomp do, or did, not pass through that syscall.
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.
Sure thing!
I've reverted managed changes, and updated the getrandom branch to fall back to /dev/urandom on ENOSYS and EPERM returns.
| #if HAVE_GETRANDOM | ||
| // Try getrandom() first - it's faster than /dev/urandom as it avoids file descriptor overhead. | ||
| // getrandom() was added in Linux 3.17 (2014) and glibc 2.25 (2017). | ||
| static volatile bool sGetrandomUnavailable; |
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.
nit: could we use the same terminology as is there below for devurandom, e.g. sMissingGetrandom?
Fixes #13628.
This PR implements two optimizations:
getrandom()syscall (if available) instead of reading from/dev/urandomis ~14% quicker >getrandom()call as performed inRandom.SharedBenchmark dot net benchmark is showing the following changes: