Support delayed allocations from persistent memory.

base/metrics/persistent_memory_allocator.cc

Index: base/metrics/persistent_memory_allocator.cc
diff --git a/base/metrics/persistent_memory_allocator.cc b/base/metrics/persistent_memory_allocator.cc
index d381d8784d1094e1d883db24a6ce03eee6300e27..4f372b8ca113a5642bafed23959ac6ab594f420a 100644
--- a/base/metrics/persistent_memory_allocator.cc
+++ b/base/metrics/persistent_memory_allocator.cc
@@ -1063,4 +1063,95 @@ void FilePersistentMemoryAllocator::FlushPartial(size_t length, bool sync) {
 }
 #endif  // !defined(OS_NACL)
 
+//----- DelayedPersistentAllocation --------------------------------------------
+
+// Forwarding constructors.
+DelayedPersistentAllocation::DelayedPersistentAllocation(
+    PersistentMemoryAllocator* allocator,
+    subtle::Atomic32* ref,
+    uint32_t type,
+    size_t size)
+    : DelayedPersistentAllocation(
+          allocator,
+          reinterpret_cast<std::atomic<Reference>*>(ref),
+          type,
+          size,
+          0) {}

[Alexei Svitkine (slow), 2017/04/12 17:04:24]

Nit: Add empty lines between ctors to make it easier for a human to parse.

[bcwhite, 2017/04/12 21:23:42]

Done.

+DelayedPersistentAllocation::DelayedPersistentAllocation(
+    PersistentMemoryAllocator* allocator,
+    subtle::Atomic32* ref,
+    uint32_t type,
+    size_t size,
+    size_t offset)
+    : DelayedPersistentAllocation(
+          allocator,
+          reinterpret_cast<std::atomic<Reference>*>(ref),
+          type,
+          size,
+          offset) {}
+DelayedPersistentAllocation::DelayedPersistentAllocation(
+    PersistentMemoryAllocator* allocator,
+    std::atomic<Reference>* ref,
+    uint32_t type,
+    size_t size)
+    : DelayedPersistentAllocation(allocator, ref, type, size, 0) {}
+
+// Real constructor.
+DelayedPersistentAllocation::DelayedPersistentAllocation(
+    PersistentMemoryAllocator* allocator,
+    std::atomic<Reference>* ref,
+    uint32_t type,
+    size_t size,
+    size_t offset)
+    : allocator_(allocator),
+      type_(type),
+      size_(size),
+      offset_(offset),
+      reference_(ref) {}

[Alexei Svitkine (slow), 2017/04/12 17:04:25]

Nit: Add a DCHECK() on the ref ptr not being null?

[bcwhite, 2017/04/12 21:23:42]

Done.

+
+DelayedPersistentAllocation::~DelayedPersistentAllocation() {}
+
+void DelayedPersistentAllocation::MakeIterable() {
+  make_iterable_ = true;

[Alexei Svitkine (slow), 2017/04/12 17:04:24]

How about the case where it was created already and MakeIterable() is called
after that?

Either we should not support that explicitly (and add a DCHECK here that Get()
hasn't been called yet) and document it - or this should do more work if there's
already a ref set. I think the former (not supporting it explicitly) is fine
given it doesn't seem we need it.

[bcwhite, 2017/04/12 21:23:42]

It would be called when the object is created and not by whatever is doing Get()
so it will happen first.  It could be a ctor parameter.

[Alexei Svitkine (slow), 2017/04/21 15:14:16]

Let's make it a ctor param them - to avoid confusion about it being possible to
call it later.

However, looking again - it doesn't seem like you use it from the other CL. If
so, how about just removing it to simplify the code? Or is it needed for the
tests you have? If that's the case, then call it MakeIterableForTesting().

(If later there's a use case for it to be called outside of tests, then it can
be renamed.)

[bcwhite, 2017/04/21 15:59:04]

I like it better as a ctor parameter.  Less confusion as the method sounds like
it should operate immediately when it does not.

+}
+
+void* DelayedPersistentAllocation::Get() const {
+  // Relaxed operations is acceptable here because it's not protecting the

[Alexei Svitkine (slow), 2017/04/12 17:04:25]

Nit: is -> are

[bcwhite, 2017/04/12 21:23:42]

Done.

+  // contents of the allocation in any way.
+  Reference ref = reference_->load(std::memory_order_relaxed);
+  if (!ref) {
+    ref = allocator_->Allocate(size_, type_);
+    if (!ref)
+      return nullptr;
+
+    // Store the new reference in its proper location using compare-and-swap.
+    // Use a "strong" exchange to ensure no false-negatives since the operation
+    // cannot be retried.
+    Reference existing = 0;  // Must be mutable; receives actual value.
+    if (reference_->compare_exchange_strong(existing, ref,
+                                            std::memory_order_relaxed,
+                                            std::memory_order_relaxed)) {
+      if (make_iterable_)
+        allocator_->MakeIterable(ref);
+    } else {
+      // Failure indicates that something else has raced ahead, performed the
+      // allocation, and stored its reference. Purge the allocation just done

[Alexei Svitkine (slow), 2017/04/12 17:04:25]

Nit: "that was just done" as otherwise it's a bit hard to read.

[bcwhite, 2017/04/12 21:23:42]

Done.

+      // and use the other one instead.
+      DCHECK_EQ(type_, allocator_->GetType(existing));
+      DCHECK_LE(size_, allocator_->GetAllocSize(existing));
+      allocator_->ChangeType(ref, 0, type_, /*clear=*/false);
+      ref = existing;
+    }
+  }
+
+  char* mem = allocator_->GetAsArray<char>(ref, type_, size_);
+  if (!mem) {
+    // This should never happen but be tolerant if it does as corruption from
+    // the outside is something to guard against.
+    NOTREACHED();
+    return nullptr;
+  }
+  return mem + offset_;
+}
+
 }  // namespace base