-
Notifications
You must be signed in to change notification settings - Fork 112
Linux/macOS: link mimalloc into executables; unify allocator helper #6183
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: master
Are you sure you want to change the base?
Changes from 7 commits
10a8a04
e87da6b
3b70abb
1c6cc82
355edac
ea2cd68
16d7311
9b06322
43041f8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| # mr_enable_mimalloc(<target>) - enable the mimalloc allocator for an EXE target. | ||
| # EXE-only; call unconditionally (it branches internally). Gated by MESHLIB_USE_MIMALLOC. | ||
| # Verified by MRMesh.MimallocRedirectActive, which keys off the MR_MIMALLOC_ENABLED | ||
| # define this sets. MSBuild twin: MimallocRedirect.props. | ||
| function(mr_enable_mimalloc target) | ||
| if(NOT MESHLIB_USE_MIMALLOC) | ||
| return() | ||
| endif() | ||
|
|
||
| if(EMSCRIPTEN OR MR_EMSCRIPTEN) | ||
| target_link_options(${target} PRIVATE "-sMALLOC=mimalloc") | ||
| return() | ||
| endif() | ||
|
|
||
| if(WIN32) | ||
| # mimalloc.dll must be the FIRST PE import (prepend) so its redirect beats | ||
| # ucrtbase; /INCLUDE:mi_version forces the import. | ||
| find_package(mimalloc CONFIG REQUIRED) | ||
| get_target_property(_existing_libs ${target} LINK_LIBRARIES) | ||
| if(_existing_libs) | ||
| set_property(TARGET ${target} PROPERTY LINK_LIBRARIES mimalloc ${_existing_libs}) | ||
| else() | ||
| set_property(TARGET ${target} PROPERTY LINK_LIBRARIES mimalloc) | ||
| endif() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Aren't both branches functionally equivalent? |
||
| target_link_options(${target} PRIVATE "/INCLUDE:mi_version") | ||
| elseif(APPLE) | ||
| # macOS: static-link mimalloc via -force_load. brew's DYNAMIC mimalloc 3.x aborts | ||
| # at launch on macOS <= 13: an allocation during libSystem init hits mi_thread_init, | ||
| # which touches a thread-local before dyld can bootstrap it -> dyld abort. Static- | ||
| # linking ties mimalloc's init to this exe's own initializers (after libSystem), | ||
| # avoiding that early path. | ||
| find_package(mimalloc CONFIG QUIET) | ||
| if(TARGET mimalloc-static) | ||
| target_link_options(${target} PRIVATE "LINKER:-force_load,$<TARGET_FILE:mimalloc-static>") | ||
| else() | ||
| set(_mi_saved_suffixes ${CMAKE_FIND_LIBRARY_SUFFIXES}) | ||
| set(CMAKE_FIND_LIBRARY_SUFFIXES ".a") | ||
| find_library(MR_MIMALLOC_STATIC NAMES mimalloc REQUIRED) | ||
| set(CMAKE_FIND_LIBRARY_SUFFIXES ${_mi_saved_suffixes}) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wouldn't the previous value be restored automatically since CMake functions create their own variable scopes? |
||
| target_link_options(${target} PRIVATE "LINKER:-force_load,${MR_MIMALLOC_STATIC}") | ||
| endif() | ||
| else() | ||
| # Linux: dynamic link engages the override (ubuntu24 via CONFIG; ubuntu22's | ||
| # libmimalloc-dev ships no config, so fall back to find_library). | ||
| find_package(mimalloc CONFIG QUIET) | ||
| if(TARGET mimalloc) | ||
| target_link_libraries(${target} PRIVATE mimalloc) | ||
| else() | ||
| find_library(MR_MIMALLOC_LIBRARY NAMES mimalloc REQUIRED) | ||
| target_link_libraries(${target} PRIVATE ${MR_MIMALLOC_LIBRARY}) | ||
| endif() | ||
| endif() | ||
|
|
||
| target_compile_definitions(${target} PRIVATE MR_MIMALLOC_ENABLED) | ||
| endfunction() | ||
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,7 @@ libharu | |
| libpng | ||
| libtiff | ||
| libzip | ||
| mimalloc | ||
| ninja | ||
| nlohmann-json | ||
| opencascade | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,6 +28,7 @@ libe57format | |
| libharu | ||
| libjpeg-turbo | ||
| libzip | ||
| mimalloc[override] | ||
| opencascade-minimal | ||
| openctm | ||
| openvdb | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,40 +1,59 @@ | ||
| #include <gtest/gtest.h> | ||
|
|
||
| #ifdef _WIN32 | ||
| #if defined( MR_MIMALLOC_ENABLED ) | ||
|
|
||
| #include <MRMesh/MRMesh.h> | ||
| #include <MRMesh/MRCube.h> | ||
|
|
||
| #include <cstdlib> | ||
| #include <string_view> | ||
|
|
||
| // mimalloc.h declares these as `bool`. Matching the actual return type is | ||
| // critical: MSVC returns `bool` in the low byte of EAX, so reading the call | ||
| // site as `int` picks up garbage in the upper 24 bits (e.g. -980287487). | ||
| // Local Debug happened to zero those bits; Release CI didn't. | ||
| extern "C" bool mi_is_redirected(); | ||
| // Declared here to avoid <mimalloc.h>. Return type MUST be bool, not int: MSVC | ||
| // returns bool in AL, so an int read picks up garbage in the upper bits (broke Release CI). | ||
| extern "C" bool mi_is_in_heap_region( const void* p ); | ||
| #ifdef _WIN32 | ||
| extern "C" bool mi_is_redirected(); | ||
| #endif | ||
|
|
||
| namespace MR | ||
| { | ||
|
|
||
| // Verifies mimalloc's transparent CRT-allocator redirect engaged for this EXE. | ||
| // Wired in MeshLib/source/MimallocRedirect.props (MSBuild) and | ||
| // MeshLib/cmake/Modules/MimallocRedirect.cmake (CMake). Skipped when | ||
| // MIMALLOC_DISABLE_REDIRECT=1 (mimalloc's own runtime kill-switch). | ||
| // Confirms mimalloc services real allocations, not just that the lib is linked. | ||
| // Wired by Mimalloc.cmake (CMake) / MimallocRedirect.props (MSBuild) via the | ||
| // MR_MIMALLOC_ENABLED define. Each allocation below must land in a mimalloc region: | ||
| // C malloc, C++ operator new, and an allocation made INSIDE MRMesh (cross-library - | ||
| // the real goal, what the process-wide override must reach, esp. macOS two-level ns). | ||
| // On Windows also asserts the redirect engaged; MIMALLOC_DISABLE_REDIRECT=1 skips that. | ||
| TEST( MRMesh, MimallocRedirectActive ) | ||
| { | ||
| #ifdef _WIN32 | ||
| if ( const char* disable = std::getenv( "MIMALLOC_DISABLE_REDIRECT" ); | ||
| disable && std::string_view( disable ) == "1" ) | ||
| { | ||
| GTEST_SKIP() << "MIMALLOC_DISABLE_REDIRECT=1; redirect intentionally disabled."; | ||
| } | ||
|
|
||
| EXPECT_TRUE( mi_is_redirected() ); | ||
| #endif | ||
|
|
||
| // C malloc, in this executable | ||
| void* p = std::malloc( 64 ); | ||
| ASSERT_NE( p, nullptr ); | ||
| EXPECT_TRUE( mi_is_in_heap_region( p ) ); | ||
| std::free( p ); | ||
|
|
||
| // C++ operator new, in this executable | ||
| int* q = new int[16]; | ||
| EXPECT_TRUE( mi_is_in_heap_region( q ) ); | ||
| delete[] q; | ||
|
|
||
| // cross-library: makeCube() fills points via operator new compiled into MRMesh, | ||
| // so its buffer must be a mimalloc region too (proves the override reaches the libs) | ||
| const Mesh cube = makeCube(); | ||
| ASSERT_NE( cube.points.data(), nullptr ); | ||
| EXPECT_TRUE( mi_is_in_heap_region( cube.points.data() ) ); | ||
| } | ||
|
|
||
| } // namespace MR | ||
|
|
||
| #endif // _WIN32 | ||
| #endif // MR_MIMALLOC_ENABLED |
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.
Keep
EMSCRIPTENonly.