Conversation
ryancinsight
force-pushed
the
wait-on-address-default
branch
from
36dad29 to
22f2268
Compare
|
We can ignore the FreeBSD warning for now. Notice that Matt is working on a separate porting process upstream. |
|
I used xtask to compare builds but it may need to be more robust: === Benchmarking build_cmake (Default, WaitOnAddress enabled) === === Results Comparison === |
however this is roughly 3-4 times slower than windows which I didnt expect...Running contention benchmark with 24 threads, 1000000 iterations per thread === Results Comparison === |
|
Both have WaitOnAddress on and build_cc is faster? That is kind of unexpected. |
|
I'm chalking that up to noise, other runs show negligible difference |
mjp41
left a comment
mjp41
left a comment
There was a problem hiding this comment.
LGTM, thanks for doing a fix so quickly.
|
@ryancinsight is it possible for you to sign the commits? That appears to be a policy that has been set on this repo. |
|
So I have made some changes upstream to allow the merge of snmalloc-rs. I replayed your changes with cherry picking onto what I think will become upstream main, and there was just one small conflict. I am happy to finish that on the snmalloc repo. @ryancinsight it still has you as the author of all the commits, but if you would prefer to PR yourself against the new main, then let me know. |
… agnosticism Added missing features to BuildFeatures struct and implemented conditional defines for correct handling across cc and cmake build systems.
…erf regression Ensures assertions and expensive checks are disabled in release builds when using build_cc, addressing issue SchrodingerZhu#202.
Enables WaitOnAddress support (using Futex) for build_cc on Linux, resolving the spinning behavior issue. Also defines SNMALLOC_HAS_LINUX_RANDOM_H and SNMALLOC_PLATFORM_HAS_GETENTROPY which are standard on modern Linux.
…ild_cc only Reverts NDEBUG change that broke cmake builds. Also guards SNMALLOC_HAS_LINUX_FUTEX_H and related definitions with #[cfg(feature = 'build_cc')] to prevent build errors when using cmake (default).
Explicitly sets WINVER/_WIN32_WINNT to 0x0A00 (Win10) by default to enable VirtualAlloc2 and WaitOnAddress. Sets to 0x0603 (Win8.1) if win8compat feature is enabled. Matches snmalloc CMake behavior and ensures WaitOnAddress is available.
Adds 'usewait-on-address' to default features in Cargo.toml. This aligns the Rust build behavior with upstream CMake defaults, where SNMALLOC_ENABLE_WAIT_ON_ADDRESS is ON by default. Prevents silent performance regression (spinning vs futex) for users relying on default features.
Add a new workspace member `xtask` containing a CLI tool to compare performance between the `build_cc` and `build_cmake` backends. The tool automates building, running a contention benchmark, and parsing throughput results to verify performance equivalence. Also includes the new benchmark example `bench_contention` which allocates and deallocates memory across a ring of threads to stress the allocator.
Updates build.rs to use compiler flags (-D) instead of CMake variables for WINVER and _WIN32_WINNT when using the cmake builder. This ensures these critical definitions reach the snmalloc C++ compiler, enabling WaitOnAddress (Futex) support on Windows and resolving a performance discrepancy where build_cmake was ~20% slower than build_cc.
Refactor the platform-specific configuration in build.rs by replacing match blocks with if/else statements for better clarity. This change consolidates Windows and Unix flag handling, fixes MSVC flag definitions, and adds specific support for FreeBSD.
Update the build script to honor CARGO_CFG_TARGET_OS and OPT_LEVEL environment variables. This ensures that snmalloc is compiled with settings consistent with the rest of the Rust project, rather than relying solely on the "debug" feature flag.
ryancinsight
force-pushed
the
wait-on-address-default
branch
from
5eb7579 to
5547e8b
Compare
Thanks for the effort in replaying these changes and handling the conflict resolution. I’ve just finished a final audit of the wait-on-address-default branch to ensure the implementation is architecturally sound—specifically, I've aligned the snmalloc build profiles and Windows CRT selection with Cargo's environment variables to prevent runtime inconsistencies. I've also signed all 10 commits to satisfy the repository's verification policy. I'm happy for you to proceed with the merge on the upstream repo, and I appreciate you preserving the authorship. If you’d like to pull the latest signed commits and refinements before finalizing, they are available on my fork. Otherwise, feel free to finish the integration as you see fit. |
|
Excellent, I'll replay your commits onto snmalloc tomorrow. Thanks for doing this |
3 participants
This pull request updates the feature configuration in
snmalloc-sys/Cargo.tomlto enable additional build options and features by default, and introduces several new optional features for more flexible builds.Feature configuration updates:
defaultfeature set now includesusewait-on-address, enabling this feature automatically for default builds.New optional features:
tracing,pageid,vendored-stl,check-loads, andgwp-asan, allowing users to enable these capabilities as needed.