[discardable] Periodic purging of unlocked discardable memory

This CL lets us schedule a purge of unlocked discardable memory from the
foreground tab to run every minute.

This is hidden behind a feature which is disabled by default, so there
should be no changes to functionality.

Bug: 1109209
Change-Id: Icb2474b1026e2283ccc772cd3a602d46ef628dc8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2370655Reviewed-by: Peng Huang <penghuang@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Benoit L <lizeb@chromium.org>
Commit-Queue: Thiabaud Engelbrecht <thiabaud@google.com>
Cr-Commit-Position: refs/heads/master@{#803848}

components/discardable_memory/client/client_discardable_shared_memory_manager.cc

@@ -9,7 +9,6 @@
 
 #include "base/atomic_sequence_num.h"
 #include "base/bind.h"
-#include "base/feature_list.h"
 #include "base/macros.h"
 #include "base/memory/discardable_memory.h"
 #include "base/memory/discardable_shared_memory.h"
@@ -27,10 +26,6 @@
 #include "components/crash/core/common/crash_key.h"
 
 namespace discardable_memory {
-namespace {
-
-// Global atomic to generate unique discardable shared memory IDs.
-base::AtomicSequenceNumber g_next_discardable_shared_memory_id;
 
 // This controls whether unlocked memory is released when |ReleaseFreeMemory| is
 // called. Enabling this causes |ReleaseFreeMemory| to release all
@@ -39,6 +34,19 @@ base::AtomicSequenceNumber g_next_discardable_shared_memory_id;
 const base::Feature kPurgeUnlockedMemory{"PurgeUnlockedMemory",
                                          base::FEATURE_DISABLED_BY_DEFAULT};
 
+// This controls whether unlocked memory is periodically purged from the
+// foreground process. Enabling this causes a task to be scheduled at regular
+// intervals to purge unlocked memory that hasn't been touched in a while. This
+// task is stopped if no discardable memory is left, and restarted at the next
+// allocation.
+const base::Feature kSchedulePeriodicPurge{"SchedulePeriodicPurge",
+                                           base::FEATURE_DISABLED_BY_DEFAULT};
+
+namespace {
+
+// Global atomic to generate unique discardable shared memory IDs.
+base::AtomicSequenceNumber g_next_discardable_shared_memory_id;
+
 size_t GetDefaultAllocationSize() {
   const size_t kOneMegabyteInBytes = 1024 * 1024;
 
@@ -81,11 +89,14 @@ void DeletedDiscardableSharedMemoryOnIO(
 
 }  // namespace
 
+constexpr base::TimeDelta
+    ClientDiscardableSharedMemoryManager::kMinAgeForScheduledPurge;
+
 ClientDiscardableSharedMemoryManager::DiscardableMemoryImpl::
     DiscardableMemoryImpl(
         ClientDiscardableSharedMemoryManager* manager,
         std::unique_ptr<DiscardableSharedMemoryHeap::Span> span)
-    : manager_(manager), span_(std::move(span)), is_locked_(true) {
+    : manager_(manager), span_(std::move(span)) {
   DCHECK_NE(manager, nullptr);
 }
 
@@ -93,10 +104,10 @@ ClientDiscardableSharedMemoryManager::DiscardableMemoryImpl::
     ~DiscardableMemoryImpl() {
   base::AutoLock lock(manager_->GetLock());
   if (!span_) {
-    DCHECK(!is_locked_);
+    DCHECK(!is_locked());
     return;
   }
-  if (is_locked_)
+  if (is_locked())
     manager_->UnlockSpan(span_.get());
 
   manager_->ReleaseMemory(this, std::move(span_));
@@ -104,30 +115,37 @@ ClientDiscardableSharedMemoryManager::DiscardableMemoryImpl::
 
 bool ClientDiscardableSharedMemoryManager::DiscardableMemoryImpl::Lock() {
   base::AutoLock lock(manager_->GetLock());
-  DCHECK(!is_locked_);
+  DCHECK(!is_locked());
 
   if (span_ && manager_->LockSpan(span_.get()))
-    is_locked_ = true;
+    last_locked_ = base::TimeTicks();
 
-  UMA_HISTOGRAM_BOOLEAN("Memory.Discardable.LockingSuccess", is_locked_);
+  bool locked = is_locked();
+  UMA_HISTOGRAM_BOOLEAN("Memory.Discardable.LockingSuccess", locked);
 
-  return is_locked_;
+  return locked;
 }
 
 void ClientDiscardableSharedMemoryManager::DiscardableMemoryImpl::Unlock() {
   base::AutoLock lock(manager_->GetLock());
-  DCHECK(is_locked_);
+  DCHECK(is_locked());
   DCHECK(span_);
 
   manager_->UnlockSpan(span_.get());
-  is_locked_ = false;
+  last_locked_ = base::TimeTicks::Now();
 }
 
 std::unique_ptr<DiscardableSharedMemoryHeap::Span>
-ClientDiscardableSharedMemoryManager::DiscardableMemoryImpl::Purge() {
+ClientDiscardableSharedMemoryManager::DiscardableMemoryImpl::Purge(
+    base::TimeTicks min_ticks) {
   DCHECK(span_);
-  if (is_locked_)
+
+  if (is_locked())
+    return nullptr;
+
+  if (last_locked_ > min_ticks)
     return nullptr;
+
   return std::move(span_);
 }
 
@@ -136,18 +154,23 @@ void* ClientDiscardableSharedMemoryManager::DiscardableMemoryImpl::data()
 #if DCHECK_IS_ON()
   {
     base::AutoLock lock(manager_->GetLock());
-    DCHECK(is_locked_);
+    DCHECK(is_locked());
   }
 #endif
   return reinterpret_cast<void*>(span_->start() * base::GetPageSize());
 }
 
+bool ClientDiscardableSharedMemoryManager::DiscardableMemoryImpl::is_locked()
+    const {
+  return last_locked_.is_null();
+}
+
 void ClientDiscardableSharedMemoryManager::DiscardableMemoryImpl::
     DiscardForTesting() {
 #if DCHECK_IS_ON()
   {
     base::AutoLock lock(manager_->GetLock());
-    DCHECK(!is_locked_);
+    DCHECK(!is_locked());
   }
 #endif
   span_->shared_memory()->Purge(base::Time::Now());
@@ -162,8 +185,10 @@ base::trace_event::MemoryAllocatorDump* ClientDiscardableSharedMemoryManager::
 
 ClientDiscardableSharedMemoryManager::ClientDiscardableSharedMemoryManager(
     mojo::PendingRemote<mojom::DiscardableSharedMemoryManager> manager,
-    scoped_refptr<base::SingleThreadTaskRunner> io_task_runner)
+    scoped_refptr<base::SingleThreadTaskRunner> io_task_runner,
+    scoped_refptr<base::SingleThreadTaskRunner> periodic_purge_task_runner)
     : heap_(std::make_unique<DiscardableSharedMemoryHeap>()),
+      periodic_purge_task_runner_(std::move(periodic_purge_task_runner)),
       io_task_runner_(std::move(io_task_runner)),
       manager_mojo_(std::make_unique<
                     mojo::Remote<mojom::DiscardableSharedMemoryManager>>()) {
@@ -176,8 +201,10 @@ ClientDiscardableSharedMemoryManager::ClientDiscardableSharedMemoryManager(
 }
 
 ClientDiscardableSharedMemoryManager::ClientDiscardableSharedMemoryManager(
-    scoped_refptr<base::SingleThreadTaskRunner> io_task_runner)
+    scoped_refptr<base::SingleThreadTaskRunner> io_task_runner,
+    scoped_refptr<base::SingleThreadTaskRunner> periodic_purge_task_runner)
     : heap_(std::make_unique<DiscardableSharedMemoryHeap>()),
+      periodic_purge_task_runner_(std::move(periodic_purge_task_runner)),
       io_task_runner_(std::move(io_task_runner)) {}
 
 ClientDiscardableSharedMemoryManager::~ClientDiscardableSharedMemoryManager() {
@@ -205,6 +232,9 @@ ClientDiscardableSharedMemoryManager::AllocateLockedDiscardableMemory(
     size_t size) {
   base::AutoLock lock(lock_);
 
+  if (timer_ == nullptr && periodic_purge_task_runner_)
+    StartScheduledPurging(periodic_purge_task_runner_);
+
   DCHECK_NE(size, 0u);
 
   auto size_in_kb = static_cast<base::HistogramBase::Sample>(size / 1024);
@@ -324,6 +354,26 @@ ClientDiscardableSharedMemoryManager::AllocateLockedDiscardableMemory(
   return std::move(discardable_memory);
 }
 
+void ClientDiscardableSharedMemoryManager::StartScheduledPurging(
+    scoped_refptr<base::SequencedTaskRunner> task_runner) {
+  DCHECK(task_runner);
+  if (base::FeatureList::IsEnabled(kSchedulePeriodicPurge)) {
+    // The expected cost of purging should be very small (< 1ms), so it can be
+    // scheduled frequently. However, we don't purge memory that has been
+    // touched recently (see: |BackgroundPurge| and |kMinAgeForScheduledPurge|),
+    // so there is no benefit to running this more than once per minute.
+    constexpr base::TimeDelta kInterval = base::TimeDelta::FromMinutes(1);
+
+    timer_ = std::make_unique<base::RepeatingTimer>();
+    timer_->SetTaskRunner(task_runner);
+
+    timer_->Start(FROM_HERE, kInterval,
+                  base::BindRepeating(
+                      &ClientDiscardableSharedMemoryManager::ScheduledPurge,
+                      base::Unretained(this)));
+  }
+}
+
 bool ClientDiscardableSharedMemoryManager::OnMemoryDump(
     const base::trace_event::MemoryDumpArgs& args,
     base::trace_event::ProcessMemoryDump* pmd) {
@@ -336,10 +386,27 @@ size_t ClientDiscardableSharedMemoryManager::GetBytesAllocated() const {
   return heap_->GetSize() - heap_->GetSizeOfFreeLists();
 }
 
-void ClientDiscardableSharedMemoryManager::PurgeUnlockedMemory() {
+void ClientDiscardableSharedMemoryManager::BackgroundPurge() {
+  PurgeUnlockedMemory(base::TimeDelta());
+}
+
+void ClientDiscardableSharedMemoryManager::ScheduledPurge() {
+  // From local testing and UMA, memory usually accumulates slowly in renderers,
+  // and can sit idle for hours. We purge only the old memory, as this should
+  // recover the memory without adverse latency effects.
+  // TODO(crbug.com/1123679): Determine if |kMinAgeForScheduledPurge| and the
+  // constant from |ScheduledPurge| need to be tuned.
+  PurgeUnlockedMemory(
+      ClientDiscardableSharedMemoryManager::kMinAgeForScheduledPurge);
+}
+
+void ClientDiscardableSharedMemoryManager::PurgeUnlockedMemory(
+    base::TimeDelta min_age) {
   {
     base::AutoLock lock(lock_);
 
+    auto now = base::TimeTicks::Now();
+
     // Iterate this way in order to avoid invalidating the iterator while
     // removing elements from |allocated_memory_| as we iterate over it.
     for (auto it = allocated_memory_.begin(); it != allocated_memory_.end();
@@ -353,7 +420,7 @@ void ClientDiscardableSharedMemoryManager::PurgeUnlockedMemory() {
       DCHECK_EQ(&lock_, &mem->manager_->GetLock());
       mem->manager_->GetLock().AssertAcquired();
 
-      auto span = mem->Purge();
+      auto span = mem->Purge(now - min_age);
       if (span) {
         allocated_memory_.erase(prev);
         ReleaseSpan(std::move(span));
@@ -366,10 +433,13 @@ void ClientDiscardableSharedMemoryManager::PurgeUnlockedMemory() {
 
 void ClientDiscardableSharedMemoryManager::ReleaseFreeMemory() {
   if (base::FeatureList::IsEnabled(kPurgeUnlockedMemory)) {
-    PurgeUnlockedMemory();
+    BackgroundPurge();
   } else {
     ReleaseFreeMemoryImpl();
   }
+
+  if (GetBytesAllocated() == 0)
+    timer_ = nullptr;
 }
 
 void ClientDiscardableSharedMemoryManager::ReleaseFreeMemoryImpl() {

components/discardable_memory/client/client_discardable_shared_memory_manager.h

@@ -10,6 +10,7 @@
 #include <memory>
 #include <set>
 
+#include "base/feature_list.h"
 #include "base/memory/discardable_memory_allocator.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/unsafe_shared_memory_region.h"
@@ -27,6 +28,8 @@ class SingleThreadTaskRunner;
 
 namespace discardable_memory {
 
+DISCARDABLE_MEMORY_EXPORT extern const base::Feature kSchedulePeriodicPurge;
+
 // Implementation of DiscardableMemoryAllocator that allocates
 // discardable memory segments through the browser process.
 class DISCARDABLE_MEMORY_EXPORT ClientDiscardableSharedMemoryManager
@@ -35,7 +38,9 @@ class DISCARDABLE_MEMORY_EXPORT ClientDiscardableSharedMemoryManager
  public:
   ClientDiscardableSharedMemoryManager(
       mojo::PendingRemote<mojom::DiscardableSharedMemoryManager> manager,
-      scoped_refptr<base::SingleThreadTaskRunner> io_task_runner);
+      scoped_refptr<base::SingleThreadTaskRunner> io_task_runner,
+      scoped_refptr<base::SingleThreadTaskRunner> periodic_purge_task_runner =
+          nullptr);
   ~ClientDiscardableSharedMemoryManager() override;
 
   // Overridden from base::DiscardableMemoryAllocator:
@@ -46,8 +51,8 @@ class DISCARDABLE_MEMORY_EXPORT ClientDiscardableSharedMemoryManager
   bool OnMemoryDump(const base::trace_event::MemoryDumpArgs& args,
                     base::trace_event::ProcessMemoryDump* pmd) override;
 
-  // Purge any unlocked memory that was allocated by this manager.
-  void PurgeUnlockedMemory();
+  // Purge all unlocked memory that was allocated by this manager.
+  void BackgroundPurge();
 
   // Release memory and associated resources that have been purged.
   void ReleaseFreeMemory() override;
@@ -62,6 +67,9 @@ class DISCARDABLE_MEMORY_EXPORT ClientDiscardableSharedMemoryManager
       const char* name,
       base::trace_event::ProcessMemoryDump* pmd) const;
 
+  void StartScheduledPurging(
+      scoped_refptr<base::SequencedTaskRunner> task_runner);
+
   struct Statistics {
     size_t total_size;
     size_t freelist_size;
@@ -73,13 +81,19 @@ class DISCARDABLE_MEMORY_EXPORT ClientDiscardableSharedMemoryManager
     bytes_allocated_limit_for_testing_ = limit;
   }
 
-  // We only have protected members for testing, everything else should be
-  // either public or private.
+  static constexpr base::TimeDelta kMinAgeForScheduledPurge =
+      base::TimeDelta::FromMinutes(5);
+
+  // These fields are only protected for testing, they would otherwise be
+  // private. Everything else should be either public or private.
  protected:
-  explicit ClientDiscardableSharedMemoryManager(
-      scoped_refptr<base::SingleThreadTaskRunner> io_task_runner);
+  ClientDiscardableSharedMemoryManager(
+      scoped_refptr<base::SingleThreadTaskRunner> io_task_runner,
+      scoped_refptr<base::SingleThreadTaskRunner> periodic_purge_task_runner);
   std::unique_ptr<DiscardableSharedMemoryHeap> heap_ GUARDED_BY(lock_);
   mutable base::Lock lock_;
+  std::unique_ptr<base::RepeatingTimer> timer_;
+  scoped_refptr<base::SingleThreadTaskRunner> periodic_purge_task_runner_;
 
  private:
   class DiscardableMemoryImpl : public base::DiscardableMemory {
@@ -101,17 +115,27 @@ class DISCARDABLE_MEMORY_EXPORT ClientDiscardableSharedMemoryManager
         const char* name,
         base::trace_event::ProcessMemoryDump* pmd) const override;
 
-    // Returns |span_| if unlocked, otherwise nullptr.
-    std::unique_ptr<DiscardableSharedMemoryHeap::Span> Purge()
+    // Returns |span_| if it has been unlocked since at least |min_ticks|,
+    // otherwise nullptr.
+    std::unique_ptr<DiscardableSharedMemoryHeap::Span> Purge(
+        base::TimeTicks min_ticks)
         EXCLUSIVE_LOCKS_REQUIRED(manager_->GetLock());
 
    private:
+    bool is_locked() const EXCLUSIVE_LOCKS_REQUIRED(manager_->GetLock());
+
     friend class ClientDiscardableSharedMemoryManager;
     ClientDiscardableSharedMemoryManager* const manager_;
     std::unique_ptr<DiscardableSharedMemoryHeap::Span> span_;
-    bool is_locked_ GUARDED_BY(manager_->GetLock());
+    // Set to an invalid base::TimeTicks when |this| is Lock()-ed, and to
+    // |TimeTicks::Now()| each time |this| is Unlock()-ed.
+    base::TimeTicks last_locked_ GUARDED_BY(manager_->GetLock());
   };
 
+  // Purge any unlocked memory from foreground that hasn't been touched in a
+  // while.
+  void ScheduledPurge();
+
   // This is only virtual for testing.
   virtual std::unique_ptr<base::DiscardableSharedMemory>
   AllocateLockedDiscardableSharedMemory(size_t size, int32_t id);
@@ -128,6 +152,8 @@ class DISCARDABLE_MEMORY_EXPORT ClientDiscardableSharedMemoryManager
   void MemoryUsageChanged(size_t new_bytes_allocated,
                           size_t new_bytes_free) const;
 
+  // Releases all unlocked memory that was last locked at least |min_age| ago.
+  void PurgeUnlockedMemory(base::TimeDelta min_age);
   void ReleaseFreeMemoryImpl();
   void ReleaseMemory(DiscardableMemoryImpl* memory,
                      std::unique_ptr<DiscardableSharedMemoryHeap::Span> span)

content/renderer/discardable_memory_utils.cc

@@ -42,7 +42,8 @@ CreateDiscardableMemoryAllocator() {
       manager_remote.InitWithNewPipeAndPassReceiver());
   return std::make_unique<
       discardable_memory::ClientDiscardableSharedMemoryManager>(
-      std::move(manager_remote), ChildProcess::current()->io_task_runner());
+      std::move(manager_remote), ChildProcess::current()->io_task_runner(),
+      ChildProcess::current()->main_thread()->main_thread_runner());
 }
 
 }  // namespace content