[WTF][JSC] An idle thread releases its free memory again while the allocator holds some of it back

Source/JavaScriptCore/runtime/WaiterListManager.cpp

@@ -133,23 +133,27 @@ WaiterListManager::WaitSyncResult WaiterListManager::waitSyncImpl(VM& vm, ValueT
         syncWaiter->setParkedList(list.copyRef());
 
 #if USE(MIMALLOC)
-        // Only the owning thread can return the memory its thread-local heap holds, so do that once in a wait that
-        // takes a while. A wait that is notified soon pays nothing.
-        MonotonicTime releaseFreeMemoryTime = MonotonicTime::now() + 100_ms;
-        bool didReleaseFreeMemory = false;
+        // Only the owning thread can return the memory its thread-local heap holds, so do that in a wait that takes a
+        // while. A wait that is notified soon pays nothing. One release does not return the large blocks that the
+        // thread used a moment ago, so the thread releases again an interval later while one is due.
+        static constexpr Seconds releaseFreeMemoryInterval = Seconds::fromMilliseconds(WTF::IdleFreeMemoryRelease::intervalInMilliseconds);
+        WTF::IdleFreeMemoryRelease freeMemoryRelease;
+        MonotonicTime releaseFreeMemoryTime = MonotonicTime::now() + releaseFreeMemoryInterval;
 #endif
         while (syncWaiter->isOnList() && time.now() < time && !shouldStopWaitingForTermination(vm)) {
 #if USE(MIMALLOC)
-            if (!didReleaseFreeMemory && releaseFreeMemoryTime < time) {
+            if (freeMemoryRelease.isDue() && releaseFreeMemoryTime < time) {
                 if (MonotonicTime::now() < releaseFreeMemoryTime) {
                     syncWaiter->condition().waitUntil(list->lock, releaseFreeMemoryTime.approximate<WallTime>());
                     continue;
                 }
-                didReleaseFreeMemory = true;
                 // A notification in the meantime takes us off the list, and the loop tests for that (and for a
                 // termination request) under the lock again before it waits.
-                DropLockForScope dropLock { listLocker };
-                WTF::releaseFastMallocFreeMemoryForIdleThread();
+                {
+                    DropLockForScope dropLock { listLocker };
+                    freeMemoryRelease.release();
+                }
+                releaseFreeMemoryTime = MonotonicTime::now() + releaseFreeMemoryInterval;
                 continue;
             }
 #endif

Source/JavaScriptCore/shell/ExternalMimallocShims.cpp

@@ -40,6 +40,13 @@ extern "C" void mi_on_thread_idle(void) noexcept
     mi_theap_collect(mi_theap_get_default(), /* force */ false);
 }
 
+// Whether the call above held back memory in pages of this thread, for another call. The collect
+// of the vendored mimalloc holds nothing back.
+extern "C" bool mi_on_thread_idle_pending_own(void) noexcept
+{
+    return false;
+}
+
 // A value above 1 skips the teardown at process exit, like the consumer's MI_NO_PROCESS_DETACH.
 __attribute__((constructor)) static void skipProcessTeardown()
 {

Source/WTF/wtf/AutomaticThread.cpp

@@ -39,8 +39,9 @@ WTF_MAKE_TZONE_ALLOCATED_IMPL(AutomaticThread);
 static constexpr bool verbose = false;
 
 #if USE(MIMALLOC)
-// How long a thread waits for more work before it releases its allocator's free memory.
-static constexpr Seconds idleReleaseDelay = 100_ms;
+// How long a thread waits for more work before it releases its allocator's free memory, and
+// before it releases again.
+static constexpr Seconds idleReleaseDelay = Seconds::fromMilliseconds(IdleFreeMemoryRelease::intervalInMilliseconds);
 #endif
 
 Ref<AutomaticThreadCondition> AutomaticThreadCondition::create()
@@ -236,8 +237,8 @@ void AutomaticThread::start(const AbstractLocker&)
             };
 
 #if USE(MIMALLOC)
-            // Whether this thread released its allocator's free memory since it last worked.
-            bool didReleaseFreeMemory = false;
+            // The releases of this thread's free memory since it last worked.
+            IdleFreeMemoryRelease idleRelease;
 #endif
 
             for (;;) {
@@ -265,22 +266,27 @@ void AutomaticThread::start(const AbstractLocker&)
                         // waiting state through the release, and polls again only if a notify
                         // arrived meanwhile (notify() clears m_isWaiting, which is checked under the
                         // lock, so a notify during the release is not lost).
+                        //
+                        // One release does not return the large blocks that the thread used a moment
+                        // ago, so the thread waits as long again and releases again while one is due
+                        // (IdleFreeMemoryRelease). Every such wait counts towards its timeout.
                         Seconds timeout = m_timeout;
-                        if (!didReleaseFreeMemory && m_timeout > idleReleaseDelay) {
+                        while (idleRelease.isDue() && timeout > idleReleaseDelay) {
                             m_waitCondition.waitFor(*m_lock, idleReleaseDelay);
                             if (!m_isWaiting)
-                                continue;
-                            didReleaseFreeMemory = true;
+                                break;
+                            timeout -= idleReleaseDelay;
                             m_isReleasingFreeMemory = true;
                             {
                                 DropLockForScope dropLock { locker };
-                                releaseFastMallocFreeMemoryForIdleThread();
+                                idleRelease.release();
                             }
                             m_isReleasingFreeMemory = false;
                             if (!m_isWaiting)
-                                continue;
-                            timeout -= idleReleaseDelay;
+                                break;
                         }
+                        if (!m_isWaiting)
+                            continue;
 #else
                         Seconds timeout = m_timeout;
 #endif
@@ -301,7 +307,7 @@ void AutomaticThread::start(const AbstractLocker&)
                 }
 
 #if USE(MIMALLOC)
-                didReleaseFreeMemory = false;
+                idleRelease.reset();
 #endif
                 WorkResult result = work();
                 if (result == WorkResult::Stop) {

Source/WTF/wtf/FastMalloc.cpp

@@ -605,9 +605,10 @@ void releaseFastMallocFreeMemoryForThisThread()
     bmalloc::api::scavengeThisThread();
 }
 
-void releaseFastMallocFreeMemoryForIdleThread()
+void IdleFreeMemoryRelease::release()
 {
-    bmalloc::api::scavengeThisThreadOnIdle();
+    ASSERT(isDue());
+    m_releasesLeft = bmalloc::api::scavengeThisThreadOnIdle() ? m_releasesLeft - 1 : 0;
 }
 
 void releaseFastMallocFreeMemory()

Source/WTF/wtf/FastMalloc.h

@@ -177,8 +177,28 @@ WTF_EXPORT_PRIVATE size_t fastMallocGoodSize(size_t);
 
 WTF_EXPORT_PRIVATE void releaseFastMallocFreeMemory();
 WTF_EXPORT_PRIVATE void releaseFastMallocFreeMemoryForThisThread();
-// For a thread that is idle and about to block for a while. See bmalloc::api::scavengeThisThreadOnIdle.
-WTF_EXPORT_PRIVATE void releaseFastMallocFreeMemoryForIdleThread();
+
+// What a thread does with its allocator's free memory while it has nothing to do. Only the owning
+// thread can return the memory that its thread-local heap holds, so a thread that is about to
+// block for a while releases it: once it has been idle for the interval, and again an interval
+// after each release that held memory back, maxReleases times in all. One release does not return
+// the large blocks that the thread used a moment ago (bmalloc::api::scavengeThisThreadOnIdle).
+// A thread that blocks after one release keeps them for as long as it blocks.
+class IdleFreeMemoryRelease {
+public:
+    // In milliseconds, because Seconds.h includes this header.
+    static constexpr unsigned intervalInMilliseconds = 100;
+
+    // Whether the thread is to call release() when it has been idle for one more interval.
+    bool isDue() const { return m_releasesLeft; }
+    WTF_EXPORT_PRIVATE void release();
+    // The thread has worked, so it has memory to release again.
+    void reset() { m_releasesLeft = maxReleases; }
+
+private:
+    static constexpr unsigned maxReleases = 4;
+    unsigned m_releasesLeft { maxReleases };
+};
 
 WTF_EXPORT_PRIVATE void fastCommitAlignedMemory(void*, size_t);
 WTF_EXPORT_PRIVATE void fastDecommitAlignedMemory(void*, size_t);

Source/bmalloc/bmalloc/bmalloc.cpp

@@ -171,15 +171,22 @@ void scavengeThisThread(bool force)
 // it still uses, and hands the arena purge to mimalloc's scavenger thread. The vendored mimalloc
 // does not have it, so it is declared here and not in mimalloc.h.
 extern "C" void mi_on_thread_idle(void) noexcept;
+// Whether that call left the free blocks of a large page of this thread, one that the thread
+// allocated from a moment ago. Nothing comes back for those but another call.
+// mi_on_thread_idle_pending() is not the question here: it is also true for the pages that other
+// threads use, and a thread that comes back for those takes their buffers from them.
+extern "C" bool mi_on_thread_idle_pending_own(void) noexcept;
 #endif
 
-void scavengeThisThreadOnIdle()
+bool scavengeThisThreadOnIdle()
 {
 #if BUSE(EXTERNAL_MIMALLOC)
     mi_on_thread_idle();
+    return mi_on_thread_idle_pending_own();
 #else
     // Not forced: that would also purge what every other thread just freed.
     scavengeThisThread(/* force */ false);
+    return false;
 #endif
 }
 

Source/bmalloc/bmalloc/bmalloc.h

@@ -418,7 +418,12 @@ BEXPORT void scavengeThisThread(bool force = true);
 // The calling thread is idle and about to block for a while. Return what its thread-local
 // allocator state holds for it: retired pages, blocks that other threads freed into its pages,
 // and the free runs inside the pages it still uses. Only the owning thread can do this.
-BEXPORT void scavengeThisThreadOnIdle();
+//
+// One call does not return everything. The allocator leaves the large blocks that the thread used
+// a moment ago, because a thread that is only between two tasks takes them again at once. Returns
+// true when it left some in pages of this thread: a thread that is still idle 100 ms later calls
+// this again, three more times at the most. WTF::IdleFreeMemoryRelease does that.
+BEXPORT bool scavengeThisThreadOnIdle();
 
 BEXPORT void scavenge();