Skip to content

Commit

Permalink
memory-infra: add struct-traits for memory dump objects
Browse files Browse the repository at this point in the history 
Introduce struct-traits for core memory-infra classes
and corresponding tests.
These struct traits will be used in the upcoming CLs
by the memory-instrumentation service to rationalize
the memory dumps in-process.

Note: the heap-profiler structures are deliberately
not serialized as there is no interest in carrying
them across processes.

BUG: 728199
Change-Id: Iea4b6fcbd56f6bc2c951a8421b9452e5900083ae
Reviewed-on: https://chromium-review.googlesource.com/654858
Commit-Queue: Primiano Tucci <primiano@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Siddhartha S <ssid@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500881}
  • Loading branch information
@primiano
primiano authored and Commit Bot committed Sep 11, 2017
1 parent 029e0ad commit 9b16a3b
Show file tree
Hide file tree
Showing 10 changed files with 597 additions and 35 deletions.
63 changes: 51 additions & 12 deletions base/trace_event/memory_allocator_dump.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

#include "base/trace_event/memory_allocator_dump.h"

#include <string.h>

#include "base/format_macros.h"
#include "base/memory/ptr_util.h"
#include "base/strings/stringprintf.h"
Expand All @@ -30,14 +32,14 @@ MemoryAllocatorDumpGuid MemoryAllocatorDump::GetDumpIdFromName(
"%d:%s", TraceLog::GetInstance()->process_id(), absolute_name.c_str()));
}

