merge from dev-reset

include/mimalloc/prim.h

@@ -37,12 +37,21 @@ int _mi_prim_free(void* addr, size_t size );
   
 // Allocate OS memory. Return NULL on error.
 // The `try_alignment` is just a hint and the returned pointer does not have to be aligned.
+// If `commit` is false, the virtual memory range only needs to be reserved (with no access) 
+// which will later be committed explicitly using `_mi_prim_commit`.
 // pre: !commit => !allow_large
 //      try_alignment >= _mi_os_page_size() and a power of 2
 int _mi_prim_alloc(size_t size, size_t try_alignment, bool commit, bool allow_large, bool* is_large, void** addr);
 
 // Commit memory. Returns error code or 0 on success.
-int _mi_prim_commit(void* addr, size_t size, bool commit);
+// For example, on Linux this would make the memory PROT_READ|PROT_WRITE.
+int _mi_prim_commit(void* addr, size_t size);
+
+// Decommit memory. Returns error code or 0 on success. The `decommitted` result is true
+// if the memory would need to be re-committed. For example, on Windows this is always true,
+// but on Linux we could use MADV_DONTNEED to decommit which does not need a recommit.
+// pre: decommitted != NULL
+int _mi_prim_decommit(void* addr, size_t size, bool* decommitted);
 
 // Reset memory. The range keeps being accessible but the content might be reset.
 // Returns error code or 0 on success.

src/os.c

@@ -345,63 +345,64 @@ static void* mi_os_page_align_area_conservative(void* addr, size_t size, size_t*
   return mi_os_page_align_areax(true, addr, size, newsize);
 }
 
-// Commit/Decommit memory.
+bool _mi_os_commit(void* addr, size_t size, bool* is_zero, mi_stats_t* tld_stats) {
-// Usually commit is aligned liberal, while decommit is aligned conservative.
+  MI_UNUSED(tld_stats);
-// (but not for the reset version where we want commit to be conservative as well)
+  mi_stats_t* stats = &_mi_stats_main;  
-static bool mi_os_commitx(void* addr, size_t size, bool commit, bool conservative, bool* is_zero, mi_stats_t* stats) {
-  // page align in the range, commit liberally, decommit conservative
   if (is_zero != NULL) { *is_zero = false; }
-  size_t csize;
+  _mi_stat_increase(&stats->committed, size);  // use size for precise commit vs. decommit
-  void* start = mi_os_page_align_areax(conservative, addr, size, &csize);
+  _mi_stat_counter_increase(&stats->commit_calls, 1);
-  if (csize == 0) return true;  // || _mi_os_is_huge_reserved(addr))
-  if (commit) {
-    _mi_stat_increase(&stats->committed, size);  // use size for precise commit vs. decommit
-    _mi_stat_counter_increase(&stats->commit_calls, 1);
-  }
-  else {
-    _mi_stat_decrease(&stats->committed, size);
-  }
 
-  int err = _mi_prim_commit(start, csize, commit);  
+  // page align range
+  size_t csize;
+  void* start = mi_os_page_align_areax(false /* conservative? */, addr, size, &csize);
+  if (csize == 0) return true;
+
+  // commit  
+  int err = _mi_prim_commit(start, csize); 
   if (err != 0) {
-    _mi_warning_message("cannot %s OS memory (error: %d (0x%x), address: %p, size: 0x%zx bytes)\n", commit ? "commit" : "decommit", err, err, start, csize);
+    _mi_warning_message("cannot commit OS memory (error: %d (0x%x), address: %p, size: 0x%zx bytes)\n", err, err, start, csize);
   }
   mi_assert_internal(err == 0);
   return (err == 0);
 }
 
-bool _mi_os_commit(void* addr, size_t size, bool* is_zero, mi_stats_t* tld_stats) {
+static bool mi_os_decommit_ex(void* addr, size_t size, bool* decommitted, mi_stats_t* tld_stats) {
   MI_UNUSED(tld_stats);
   mi_stats_t* stats = &_mi_stats_main;
-  return mi_os_commitx(addr, size, true, false /* liberal */, is_zero, stats);
+  mi_assert_internal(decommitted!=NULL);
+  _mi_stat_decrease(&stats->committed, size);
+
+  // page align
+  size_t csize;
+  void* start = mi_os_page_align_area_conservative(addr, size, &csize);
+  if (csize == 0) return true; 
+
+  // decommit
+  *decommitted = true;
+  int err = _mi_prim_decommit(start,csize,decommitted);  
+  if (err != 0) {
+    _mi_warning_message("cannot decommit OS memory (error: %d (0x%x), address: %p, size: 0x%zx bytes)\n", err, err, start, csize);
+  }
+  mi_assert_internal(err == 0);
+  return (err == 0);
 }
 
 bool _mi_os_decommit(void* addr, size_t size, mi_stats_t* tld_stats) {
-  MI_UNUSED(tld_stats);
+  bool decommitted = true;
-  mi_stats_t* stats = &_mi_stats_main;
+  return mi_os_decommit_ex(addr, size, &decommitted, tld_stats);
-  bool is_zero;
-  return mi_os_commitx(addr, size, false, true /* conservative */, &is_zero, stats);
 }
 
-/*
-static bool mi_os_commit_unreset(void* addr, size_t size, bool* is_zero, mi_stats_t* stats) {  
-  return mi_os_commitx(addr, size, true, true // conservative
-                      , is_zero, stats);
-}
-*/
 
 // Signal to the OS that the address range is no longer in use
 // but may be used later again. This will release physical memory
 // pages and reduce swapping while keeping the memory committed.
 // We page align to a conservative area inside the range to reset.
-static bool mi_os_resetx(void* addr, size_t size, bool reset, mi_stats_t* stats) {
+bool _mi_os_reset(void* addr, size_t size, mi_stats_t* stats) { 
   // page align conservatively within the range
   size_t csize;
   void* start = mi_os_page_align_area_conservative(addr, size, &csize);
   if (csize == 0) return true;  // || _mi_os_is_huge_reserved(addr)
-  if (reset) _mi_stat_increase(&stats->reset, csize);
+  _mi_stat_increase(&stats->reset, csize);
-        else _mi_stat_decrease(&stats->reset, csize);
-  if (!reset) return true; // nothing to do on unreset!
 
   #if (MI_DEBUG>1) && !MI_SECURE && !MI_TRACK_ENABLED // && !MI_TSAN
   memset(start, 0, csize); // pretend it is eagerly reset
@@ -417,35 +418,19 @@ static bool mi_os_resetx(void* addr, size_t size, bool reset, mi_stats_t* stats)
   return (err == 0);
 }
 
-// Signal to the OS that the address range is no longer in use
-// but may be used later again. This will release physical memory
-// pages and reduce swapping while keeping the memory committed.
-// We page align to a conservative area inside the range to reset.
-bool _mi_os_reset(void* addr, size_t size, mi_stats_t* tld_stats) {
-  MI_UNUSED(tld_stats);
-  mi_stats_t* stats = &_mi_stats_main;
-  return mi_os_resetx(addr, size, true, stats);
-}
 
-/*
+// either resets or decommits memory, returns true if the memory was decommitted 
-bool _mi_os_unreset(void* addr, size_t size, bool* is_zero, mi_stats_t* tld_stats) {
+// (in the sense that it needs to be re-committed if the memory is re-used later on).
-  MI_UNUSED(tld_stats);
+bool _mi_os_purge(void* p, size_t size, mi_stats_t* stats)
-  mi_stats_t* stats = &_mi_stats_main;
-  *is_zero = false;
-  return mi_os_resetx(addr, size, false, stats);
-}
-*/
-
-// either resets or decommits memory, returns true if the memory was decommitted.
-bool _mi_os_purge(void* p, size_t size, mi_stats_t* stats) 
 {
   if (!mi_option_is_enabled(mi_option_allow_purge)) return false;
 
   if (mi_option_is_enabled(mi_option_purge_decommits) &&   // should decommit?
       !_mi_preloading())                                   // don't decommit during preloading (unsafe)
   {
-    _mi_os_decommit(p, size, stats);
+    bool decommitted;
-    return true;  // decommitted
+    mi_os_decommit_ex(p, size, &decommitted, stats);
+    return decommitted;   
   }
   else {
     _mi_os_reset(p, size, stats);

src/prim/unix/prim.c

@@ -340,32 +340,34 @@ static void unix_mprotect_hint(int err) {
   #endif
 }
 
-
+int _mi_prim_commit(void* start, size_t size) {
-int _mi_prim_commit(void* start, size_t size, bool commit) {
+  // commit: ensure we can access the area
-  int err = 0;
+  int err = mprotect(start, size, (PROT_READ | PROT_WRITE));
-  if (commit) {
+  if (err != 0) { err = errno; }  
-    // commit: ensure we can access the area
-    err = mprotect(start, size, (PROT_READ | PROT_WRITE));
-    if (err != 0) { err = errno; }
-  }
-  else {
-    #if defined(MADV_DONTNEED) && MI_DEBUG == 0 && MI_SECURE == 0
-    // decommit: use MADV_DONTNEED as it decreases rss immediately (unlike MADV_FREE)
-    // (on the other hand, MADV_FREE would be good enough.. it is just not reflected in the stats :-( )
-    err = unix_madvise(start, size, MADV_DONTNEED);
-    #else
-    // decommit: just disable access (also used in debug and secure mode to trap on illegal access)
-    err = mprotect(start, size, PROT_NONE);
-    if (err != 0) { err = errno; }
-    #endif    
-  }
   unix_mprotect_hint(err);
   return err;
 }
 
+int _mi_prim_decommit(void* start, size_t size, bool* decommitted) {
+  int err = 0;
+  #if defined(MADV_DONTNEED) && !MI_DEBUG && !MI_SECURE
+    // decommit: use MADV_DONTNEED as it decreases rss immediately (unlike MADV_FREE)
+    // (on the other hand, MADV_FREE would be good enough.. it is just not reflected in the stats :-( )
+    *decommitted = false;
+    err = unix_madvise(start, size, MADV_DONTNEED);
+  #else
+    // decommit: just disable access (also used in debug and secure mode to trap on illegal access)
+    *decommitted = true;  // needs recommit to reuse the memory
+    err = mprotect(start, size, PROT_NONE);
+    if (err != 0) { err = errno; }
+  #endif
+  return err;
+}
+
 int _mi_prim_reset(void* start, size_t size) {
-  // note: disable the use of MADV_FREE since it leads to confusing stats :-(
+  // We always use MADV_DONTNEED even if it may be a bit more expensive as this
-  #if 0 // defined(MADV_FREE)
+  // guarantees that we see the actual rss reflected in tools like `top`.
+  #if 0 && defined(MADV_FREE)
   static _Atomic(size_t) advice = MI_ATOMIC_VAR_INIT(MADV_FREE);
   int oadvice = (int)mi_atomic_load_relaxed(&advice);
   int err;

src/prim/wasi/prim.c

@@ -126,8 +126,14 @@ int _mi_prim_alloc(size_t size, size_t try_alignment, bool commit, bool allow_la
 // Commit/Reset/Protect
 //---------------------------------------------
 
-int _mi_prim_commit(void* addr, size_t size, bool commit) {
+int _mi_prim_commit(void* addr, size_t size) {
-  MI_UNUSED(addr); MI_UNUSED(size); MI_UNUSED(commit);
+  MI_UNUSED(addr); MI_UNUSED(size); 
+  return 0;
+}
+
+int _mi_prim_decommit(void* addr, size_t size, bool* decommitted) {
+  MI_UNUSED(addr); MI_UNUSED(size);
+  *decommitted = false;
   return 0;
 }
 

src/prim/windows/prim.c

@@ -257,15 +257,15 @@ int _mi_prim_alloc(size_t size, size_t try_alignment, bool commit, bool allow_la
 #pragma warning(disable:6250)   // suppress warning calling VirtualFree without MEM_RELEASE (for decommit)
 #endif
 
-int _mi_prim_commit(void* addr, size_t size, bool commit) {
+int _mi_prim_commit(void* addr, size_t size) {
-  if (commit) {
+  void* p = VirtualAlloc(addr, size, MEM_COMMIT, PAGE_READWRITE);
-    void* p = VirtualAlloc(addr, size, MEM_COMMIT, PAGE_READWRITE);
+  return (p == addr ? 0 : (int)GetLastError());  
-    return (p == addr ? 0 : (int)GetLastError());
+}
-  }
+
-  else {
+int _mi_prim_decommit(void* addr, size_t size, bool* decommitted) {  
-    BOOL ok = VirtualFree(addr, size, MEM_DECOMMIT);
+  BOOL ok = VirtualFree(addr, size, MEM_DECOMMIT);
-    return (ok ? 0 : (int)GetLastError());  
+  *decommitted = true;  // for safetly, assume always decommitted even in the case of an error.
-  }
+  return (ok ? 0 : (int)GetLastError());
 }
 
 int _mi_prim_reset(void* addr, size_t size) {

src/region.c

@@ -480,7 +480,8 @@ bool _mi_mem_unreset(void* p, size_t size, bool* is_zero, mi_os_tld_t* tld) {
     return _mi_os_commit(p, size, is_zero, tld->stats);
   }
   else {
-    return _mi_os_unreset(p, size, is_zero, tld->stats);
+    // return _mi_os_unreset(p, size, is_zero, tld->stats);
+    return true;
   }
 }