Change initialization strategy of AllocationRegister back

third_party/WebKit/Source/platform/PartitionAllocMemoryDumpProvider.cpp

 // Copyright 2015 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 "platform/PartitionAllocMemoryDumpProvider.h"
 
 #include "base/trace_event/heap_profiler_allocation_context.h"
 #include "base/trace_event/heap_profiler_allocation_context_tracker.h"
 #include "base/trace_event/heap_profiler_allocation_register.h"
 #include "base/trace_event/process_memory_dump.h"
@@ skipping 119 matching lines @@
     // This method calls memoryStats.partitionsDumpBucketStats with memory statistics.
     WTF::Partitions::dumpMemoryStats(levelOfDetail == WebMemoryDumpLevelOfDetail::Light, &partitionStatsDumper);
 
     WebMemoryAllocatorDump* allocatedObjectsDump = memoryDump->createMemoryAllocatorDump(String(Partitions::kAllocatedObjectPoolName));
     allocatedObjectsDump->addScalar("size", "bytes", partitionStatsDumper.totalActiveBytes());
     memoryDump->addOwnershipEdge(allocatedObjectsDump->guid(), partitionsDump->guid());
 
     return true;
 }
 
+// |m_allocationRegister| should be initialized only when necessary to avoid waste of memory.
 PartitionAllocMemoryDumpProvider::PartitionAllocMemoryDumpProvider()
-    : m_allocationRegister(adoptPtr(new base::trace_event::AllocationRegister()))
+    : m_allocationRegister(nullptr)
     , m_isHeapProfilingEnabled(false)
 {
 }
 
 PartitionAllocMemoryDumpProvider::~PartitionAllocMemoryDumpProvider()
 {
 }
 
 void PartitionAllocMemoryDumpProvider::onHeapProfilingEnabled(bool enabled)
 {
     if (enabled) {
+        {
+            MutexLocker locker(m_allocationRegisterMutex);
+            if (!m_allocationRegister)
+                m_allocationRegister = adoptPtr(new base::trace_event::AllocationRegister());
+        }
         PartitionAllocHooks::setAllocationHook(reportAllocation);
         PartitionAllocHooks::setFreeHook(reportFree);
     } else {
         PartitionAllocHooks::setAllocationHook(nullptr);
         PartitionAllocHooks::setFreeHook(nullptr);

[esprehn, 2016/02/09 07:07:13]

should we clear m_allocationRegister here? What eventually cleans up
m_allocationRegister?

[Primiano Tucci (use gerrit), 2016/02/09 11:00:24]

That's a good point, missed it.
So, right now there is nothing ever tearing down heap profiler once its started
(you only see the "enabled=true" part in this function) but that is a bug I'm
going to fix soon.

FWIW, I can guarantee you that we are never going to support re-enabling heap
profiling (enable -> disable -> enable).
The reason is that it is conceptually wrong. If you interrupt heap profiling you
lose data and the result is meaningless.
But not sure whether this is going to make a difference in this code at all.

     }
     m_isHeapProfilingEnabled = enabled;
 }
 
 void PartitionAllocMemoryDumpProvider::insert(void* address, size_t size, const char* typeName)
 {
     base::trace_event::AllocationContext context = base::trace_event::AllocationContextTracker::GetContextSnapshot();
     context.type_name = typeName;
     MutexLocker locker(m_allocationRegisterMutex);

[haraken, 2016/02/05 11:20:59]

Nit: It would be heavy to use MutexLocker in insert/remove. You could use
releaseStore/acquireLoad. You don't need to address it in this CL.

[Primiano Tucci (use gerrit), 2016/02/05 11:42:08]

Note that here we have also to protect against concurrent accesses to the
allocation register (Which is not thread safe)
is releaseStore/acquireLoad going to protect this?

-    m_allocationRegister->Insert(address, size, context);
+    if (m_allocationRegister)
+        m_allocationRegister->Insert(address, size, context);
 }
 
 void PartitionAllocMemoryDumpProvider::remove(void* address)
 {
     MutexLocker locker(m_allocationRegisterMutex);
-    m_allocationRegister->Remove(address);
+    if (m_allocationRegister)
+        m_allocationRegister->Remove(address);
 }
 
 } // namespace blink