MemoryAllocatorDump::MemoryAllocatorDump(const std::string& absolute_name,
ProcessMemoryDump* process_memory_dump,
const MemoryAllocatorDumpGuid& guid)
MemoryAllocatorDump::MemoryAllocatorDump(
const std::string& absolute_name,
MemoryDumpLevelOfDetail level_of_detail,
const MemoryAllocatorDumpGuid& guid)
: absolute_name_(absolute_name),
process_memory_dump_(process_memory_dump),
guid_(guid),
flags_(Flags::DEFAULT),
size_(0) {
level_of_detail_(level_of_detail),
flags_(Flags::DEFAULT) {
// The |absolute_name| cannot be empty.
DCHECK(!absolute_name.empty());

Expand All @@ -46,6 +48,15 @@ MemoryAllocatorDump::MemoryAllocatorDump(const std::string& absolute_name,
DCHECK(absolute_name[0] != '/' && *absolute_name.rbegin() != '/');
}

MemoryAllocatorDump::MemoryAllocatorDump(const std::string& absolute_name,
ProcessMemoryDump* process_memory_dump,
const MemoryAllocatorDumpGuid& guid)
: MemoryAllocatorDump(absolute_name,
process_memory_dump
? process_memory_dump->dump_args().level_of_detail
: MemoryDumpLevelOfDetail::FIRST,
guid) {}

// If the caller didn't provide a guid, make one up by hashing the
// absolute_name with the current PID.
// Rationale: |absolute_name| is already supposed to be unique within a
Expand All @@ -63,17 +74,14 @@ MemoryAllocatorDump::~MemoryAllocatorDump() {
void MemoryAllocatorDump::AddScalar(const char* name,
const char* units,
uint64_t value) {
if (strcmp(kNameSize, name) == 0)
size_ = value;
entries_.emplace_back(name, units, value);
}

void MemoryAllocatorDump::AddString(const char* name,
const char* units,
const std::string& value) {
// String attributes are disabled in background mode.
if (process_memory_dump_->dump_args().level_of_detail ==
MemoryDumpLevelOfDetail::BACKGROUND) {
if (level_of_detail_ == MemoryDumpLevelOfDetail::BACKGROUND) {
NOTREACHED();
return;
}
Expand Down Expand Up @@ -114,14 +122,31 @@ void MemoryAllocatorDump::AsValueInto(TracedValue* value) const {
value->EndDictionary(); // "allocator_name/heap_subheap": { ... }
}

uint64_t MemoryAllocatorDump::GetSizeInternal() const {
if (cached_size_.has_value())
return *cached_size_;
for (const auto& entry : entries_) {
if (entry.entry_type == Entry::kUint64 && entry.units == kUnitsBytes &&
strcmp(entry.name.c_str(), kNameSize) == 0) {
cached_size_ = entry.value_uint64;
return entry.value_uint64;
}
}
return 0;
};

std::unique_ptr<TracedValue> MemoryAllocatorDump::attributes_for_testing()
const {
std::unique_ptr<TracedValue> attributes = base::MakeUnique<TracedValue>();
std::unique_ptr<TracedValue> attributes = MakeUnique<TracedValue>();
DumpAttributes(attributes.get());
return attributes;
}

MemoryAllocatorDump::Entry::Entry(Entry&& other) = default;
MemoryAllocatorDump::Entry::Entry() : entry_type(kString), value_uint64() {}
MemoryAllocatorDump::Entry::Entry(MemoryAllocatorDump::Entry&&) noexcept =
default;
MemoryAllocatorDump::Entry& MemoryAllocatorDump::Entry::operator=(
MemoryAllocatorDump::Entry&&) = default;

MemoryAllocatorDump::Entry::Entry(std::string name,
std::string units,
Expand All @@ -145,5 +170,19 @@ bool MemoryAllocatorDump::Entry::operator==(const Entry& rhs) const {
return false;
}

void PrintTo(const MemoryAllocatorDump::Entry& entry, std::ostream* out) {
switch (entry.entry_type) {
case MemoryAllocatorDump::Entry::EntryType::kUint64:
*out << "<Entry(\"" << entry.name << "\", \"" << entry.units << "\", "
<< entry.value_uint64 << ")>";
return;
case MemoryAllocatorDump::Entry::EntryType::kString:
*out << "<Entry(\"" << entry.name << "\", \"" << entry.units << "\", \""
<< entry.value_string << "\")>";
return;
}
NOTREACHED();
}

} // namespace trace_event
} // namespace base
42 changes: 32 additions & 10 deletions base/trace_event/memory_allocator_dump.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,16 @@
#include <stdint.h>

#include <memory>
#include <ostream>
#include <string>

#include "base/base_export.h"
#include "base/gtest_prod_util.h"
#include "base/logging.h"
#include "base/macros.h"
#include "base/optional.h"
#include "base/trace_event/memory_allocator_dump_guid.h"
#include "base/trace_event/memory_dump_request_args.h"
#include "base/trace_event/trace_event_argument.h"
#include "base/values.h"

Expand Down Expand Up @@ -47,9 +50,11 @@ class BASE_EXPORT MemoryAllocatorDump {
// indefinitely lived const char* strings, the only reason we copy
// them into a std::string is to handle Mojo (de)serialization.
// TODO(hjd): Investigate optimization (e.g. using StringPiece).
Entry(); // Only for deserialization.
Entry(std::string name, std::string units, uint64_t value);
Entry(std::string name, std::string units, std::string value);
Entry(Entry&& other);
Entry(Entry&& other) noexcept;
Entry& operator=(Entry&& other);
bool operator==(const Entry& rhs) const;

std::string name;
Expand All @@ -69,11 +74,15 @@ class BASE_EXPORT MemoryAllocatorDump {
const std::string& absolute_name);

// MemoryAllocatorDump is owned by ProcessMemoryDump.
// TODO(primiano): remove this constructor, ProcessMemoryDump* is not used
// for anything other than extracting the MemoryDumpLevelOfDetail these days.
MemoryAllocatorDump(const std::string& absolute_name,
ProcessMemoryDump* process_memory_dump,
const MemoryAllocatorDumpGuid& guid);
ProcessMemoryDump*,
const MemoryAllocatorDumpGuid&);
MemoryAllocatorDump(const std::string& absolute_name,
ProcessMemoryDump* process_memory_dump);
MemoryDumpLevelOfDetail,
const MemoryAllocatorDumpGuid&);
MemoryAllocatorDump(const std::string& absolute_name, ProcessMemoryDump*);
~MemoryAllocatorDump();

// Standard attribute |name|s for the AddScalar and AddString() methods.
Expand Down Expand Up @@ -106,13 +115,15 @@ class BASE_EXPORT MemoryAllocatorDump {

// Get the size for this dump.
// The size is the value set with AddScalar(kNameSize, kUnitsBytes, size);
// TODO(hjd): Transitional until we send the full PMD. See crbug.com/704203
uint64_t GetSizeInternal() const { return size_; };
// TODO(hjd): this should return an Optional<uint64_t>.
uint64_t GetSizeInternal() const;

MemoryDumpLevelOfDetail level_of_detail() const { return level_of_detail_; }

// Use enum Flags to set values.
void set_flags(int flags) { flags_ |= flags; }
void clear_flags(int flags) { flags_ &= ~flags; }
int flags() { return flags_; }
int flags() const { return flags_; }

// |guid| is an optional global dump identifier, unique across all processes
// within the scope of a global dump. It is only required when using the
Expand All @@ -124,6 +135,15 @@ class BASE_EXPORT MemoryAllocatorDump {

const std::vector<Entry>& entries_for_testing() const { return entries_; }

// Only for mojo serialization, which can mutate the collection.
std::vector<Entry>* mutable_entries_for_serialization() const {
cached_size_.reset(); // The caller can mutate the collection.

// Mojo takes a const input argument even for move-only types that can be
// mutate while serializing (like this one). Hence the const_cast.
return const_cast<std::vector<Entry>*>(&entries_);
}

// Decprecated testing method. Use entries_for_testing instead.
// TODO(hjd): Remove this and refactor callers to use entries_for_testing then
// inline DumpAttributes.
Expand All @@ -133,16 +153,18 @@ class BASE_EXPORT MemoryAllocatorDump {
void DumpAttributes(TracedValue* value) const;

const std::string absolute_name_;
ProcessMemoryDump* const process_memory_dump_; // Not owned (PMD owns this).
MemoryAllocatorDumpGuid guid_;
MemoryDumpLevelOfDetail level_of_detail_;
int flags_; // See enum Flags.
uint64_t size_;

mutable Optional<uint64_t> cached_size_; // Lazy, for GetSizeInternal().
std::vector<Entry> entries_;

DISALLOW_COPY_AND_ASSIGN(MemoryAllocatorDump);
};

// This is required by gtest to print a readable output on test failures.
void BASE_EXPORT PrintTo(const MemoryAllocatorDump::Entry&, std::ostream*);

} // namespace trace_event
} // namespace base

Expand Down
1 change: 0 additions & 1 deletion base/trace_event/memory_allocator_dump_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,6 @@ TEST(MemoryAllocatorDumpTest, ForbidStringsInBackgroundModeDeathTest) {
MemoryAllocatorDump* dump = pmd.CreateAllocatorDump("malloc");
ASSERT_DEATH(dump->AddString("foo", "bar", "baz"), "");
}

#endif

} // namespace trace_event
Expand Down
37 changes: 36 additions & 1 deletion base/trace_event/process_memory_dump.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,6 @@ base::Optional<size_t> ProcessMemoryDump::CountResidentBytesInSharedMemory(

#endif // defined(COUNT_RESIDENT_BYTES_SUPPORTED)

ProcessMemoryDump::ProcessMemoryDump() {}
ProcessMemoryDump::ProcessMemoryDump(
scoped_refptr<HeapProfilerSerializationState>
heap_profiler_serialization_state,
Expand Down Expand Up @@ -300,6 +299,31 @@ void ProcessMemoryDump::DumpHeapUsage(
overhead.DumpInto(base_name.c_str(), this);
}

void ProcessMemoryDump::SetAllocatorDumpsForSerialization(
std::vector<std::unique_ptr<MemoryAllocatorDump>> dumps) {
DCHECK(allocator_dumps_.empty());
for (std::unique_ptr<MemoryAllocatorDump>& dump : dumps)
AddAllocatorDumpInternal(std::move(dump));
}

std::vector<ProcessMemoryDump::MemoryAllocatorDumpEdge>
ProcessMemoryDump::GetAllEdgesForSerialization() const {
std::vector<MemoryAllocatorDumpEdge> edges;
edges.reserve(allocator_dumps_edges_.size());
for (const auto& it : allocator_dumps_edges_)
edges.push_back(it.second);
return edges;
}

void ProcessMemoryDump::SetAllEdgesForSerialization(
const std::vector<ProcessMemoryDump::MemoryAllocatorDumpEdge>& edges) {
DCHECK(allocator_dumps_edges_.empty());
for (const MemoryAllocatorDumpEdge& edge : edges) {
auto it_and_inserted = allocator_dumps_edges_.emplace(edge.source, edge);
DCHECK(it_and_inserted.second);
}
}

void ProcessMemoryDump::Clear() {
allocator_dumps_.clear();
allocator_dumps_edges_.clear();
Expand Down Expand Up @@ -454,5 +478,16 @@ MemoryAllocatorDump* ProcessMemoryDump::GetBlackHoleMad() {
return black_hole_mad_.get();
}

bool ProcessMemoryDump::MemoryAllocatorDumpEdge::operator==(
const MemoryAllocatorDumpEdge& other) const {
return source == other.source && target == other.target &&
importance == other.importance && overridable == other.overridable;
}

bool ProcessMemoryDump::MemoryAllocatorDumpEdge::operator!=(
const MemoryAllocatorDumpEdge& other) const {
return !(*this == other);
}

} // namespace trace_event
} // namespace base
23 changes: 18 additions & 5 deletions base/trace_event/process_memory_dump.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,10 @@ class TracedValue;
// produced by the MemoryDumpProvider(s) for a specific process.
class BASE_EXPORT ProcessMemoryDump {
public:
struct MemoryAllocatorDumpEdge {
struct BASE_EXPORT MemoryAllocatorDumpEdge {
bool operator==(const MemoryAllocatorDumpEdge&) const;
bool operator!=(const MemoryAllocatorDumpEdge&) const;

MemoryAllocatorDumpGuid source;
MemoryAllocatorDumpGuid target;
int importance;
Expand All @@ -50,10 +53,9 @@ class BASE_EXPORT ProcessMemoryDump {
// Maps allocator dumps absolute names (allocator_name/heap/subheap) to
// MemoryAllocatorDump instances.
using AllocatorDumpsMap =
std::unordered_map<std::string, std::unique_ptr<MemoryAllocatorDump>>;
std::map<std::string, std::unique_ptr<MemoryAllocatorDump>>;

using HeapDumpsMap =
std::unordered_map<std::string, std::unique_ptr<TracedValue>>;
using HeapDumpsMap = std::map<std::string, std::unique_ptr<TracedValue>>;

// Stores allocator dump edges indexed by source allocator dump GUID.
using AllocatorDumpEdgesMap =
Expand All @@ -78,7 +80,6 @@ class BASE_EXPORT ProcessMemoryDump {
const SharedMemory& shared_memory);
#endif

ProcessMemoryDump();
ProcessMemoryDump(scoped_refptr<HeapProfilerSerializationState>
heap_profiler_serialization_state,
const MemoryDumpArgs& dump_args);
Expand Down Expand Up @@ -133,6 +134,18 @@ class BASE_EXPORT ProcessMemoryDump {
// Returns the map of the MemoryAllocatorDumps added to this dump.
const AllocatorDumpsMap& allocator_dumps() const { return allocator_dumps_; }

AllocatorDumpsMap* mutable_allocator_dumps_for_serialization() const {
// Mojo takes a const input argument even for move-only types that can be
// mutate while serializing (like this one). Hence the const_cast.
return const_cast<AllocatorDumpsMap*>(&allocator_dumps_);
}
void SetAllocatorDumpsForSerialization(
std::vector<std::unique_ptr<MemoryAllocatorDump>>);

// Only for mojo serialization.
std::vector<MemoryAllocatorDumpEdge> GetAllEdgesForSerialization() const;
void SetAllEdgesForSerialization(const std::vector<MemoryAllocatorDumpEdge>&);

// Dumps heap usage with |allocator_name|.
void DumpHeapUsage(
const std::unordered_map<base::trace_event::AllocationContext,
Expand Down
10 changes: 9 additions & 1 deletion ...ces/resource_coordinator/public/cpp/memory_instrumentation/memory_instrumentation.typemap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@

mojom = "//services/resource_coordinator/public/interfaces/memory_instrumentation/memory_instrumentation.mojom"

public_headers = [ "//base/trace_event/memory_dump_request_args.h" ]
public_headers = [
"//base/trace_event/memory_dump_request_args.h",
"//base/trace_event/process_memory_dump.h",
"//base/trace_event/memory_allocator_dump.h",
]
traits_headers = [ "//services/resource_coordinator/public/cpp/memory_instrumentation/memory_instrumentation_struct_traits.h" ]
deps = [
"//base",
Expand All @@ -13,4 +17,8 @@ type_mappings = [
"memory_instrumentation.mojom.DumpType=base::trace_event::MemoryDumpType",
"memory_instrumentation.mojom.LevelOfDetail=base::trace_event::MemoryDumpLevelOfDetail",
"memory_instrumentation.mojom.RequestArgs=base::trace_event::MemoryDumpRequestArgs",
"memory_instrumentation.mojom.RawAllocatorDumpEdge=base::trace_event::ProcessMemoryDump::MemoryAllocatorDumpEdge",
"memory_instrumentation.mojom.RawAllocatorDumpEntry=base::trace_event::MemoryAllocatorDump::Entry[move_only]",
"memory_instrumentation.mojom.RawAllocatorDump=std::unique_ptr<base::trace_event::MemoryAllocatorDump>[move_only]",
"memory_instrumentation.mojom.RawProcessMemoryDump=std::unique_ptr<base::trace_event::ProcessMemoryDump>[move_only]",
]
Loading

0 comments on commit 9b16a3b

Please sign in to comment.