mi_collect(false) does not always complete scheduled purges

[4jsquared]

In some cases, purgeable slices from arenas are not purged when calling mi_collect(false) regularly.
Specifically, it seems like mi_arenas_try_purge resets subproc->purge_expire if it doesn't purge anything from any of the arenas - however this can happen because the arenas have their own purge_expire which hasn't been reached yet.

Observered in 3.1.6 and 3.2.8

[mianyuntong]

I just found this problem on 3.1.5 yesterday, and I modified 'mi_arena_schedule_purge' to test, it seems right to me now.

static void mi_arena_schedule_purge(mi_arena_t* arena, size_t slice_index, size_t slice_count) {
  const long delay = mi_arena_purge_delay();
  if (arena->memid.is_pinned || delay < 0 || _mi_preloading()) return;  // is purging allowed at all?

  mi_assert_internal(mi_bbitmap_is_clearN(arena->slices_free, slice_index, slice_count));
  if (delay == 0) {
    // purge directly
    mi_arena_purge(arena, slice_index, slice_count);
  }
  else {
    // schedule purge
    const mi_msecs_t expire = _mi_clock_now() + delay;
    mi_msecs_t expire0 = 0;
    if (mi_atomic_casi64_strong_acq_rel(&arena->purge_expire, &expire0, expire)) {
      // expiration was not yet set
      // maybe set the global arenas expire as well (if it wasn't set already)
      mi_assert_internal(expire0==0);
      mi_atomic_casi64_strong_acq_rel(&arena->subproc->purge_expire, &expire0, expire);
    }
    else {
      // already an expiration was set
      // MY MODIFICATION:
      // Arena already had an expiration set (expire0 holds its current value).
      // Still update subproc->purge_expire so try_purge can run: when subproc is 0
      // we must set it; when subproc is set, set to the earlier of the two.
      if (expire0 != 0) {
        mi_msecs_t subproc_exp = mi_atomic_loadi64_acquire(&arena->subproc->purge_expire);
        mi_msecs_t new_exp = (subproc_exp == 0) ? expire0
            : ((expire0 < subproc_exp) ? expire0 : subproc_exp);
        if (new_exp != subproc_exp) {
            mi_msecs_t expected = subproc_exp;
            mi_atomic_casi64_strong_acq_rel(&arena->subproc->purge_expire, &expected, new_exp);
        }
      }
    }
    mi_bitmap_setN(arena->slices_purge, slice_index, slice_count, NULL);
  }
}

[daanx]

Thanks! I added a potential fix -- a bit different what you did as I now check the return value of mi_arena_purge to distinguish if a purge didn't happen because the arena->purge_expire time has not been reached yet. Let me know if this fixes the issue for you.

[4jsquared]

@daanx - tested those fixes (slightly adjusted for the version I have here), and that seems to fix the issue - thanks!