Skip to content

Conversation

@mjp41
Copy link
Member

@mjp41 mjp41 commented Jan 29, 2026

This is @ryancinsight's PR from snmalloc-rs replayed on the post-merged snmalloc and snmalloc-rs.

This pull request updates the feature configuration in snmalloc-sys/Cargo.toml to enable additional build options and features by default, and introduces several new optional features for more flexible builds.

Feature configuration updates:

The default feature set now includes usewait-on-address, enabling this feature automatically for default builds.
New optional features:

Added several new optional features: tracing, pageid, vendored-stl, check-loads, and gwp-asan, allowing users to enable these capabilities as needed.

Original PR: SchrodingerZhu/snmalloc-rs#203

… 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 microsoft#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.
@mjp41 mjp41 requested a review from SchrodingerZhu January 29, 2026 09:42
@mjp41
Copy link
Member Author

mjp41 commented Jan 29, 2026

@ryancinsight if you have a chance, would you be able to check I have not messed up the merge. Thanks

Comment on lines +23 to 24
- `debug`: Enable the `Debug` mode in `snmalloc`. This is also automatically enabled if Cargo's `DEBUG` environment variable is set to `true`.
- `native-cpu`: Optimize `snmalloc` for the native CPU of the host machine. (this is not a default behavior
Copy link
Contributor

Choose a reason for hiding this comment

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

@SchrodingerZhu did you have a reason to separate snmalloc debug from rust compile, I modified this yesterday base on @mjp41 feedback but seeing this flag in readme is making me question if you had a reason

Copy link
Collaborator

Choose a reason for hiding this comment

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

At that time letting cmake-rs decide debug build was not that prevalent. I agree it is more preferable to have a unified debug build now.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm happy to simplify the options. As we're about to release with a big version number change, Now is a good time to put a breaking change in.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm happy to simplify the options. As we're about to release with a big version number change, Now is a good time to put a breaking change in.

Copy link
Member Author

Choose a reason for hiding this comment

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

I propose we take this as is, and then if someone could simplify the options in a subsequent PR that would be great.

@ryancinsight
Copy link
Contributor

left one comment, otherwise looks good, you may not need the xtask or example, they are kind of simple benchmarks that I was using locally to validate and compare performance of builds

@SchrodingerZhu
Copy link
Collaborator

Also LGTM

@mjp41 mjp41 merged commit 4a169bf into microsoft:main Jan 29, 2026
185 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants