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
RFC: Use mimalloc to avoid freelist woes on *nix #642
Comments
I'd If I'm reading the heap doc right, then mimalloc might not help with the concurrency issues at all?
|
Ah, that's unfortunate... |
My Running interactively on Win32, it seems to work just as well as the clean version; I never got a window on my Buster box, so no interactive comparisons there. Benchmarks say
Granted, I have zero idea if this data is of any use at all, but. |
Would you try it with OpenMP re-enabled? In other words revert this a80a033 |
Even with the diff below for solvespace to link to libgomp, it didn't go above 1 CPU on Windows (as expected, I guess?): diff --git a/CMakeLists.txt b/CMakeLists.txt
index 03baff2..4348add 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -99,7 +99,7 @@ endif()
# We use OpenMP to speed up some geometric operations, but it's optional.
include(FindOpenMP)
# No MinGW distribution actually ships an OpenMP runtime, but FindOpenMP detects it anyway.
-if(OpenMP_FOUND AND NOT MINGW)
+if(OpenMP_FOUND)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${OpenMP_CXX_FLAGS}")
else()
message(WARNING "OpenMP not found, geometric operations will be single-threaded") Buster slammed all 24 CPUs out of the box.
|
Thanks! So minmalloc appears to work fine with OpenMP enabled. Is this not contrary to the statement above that you (@nabijaczleweli)pointed out? Perhaps run it with sanitizers to see if some problems are detected? |
Well, the documentation does say not to do this, so presumably it's just a race to the bottom. I reconfigured with
back, reran the benchmark, now agonisingly slow, and:
|
Does it do the same thing without mimalloc? I might just not be cleaning up enough stuff in the benchmark runner. |
It yields this, which is identical, save for the addresses and times
|
Yeah so this is a bug(ish) in the benchmark runner. Maybe worth asking the authors of mimalloc if that's still a restriction or might have been fixed? |
Came across microsoft/mimalloc#96 (September 2019):
Expanded in microsoft/mimalloc#214 (March):
This sounds like a deeply-ingrained design decision we're not hitting the corner of, so I wouldn't really get my hopes up? |
Okay, so you can wrap |
…ned to a single thread by design Ref: solvespace#642 (comment)
Something like nabijaczleweli@dc98046? |
Well... no. Sure, the worker threads all clean up after themselves once they quit, but the main thread doesn't, so you still do need |
Ah yeah, didn't think about that; nabijaczleweli@f123199? |
Something like that |
The heaps are wrapped in a RAIIish thread_local handler, since being affined affined to a single thread for allocations is required by the API Ref: solvespace#642
The heaps are wrapped in a RAIIish thread_local handler, since being affined affined to a single thread for allocations is required by the API Ref: solvespace#642
The heaps are wrapped in a RAIIish thread_local handler, since being affined affined to a single thread for allocations is required by the API Ref: solvespace#642
The heaps are wrapped in a RAIIish thread_local handler, since being affined affined to a single thread for allocations is required by the API Ref: #642
Right now we implement
AllocTemporary
andFreeAllTemporary
on *nix using a freelist. This does work but it is strictly worse than what happens on Windows, which is unfortunate because I'd like to not privilege any platforms. There are also some multithreading related issues that are currently solved in not especially elegant way.It looks like mimalloc is pretty much perfect:
One thing to note is their performance claims. It's not a bad allocator but the revolutionary claims should be taken with a grain of salt.
The text was updated successfully, but these errors were encountered: