Win32: use CRT heap instead of our own.

whitequark · whitequark · commit 521473ee4bde · 2020-05-10T05:35:42.000Z
Historically SolveSpace used its own heap on Windows since it gave better control and debugging options, but a lot of development these days happens on Linux, where that heap was a stub around malloc/free, and also Windows debugging tools got a lot better. In terms of immediate benefit, this commit fixes heap corruption on Windows introduced in commits b4e1ce4 and 47e8279, caused by allocating with HEAP_NO_SERIALIZE in parallel from OpenMP threads. Without HEAP_NO_SERIALIZE there's no performance benefit to keeping our own heap, either. The vl() function is also removed because for development there are better tools now, and the only place where it was permanently called from became a no-op, since temporary heap always validates after FreeAllTemporary() recreates it.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -98,7 +98,8 @@ endif()
 
 # We use OpenMP to speed up some geometric operations, but it's optional.
 include(FindOpenMP)
-if(OpenMP_FOUND)
+# No MinGW distribution actually ships an OpenMP runtime, but FindOpenMP detects it anyway.
+if(OpenMP_FOUND AND NOT MINGW)
     set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${OpenMP_CXX_FLAGS}")
 else()
     message(WARNING "OpenMP not found, geometric operations will be single-threaded")
diff --git a/src/platform/utilwin.cpp b/src/platform/utilwin.cpp
@@ -13,7 +13,7 @@
 #include <shellapi.h>
 
 namespace SolveSpace {
-static HANDLE PermHeap, TempHeap;
+static HANDLE TempHeap;
 
 void dbp(const char *str, ...)
 {
@@ -51,23 +51,15 @@ void FreeAllTemporary()
 {
     if(TempHeap) HeapDestroy(TempHeap);
     TempHeap = HeapCreate(HEAP_NO_SERIALIZE, 1024*1024*20, 0);
-    // This is a good place to validate, because it gets called fairly
-    // often.
-    vl();
 }
 
 void *MemAlloc(size_t n) {
-    void *p = HeapAlloc(PermHeap, HEAP_NO_SERIALIZE | HEAP_ZERO_MEMORY, n);
+    void *p = malloc(n);
     ssassert(p != NULL, "Cannot allocate memory");
     return p;
 }
 void MemFree(void *p) {
-    HeapFree(PermHeap, HEAP_NO_SERIALIZE, p);
-}
-
-void vl() {
-    ssassert(HeapValidate(TempHeap, HEAP_NO_SERIALIZE, NULL), "Corrupted heap");
-    ssassert(HeapValidate(PermHeap, HEAP_NO_SERIALIZE, NULL), "Corrupted heap");
+    free(p);
 }
 
 std::vector<std::string> InitPlatform(int argc, char **argv) {
@@ -81,8 +73,6 @@ std::vector<std::string> InitPlatform(int argc, char **argv) {
     }
 #endif
 
-    // Create the heap used for long-lived stuff (that gets freed piecewise).
-    PermHeap = HeapCreate(HEAP_NO_SERIALIZE, 1024*1024*20, 0);
     // Create the heap that we use to store Exprs and other temp stuff.
     FreeAllTemporary();
 
diff --git a/src/solvespace.h b/src/solvespace.h
@@ -152,7 +152,6 @@ void *AllocTemporary(size_t n);
 void FreeAllTemporary();
 void *MemAlloc(size_t n);
 void MemFree(void *p);
-void vl(); // debug function to validate heaps
 
 // End of platform-specific functions
 //================
