base: Use LazyInstanceTraits instead of SharedState class for discardable memory.

base/memory/discardable_memory_emulated.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/memory/discardable_memory_emulated.h"
 
 #include "base/lazy_instance.h"
 #include "base/memory/discardable_memory_manager.h"
 
 namespace base {
 namespace {
 
 // This is admittedly pretty magical.
 const size_t kEmulatedMemoryLimit = 512 * 1024 * 1024;
 const size_t kEmulatedSoftMemoryLimit = 32 * 1024 * 1024;
 const size_t kEmulatedHardMemoryLimitExpirationTimeMs = 1000;
 
-struct SharedState {
-  SharedState()
-      : manager(kEmulatedMemoryLimit,
-                kEmulatedSoftMemoryLimit,
-                TimeDelta::FromMilliseconds(
-                    kEmulatedHardMemoryLimitExpirationTimeMs)) {}
+// internal::DiscardableMemoryManager has an explicit constructor that takes
+// a number of memory limit parameters. The DefaultLazyInstanceTraits doesn't
+// handle the case. Thus, we need our own class here.
+struct DiscardableMemoryManagerLazyInstanceTraits {
+  static const bool kRegisterOnExit = false;

[danakj, 2014/10/15 18:04:08]

why false?

[reveman, 2014/10/15 18:38:56]

There's no guarantee that this is not be used after OnExit is called. For
example, discardable memory clients might store DM instances in LazyInstances.
Added comments to make this clear.

+#ifndef NDEBUG
+  static const bool kAllowedToAccessOnNonjoinableThread = true;

[danakj, 2014/10/15 18:04:08]

why true?

Sounds like you're making a LeakyLazyInstanceTraits not a
DefaultLazyInstanceTraits as the comment suggests? And not what this code used
to do?

[reveman, 2014/10/15 18:38:57]

This is what the code used to do. It used LazyInstance<SharedState>::Leaky.

I've updated the comments to instead mention LeakyLazyInstanceTraits instead of
DefaultLazyInstanceTraits.

+#endif
 
-  internal::DiscardableMemoryManager manager;
+  static internal::DiscardableMemoryManager* New(void* instance) {
+    return new (instance) internal::DiscardableMemoryManager(
+        kEmulatedMemoryLimit,

[danakj, 2014/10/15 18:04:08]

What about adding an Initialize(pass, constants, here) method instead that the
DiscardableMemoryEmulated can call after construction, instead of having to make
our own traits here?

[reveman, 2014/10/15 18:38:57]

That would not be thread safe unless I misunderstood your suggestion. There
would be a race when two threads would use this for the first time..

[danakj, 2014/10/15 18:48:47]

Oh, Register holds a mutex I guess? I see.

[reveman, 2014/10/15 18:54:36]

hm, I probably misunderstood your suggestion. When would this Initialize call
that pass parameters to the manager be made?

+        kEmulatedSoftMemoryLimit,
+        TimeDelta::FromMilliseconds(kEmulatedHardMemoryLimitExpirationTimeMs));
+  }
+  static void Delete(internal::DiscardableMemoryManager* instance) {
+    instance->~DiscardableMemoryManager();
+  }
 };
-LazyInstance<SharedState>::Leaky g_shared_state = LAZY_INSTANCE_INITIALIZER;
+
+LazyInstance<internal::DiscardableMemoryManager,
+             DiscardableMemoryManagerLazyInstanceTraits>
+    g_manager = LAZY_INSTANCE_INITIALIZER;
 
 }  // namespace
 
 namespace internal {
 
 DiscardableMemoryEmulated::DiscardableMemoryEmulated(size_t bytes)
     : bytes_(bytes),
       is_locked_(false) {
-  g_shared_state.Pointer()->manager.Register(this, bytes);
+  g_manager.Pointer()->Register(this, bytes);

[danakj, 2014/10/15 18:59:33]

before/after this register call, in this method.

[reveman, 2014/10/15 19:02:04]

Two threads can call this ctor at the same time. Both might end up calling
Initialize.

[danakj, 2014/10/15 19:11:53]

Right, sketchy. Carry on then :)

 }
 
 DiscardableMemoryEmulated::~DiscardableMemoryEmulated() {
   if (is_locked_)
     Unlock();
-  g_shared_state.Pointer()->manager.Unregister(this);
+  g_manager.Pointer()->Unregister(this);
 }
 
 // static
 bool DiscardableMemoryEmulated::ReduceMemoryUsage() {
-  return g_shared_state.Pointer()->manager.ReduceMemoryUsage();
+  return g_manager.Pointer()->ReduceMemoryUsage();
 }
 
 // static
 void DiscardableMemoryEmulated::ReduceMemoryUsageUntilWithinLimit(
     size_t bytes) {
-  g_shared_state.Pointer()->manager.ReduceMemoryUsageUntilWithinLimit(bytes);
+  g_manager.Pointer()->ReduceMemoryUsageUntilWithinLimit(bytes);
 }
 
 // static
 void DiscardableMemoryEmulated::PurgeForTesting() {
-  g_shared_state.Pointer()->manager.PurgeAll();
+  g_manager.Pointer()->PurgeAll();
 }
 
 bool DiscardableMemoryEmulated::Initialize() {
   return Lock() != DISCARDABLE_MEMORY_LOCK_STATUS_FAILED;
 }
 
 DiscardableMemoryLockStatus DiscardableMemoryEmulated::Lock() {
   DCHECK(!is_locked_);
 
   bool purged = false;
-  if (!g_shared_state.Pointer()->manager.AcquireLock(this, &purged))
+  if (!g_manager.Pointer()->AcquireLock(this, &purged))
     return DISCARDABLE_MEMORY_LOCK_STATUS_FAILED;
 
   is_locked_ = true;
   return purged ? DISCARDABLE_MEMORY_LOCK_STATUS_PURGED
                 : DISCARDABLE_MEMORY_LOCK_STATUS_SUCCESS;
 }
 
 void DiscardableMemoryEmulated::Unlock() {
   DCHECK(is_locked_);
-  g_shared_state.Pointer()->manager.ReleaseLock(this);
+  g_manager.Pointer()->ReleaseLock(this);
   is_locked_ = false;
 }
 
 void* DiscardableMemoryEmulated::Memory() const {
   DCHECK(is_locked_);
   DCHECK(memory_);
   return memory_.get();
 }
 
 bool DiscardableMemoryEmulated::AllocateAndAcquireLock() {
   if (memory_)
     return true;
 
   memory_.reset(new uint8[bytes_]);
   return false;
 }
 
 void DiscardableMemoryEmulated::Purge() {
   memory_.reset();
 }
 
 }  // namespace internal
 }  // namespace base