Index: base/trace_event/process_memory_dump.cc |
diff --git a/base/trace_event/process_memory_dump.cc b/base/trace_event/process_memory_dump.cc |
index 52eccbe1a0cfd8263eca4e7407b463e36fcc36ff..058cfee3b44f74264ad0a4ec5bb06d0f94bf44bb 100644 |
--- a/base/trace_event/process_memory_dump.cc |
+++ b/base/trace_event/process_memory_dump.cc |
@@ -4,6 +4,7 @@ |
#include "base/trace_event/process_memory_dump.h" |
+#include <ctype.h> |
#include <errno.h> |
#include <vector> |
@@ -12,6 +13,7 @@ |
#include "base/process/process_metrics.h" |
#include "base/strings/stringprintf.h" |
#include "base/trace_event/heap_profiler_heap_dump_writer.h" |
+#include "base/trace_event/memory_dump_request_args.h" |
#include "base/trace_event/process_memory_totals.h" |
#include "base/trace_event/trace_event_argument.h" |
#include "build/build_config.h" |
@@ -46,6 +48,51 @@ size_t GetSystemPageCount(size_t mapped_size, size_t page_size) { |
} |
#endif |
+// A list of string names that are allowed for the allocator dumps in background |
+// mode. |
+const char* const kDumpNameWhitelist[] = { |
Primiano Tucci (use gerrit)
2016/05/27 17:23:35
Ok I think at this point we want a memory_infra_ba
ssid
2016/05/31 22:33:20
Done.
|
+ // TODO(ssid): Fill this list with dump names, crbug.com/613198. |
+ "", |
+}; |
+ |
+// Check if the string is in the whitelist. |
+bool IsNameValidForBackgroundMode(const std::string& name, |
Primiano Tucci (use gerrit)
2016/05/27 17:23:35
s/Valid/Whitelisted/ ... which makes then the comm
ssid
2016/05/31 22:33:20
Acknowledged.
|
+ const char* whitelisted_name_for_testing) { |
Primiano Tucci (use gerrit)
2016/05/27 17:23:35
see my comment in the other cl, having to pass thi
ssid
2016/05/31 22:33:20
Done.
|
+ static size_t kListSize = arraysize(kDumpNameWhitelist); |
+ |
+ // Remove special characters, numbers (including hexadecimal which are makred |
Dmitry Skiba
2016/05/31 06:48:46
*marked
ssid
2016/05/31 22:33:20
Done.
|
+ // by '0x') from the given string. |
+ const size_t length = name.size(); |
+ std::unique_ptr<char[]> stripped_str(new char[length]); |
Dmitry Skiba
2016/05/31 06:48:46
Uhm, why not std::string? And push_back() instead
ssid
2016/05/31 22:33:20
Done.
|
+ size_t str_index = 0; |
+ bool parsing_hex = false; |
+ for (size_t i = 0; i < length; ++i) { |
+ if (parsing_hex) { |
+ if (isxdigit(name[i])) { |
Primiano Tucci (use gerrit)
2016/05/27 17:23:35
instead of having this state machine here, can you
ssid
2016/05/27 17:42:48
Sorry, I don't get it.
Yeah I would still need thi
Primiano Tucci (use gerrit)
2016/05/27 17:51:19
Ok sorry this was a stupid comment from my side. I
ssid
2016/05/31 22:33:20
isalpha is there because there could be cases wher
Primiano Tucci (use gerrit)
2016/06/02 20:24:04
Ah right, I think it's ok.
|
+ continue; |
+ } else { |
+ parsing_hex = false; |
+ } |
+ } |
+ if (i + 1 < length && name[i] == '0' && name[i + 1] == 'x') { |
+ parsing_hex = true; |
+ ++i; |
+ } else if (isalpha(name[i])) { |
+ stripped_str[str_index++] = name[i]; |
+ } |
+ } |
+ stripped_str[str_index] = '\0'; |
+ |
+ if (whitelisted_name_for_testing) |
+ return strcmp(stripped_str.get(), whitelisted_name_for_testing) == 0; |
+ |
+ for (size_t i = 0; i < kListSize; ++i) { |
+ if (strcmp(stripped_str.get(), kDumpNameWhitelist[i]) == 0) |
+ return true; |
+ } |
+ return false; |
+} |
+ |
} // namespace |
#if defined(COUNT_RESIDENT_BYTES_SUPPORTED) |
@@ -148,10 +195,17 @@ size_t ProcessMemoryDump::CountResidentBytes(void* start_address, |
#endif // defined(COUNT_RESIDENT_BYTES_SUPPORTED) |
ProcessMemoryDump::ProcessMemoryDump( |
- scoped_refptr<MemoryDumpSessionState> session_state) |
+ scoped_refptr<MemoryDumpSessionState> session_state, |
+ MemoryDumpLevelOfDetail level_of_detail) |
: has_process_totals_(false), |
has_process_mmaps_(false), |
- session_state_(std::move(session_state)) {} |
+ session_state_(std::move(session_state)), |
+ level_of_detail_(level_of_detail), |
+ whitelisted_name_for_testing_(nullptr) { |
+ if (level_of_detail_ == MemoryDumpLevelOfDetail::BACKGROUND) { |
+ dummy_mad_.reset(new MemoryAllocatorDump("dummy", this)); |
+ } |
+} |
ProcessMemoryDump::~ProcessMemoryDump() {} |
@@ -170,6 +224,13 @@ MemoryAllocatorDump* ProcessMemoryDump::CreateAllocatorDump( |
MemoryAllocatorDump* ProcessMemoryDump::AddAllocatorDumpInternal( |
std::unique_ptr<MemoryAllocatorDump> mad) { |
+ // In background mode return dummy dump if invalid dump name is given. |
+ if (level_of_detail_ == MemoryDumpLevelOfDetail::BACKGROUND && |
+ !IsNameValidForBackgroundMode(mad->absolute_name(), |
+ whitelisted_name_for_testing_)) { |
+ return dummy_mad_.get(); |
Primiano Tucci (use gerrit)
2016/05/27 17:23:35
maybe NOTREACHED also here to help debugging?
ssid
2016/05/31 22:33:20
I have changed the tests to use AddAllocatorDumpIn
Primiano Tucci (use gerrit)
2016/06/02 20:24:03
You can just check the allocator_dumps().count("na
|
+ } |
+ |
auto insertion_result = allocator_dumps_.insert( |
std::make_pair(mad->absolute_name(), std::move(mad))); |
MemoryAllocatorDump* inserted_mad = insertion_result.first->second.get(); |
@@ -181,7 +242,11 @@ MemoryAllocatorDump* ProcessMemoryDump::AddAllocatorDumpInternal( |
MemoryAllocatorDump* ProcessMemoryDump::GetAllocatorDump( |
const std::string& absolute_name) const { |
auto it = allocator_dumps_.find(absolute_name); |
- return it == allocator_dumps_.end() ? nullptr : it->second.get(); |
+ if (it != allocator_dumps_.end()) |
+ return it->second.get(); |
Primiano Tucci (use gerrit)
2016/05/27 17:23:35
should you just leave this as it is and return nul
ssid
2016/05/27 17:42:48
I can't do that. Some dump providers assume that t
Primiano Tucci (use gerrit)
2016/06/02 20:24:03
Acknowledged.
|
+ return level_of_detail_ == MemoryDumpLevelOfDetail::BACKGROUND |
+ ? dummy_mad_.get() |
+ : nullptr; |
} |
MemoryAllocatorDump* ProcessMemoryDump::GetOrCreateAllocatorDump( |
@@ -192,6 +257,10 @@ MemoryAllocatorDump* ProcessMemoryDump::GetOrCreateAllocatorDump( |
MemoryAllocatorDump* ProcessMemoryDump::CreateSharedGlobalAllocatorDump( |
const MemoryAllocatorDumpGuid& guid) { |
+ // Global dumps are disabled in background mode. |
+ if (level_of_detail_ == MemoryDumpLevelOfDetail::BACKGROUND) |
Primiano Tucci (use gerrit)
2016/05/27 17:23:35
I don't think global dumps are a problem right? th
ssid
2016/05/27 17:42:48
Global dumps are not required. Anyway we report ju
|
+ return dummy_mad_.get(); |
+ |
// A shared allocator dump can be shared within a process and the guid could |
// have been created already. |
MemoryAllocatorDump* mad = GetSharedGlobalAllocatorDump(guid); |
@@ -206,6 +275,10 @@ MemoryAllocatorDump* ProcessMemoryDump::CreateSharedGlobalAllocatorDump( |
MemoryAllocatorDump* ProcessMemoryDump::CreateWeakSharedGlobalAllocatorDump( |
const MemoryAllocatorDumpGuid& guid) { |
+ // Global dumps are disabled in background mode. |
+ if (level_of_detail_ == MemoryDumpLevelOfDetail::BACKGROUND) |
Primiano Tucci (use gerrit)
2016/05/27 17:23:35
ditto don't think you need this
|
+ return dummy_mad_.get(); |
+ |
MemoryAllocatorDump* mad = GetSharedGlobalAllocatorDump(guid); |
if (mad) |
return mad; |
@@ -333,6 +406,10 @@ void ProcessMemoryDump::AddOwnershipEdge( |
void ProcessMemoryDump::AddSuballocation(const MemoryAllocatorDumpGuid& source, |
const std::string& target_node_name) { |
+ // Do not create new dumps for suballocations in background mode. |
+ if (level_of_detail_ == MemoryDumpLevelOfDetail::BACKGROUND) |
Primiano Tucci (use gerrit)
2016/05/27 17:23:35
ditto
ssid
2016/05/27 17:42:48
Why create unnecessary dumps when we are not inter
|
+ return; |
+ |
std::string child_mad_name = target_node_name + "/__" + source.ToString(); |
MemoryAllocatorDump* target_child_mad = CreateAllocatorDump(child_mad_name); |
AddOwnershipEdge(source, target_child_mad->guid()); |