Commit 3a30e747 authored by Joe Mason's avatar Joe Mason Committed by Chromium LUCI CQ

[PM] Split unassociated bytes into detached and shared bytes

Currently all unassociated bytes are lumped together into one counter.

There are actually two types of unassociated bytes:
- bytes of detached V8 contexts (i.e. contexts not attached to the
  frame tree.
- bytes shared between all V8 contexts.

Besides splitting the bytes, this CL also adds the corresponding
memory types and reports them in the web memory API.

Based on this patch by ulan@chromium.org:
https://chromium-review.googlesource.com/c/chromium/src/+/2626662

Bug: 1085129
Change-Id: I0097e3b1728f5cbfe72746f0a1fd804215487237
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2626978
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: default avatarUlan Degenbaev <ulan@chromium.org>
Auto-Submit: Joe Mason <joenotcharles@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843432}
parent c680219e
...@@ -63,7 +63,6 @@ struct WebMemoryBreakdownEntry { ...@@ -63,7 +63,6 @@ struct WebMemoryBreakdownEntry {
WebMemoryUsage? memory; WebMemoryUsage? memory;
array<WebMemoryAttribution> attribution; array<WebMemoryAttribution> attribution;
// TODO(1085129): Add memory types once they are implemented.
}; };
// The result of the MeasureMemory function. // The result of the MeasureMemory function.
...@@ -75,5 +74,14 @@ struct WebMemoryMeasurement { ...@@ -75,5 +74,14 @@ struct WebMemoryMeasurement {
kDefault, kDefault,
kEager kEager
}; };
// A breakdown of memory used for all ExecutionContexts that were measured.
array<WebMemoryBreakdownEntry> breakdown; array<WebMemoryBreakdownEntry> breakdown;
// Memory that is used by V8 contexts that are no longer attached to any
// ExecutionContext.
WebMemoryUsage? detached_memory;
// Memory that is not associated with any particular V8 context.
WebMemoryUsage? shared_memory;
}; };
...@@ -77,8 +77,11 @@ namespace v8_memory { ...@@ -77,8 +77,11 @@ namespace v8_memory {
// const V8DetailedMemoryProcessData* data) override { // const V8DetailedMemoryProcessData* data) override {
// DCHECK(data); // DCHECK(data);
// LOG(INFO) << "Process " << process_node->GetProcessId() << // LOG(INFO) << "Process " << process_node->GetProcessId() <<
// " reported " << data->unassociated_v8_bytes_used() << // " reported " << data->detached_v8_bytes_used() <<
// " bytes of V8 memory that wasn't associated with a frame."; // " bytes of V8 memory that wasn't associated with a frame.";
// LOG(INFO) << "Process " << process_node->GetProcessId() <<
// " reported " << data->shared_v8_bytes_used() <<
// " bytes of V8 memory that are shared between all frames";
// for (auto* frame_node : process_node->GetFrameNodes()) { // for (auto* frame_node : process_node->GetFrameNodes()) {
// auto* frame_data = // auto* frame_data =
// V8DetailedMemoryExecutionContextData::ForFrame(frame_node); // V8DetailedMemoryExecutionContextData::ForFrame(frame_node);
...@@ -142,8 +145,11 @@ namespace v8_memory { ...@@ -142,8 +145,11 @@ namespace v8_memory {
// return; // return;
// } // }
// LOG(INFO) << "Process " << process->GetID() << // LOG(INFO) << "Process " << process->GetID() <<
// " reported " << process_data.unassociated_v8_bytes_used() << // " reported " << process_data.detached_v8_bytes_used() <<
// " bytes of V8 memory that wasn't associated with a frame."; // " bytes of V8 memory that wasn't associated with a frame.";
// LOG(INFO) << "Process " << process_node->GetProcessId() <<
// " reported " << data->shared_v8_bytes_used() <<
// " bytes of V8 memory that are shared between all frames";
// for (std::pair< // for (std::pair<
// content::GlobalFrameRoutingId, // content::GlobalFrameRoutingId,
// V8DetailedMemoryExecutionContextData // V8DetailedMemoryExecutionContextData
...@@ -323,17 +329,24 @@ class V8DetailedMemoryProcessData { ...@@ -323,17 +329,24 @@ class V8DetailedMemoryProcessData {
virtual ~V8DetailedMemoryProcessData() = default; virtual ~V8DetailedMemoryProcessData() = default;
bool operator==(const V8DetailedMemoryProcessData& other) const { bool operator==(const V8DetailedMemoryProcessData& other) const {
return unassociated_v8_bytes_used_ == other.unassociated_v8_bytes_used_; return detached_v8_bytes_used_ == other.detached_v8_bytes_used_ &&
shared_v8_bytes_used_ == other.shared_v8_bytes_used_;
} }
// Returns the number of bytes used by V8 at the last measurement in this // Returns the number of bytes used by V8 at the last measurement in this
// process that could not be attributed to a frame. // process that could not be attributed to a frame.
uint64_t unassociated_v8_bytes_used() const { uint64_t detached_v8_bytes_used() const { return detached_v8_bytes_used_; }
return unassociated_v8_bytes_used_;
void set_detached_v8_bytes_used(uint64_t detached_v8_bytes_used) {
detached_v8_bytes_used_ = detached_v8_bytes_used;
} }
void set_unassociated_v8_bytes_used(uint64_t unassociated_v8_bytes_used) { // Returns the number of bytes used by V8 at the last measurement in this
unassociated_v8_bytes_used_ = unassociated_v8_bytes_used; // process that are shared between all frames.
uint64_t shared_v8_bytes_used() const { return shared_v8_bytes_used_; }
void set_shared_v8_bytes_used(uint64_t shared_v8_bytes_used) {
shared_v8_bytes_used_ = shared_v8_bytes_used;
} }
// Returns process data for the given node, or nullptr if no measurement has // Returns process data for the given node, or nullptr if no measurement has
...@@ -343,7 +356,8 @@ class V8DetailedMemoryProcessData { ...@@ -343,7 +356,8 @@ class V8DetailedMemoryProcessData {
const ProcessNode* node); const ProcessNode* node);
private: private:
uint64_t unassociated_v8_bytes_used_ = 0; uint64_t detached_v8_bytes_used_ = 0;
uint64_t shared_v8_bytes_used_ = 0;
}; };
class V8DetailedMemoryObserver : public base::CheckedObserver { class V8DetailedMemoryObserver : public base::CheckedObserver {
......
...@@ -466,8 +466,9 @@ void NodeAttachedProcessData::OnV8MemoryUsage( ...@@ -466,8 +466,9 @@ void NodeAttachedProcessData::OnV8MemoryUsage(
// Distribute the data to the frames. // Distribute the data to the frames.
// If a frame doesn't have corresponding data in the result, clear any data // If a frame doesn't have corresponding data in the result, clear any data
// it may have had. Any datum in the result that doesn't correspond to an // it may have had. Any datum in the result that doesn't correspond to an
// existing frame is likewise accured to unassociated usage. // existing frame is likewise accrued to detached bytes.
uint64_t unassociated_v8_bytes_used = 0; uint64_t detached_v8_bytes_used = 0;
uint64_t shared_v8_bytes_used = 0;
// Create a mapping from token to execution context usage for the merge below. // Create a mapping from token to execution context usage for the merge below.
std::vector<std::pair<blink::ExecutionContextToken, std::vector<std::pair<blink::ExecutionContextToken,
...@@ -477,7 +478,8 @@ void NodeAttachedProcessData::OnV8MemoryUsage( ...@@ -477,7 +478,8 @@ void NodeAttachedProcessData::OnV8MemoryUsage(
for (auto& entry : isolate->contexts) { for (auto& entry : isolate->contexts) {
tmp.emplace_back(entry->token, std::move(entry)); tmp.emplace_back(entry->token, std::move(entry));
} }
unassociated_v8_bytes_used += isolate->unassociated_bytes_used; detached_v8_bytes_used += isolate->detached_bytes_used;
shared_v8_bytes_used += isolate->shared_bytes_used;
} }
size_t found_frame_count = tmp.size(); size_t found_frame_count = tmp.size();
...@@ -522,12 +524,13 @@ void NodeAttachedProcessData::OnV8MemoryUsage( ...@@ -522,12 +524,13 @@ void NodeAttachedProcessData::OnV8MemoryUsage(
// Execution context was already consumed. // Execution context was already consumed.
continue; continue;
} }
// Accrue the data for non-existent frames to unassociated bytes. // Accrue the data for non-existent frames to detached bytes.
unassociated_v8_bytes_used += it.second->bytes_used; detached_v8_bytes_used += it.second->bytes_used;
} }
data_available_ = true; data_available_ = true;
data_.set_unassociated_v8_bytes_used(unassociated_v8_bytes_used); data_.set_detached_v8_bytes_used(detached_v8_bytes_used);
data_.set_shared_v8_bytes_used(shared_v8_bytes_used);
// Schedule another measurement for this process node unless one is already // Schedule another measurement for this process node unless one is already
// scheduled. // scheduled.
...@@ -1016,8 +1019,9 @@ base::Value V8DetailedMemoryDecorator::DescribeProcessNodeData( ...@@ -1016,8 +1019,9 @@ base::Value V8DetailedMemoryDecorator::DescribeProcessNodeData(
DCHECK_EQ(content::PROCESS_TYPE_RENDERER, process_node->GetProcessType()); DCHECK_EQ(content::PROCESS_TYPE_RENDERER, process_node->GetProcessType());
base::Value dict(base::Value::Type::DICTIONARY); base::Value dict(base::Value::Type::DICTIONARY);
dict.SetIntKey("unassociated_v8_bytes_used", dict.SetIntKey("detached_v8_bytes_used",
process_data->unassociated_v8_bytes_used()); process_data->detached_v8_bytes_used());
dict.SetIntKey("shared_v8_bytes_used", process_data->shared_v8_bytes_used());
return dict; return dict;
} }
......
...@@ -207,6 +207,18 @@ WebMemoryAggregator::AggregateMeasureMemoryResult() { ...@@ -207,6 +207,18 @@ WebMemoryAggregator::AggregateMeasureMemoryResult() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
aggregation_result_ = mojom::WebMemoryMeasurement::New(); aggregation_result_ = mojom::WebMemoryMeasurement::New();
VisitFrame(nullptr, aggregation_start_node_); VisitFrame(nullptr, aggregation_start_node_);
auto* process_data = V8DetailedMemoryProcessData::ForProcessNode(
aggregation_start_node_->GetProcessNode());
if (process_data) {
aggregation_result_->detached_memory = mojom::WebMemoryUsage::New();
aggregation_result_->detached_memory->bytes =
process_data->detached_v8_bytes_used();
aggregation_result_->shared_memory = mojom::WebMemoryUsage::New();
aggregation_result_->shared_memory->bytes =
process_data->shared_v8_bytes_used();
}
return std::move(aggregation_result_); return std::move(aggregation_result_);
} }
......
...@@ -14,16 +14,15 @@ struct PerContextV8MemoryUsage { ...@@ -14,16 +14,15 @@ struct PerContextV8MemoryUsage {
}; };
struct PerIsolateV8MemoryUsage { struct PerIsolateV8MemoryUsage {
// The number of V8 heap bytes that were not associated with a specific
// V8 context, most likely because they're shared objects.
uint64 unassociated_bytes_used;
// The number of V8 contexts not associated with an execution context, // The number of V8 contexts not associated with an execution context,
// likely web application leaks, and their associated byte usage. At the present time // likely web application leaks, and their associated byte usage.
// (April 2020), it's expected and normal to see one unassociated context per // Such contexts are known as detached contexts.
// renderer process accounting for ~70kB. uint64 num_detached_contexts;
uint64 num_unassociated_contexts; uint64 detached_bytes_used;
uint64 unassociated_context_bytes_used;
// Bytes shared across all V8 contexts. These bytes are not attributed
// to any particular V8 context.
uint64 shared_bytes_used;
// The V8 memory usage by individual V8 contexts in this process. // The V8 memory usage by individual V8 contexts in this process.
array<PerContextV8MemoryUsage> contexts; array<PerContextV8MemoryUsage> contexts;
......
...@@ -63,8 +63,8 @@ class FrameAssociatedMeasurementDelegate : public v8::MeasureMemoryDelegate { ...@@ -63,8 +63,8 @@ class FrameAssociatedMeasurementDelegate : public v8::MeasureMemoryDelegate {
// TODO(crbug.com/1080672): It would be prefereable to count the // TODO(crbug.com/1080672): It would be prefereable to count the
// V8SchemaRegistry context's overhead with unassociated_bytes, but at // V8SchemaRegistry context's overhead with unassociated_bytes, but at
// present there isn't a public API that allows this distinction. // present there isn't a public API that allows this distinction.
++(isolate_memory_usage->num_unassociated_contexts); ++(isolate_memory_usage->num_detached_contexts);
isolate_memory_usage->unassociated_context_bytes_used += size; isolate_memory_usage->detached_bytes_used += size;
continue; continue;
} }
if (DOMWrapperWorld::World(context).GetWorldId() != if (DOMWrapperWorld::World(context).GetWorldId() !=
...@@ -85,7 +85,7 @@ class FrameAssociatedMeasurementDelegate : public v8::MeasureMemoryDelegate { ...@@ -85,7 +85,7 @@ class FrameAssociatedMeasurementDelegate : public v8::MeasureMemoryDelegate {
#endif #endif
isolate_memory_usage->contexts.push_back(std::move(context_memory_usage)); isolate_memory_usage->contexts.push_back(std::move(context_memory_usage));
} }
isolate_memory_usage->unassociated_bytes_used = unattributed_size_in_bytes; isolate_memory_usage->shared_bytes_used = unattributed_size_in_bytes;
std::move(callback_).Run(std::move(isolate_memory_usage)); std::move(callback_).Run(std::move(isolate_memory_usage));
} }
......
...@@ -34,6 +34,7 @@ using performance_manager::mojom::blink::WebMemoryAttributionPtr; ...@@ -34,6 +34,7 @@ using performance_manager::mojom::blink::WebMemoryAttributionPtr;
using performance_manager::mojom::blink::WebMemoryBreakdownEntryPtr; using performance_manager::mojom::blink::WebMemoryBreakdownEntryPtr;
using performance_manager::mojom::blink::WebMemoryMeasurement; using performance_manager::mojom::blink::WebMemoryMeasurement;
using performance_manager::mojom::blink::WebMemoryMeasurementPtr; using performance_manager::mojom::blink::WebMemoryMeasurementPtr;
using performance_manager::mojom::blink::WebMemoryUsagePtr;
namespace blink { namespace blink {
...@@ -180,6 +181,19 @@ MemoryBreakdownEntry* ConvertBreakdown( ...@@ -180,6 +181,19 @@ MemoryBreakdownEntry* ConvertBreakdown(
return result; return result;
} }
MemoryBreakdownEntry* CreateUnattributedBreakdown(
const WebMemoryUsagePtr& memory,
const WTF::String& memory_type) {
auto* result = MemoryBreakdownEntry::Create();
DCHECK(memory);
result->setBytes(memory->bytes);
result->setAttribution({});
Vector<String> types;
types.push_back(memory_type);
result->setTypes(types);
return result;
}
MemoryBreakdownEntry* EmptyBreakdown() { MemoryBreakdownEntry* EmptyBreakdown() {
auto* result = MemoryBreakdownEntry::Create(); auto* result = MemoryBreakdownEntry::Create();
result->setBytes(0); result->setBytes(0);
...@@ -195,6 +209,11 @@ MemoryMeasurement* ConvertResult(const WebMemoryMeasurementPtr& measurement) { ...@@ -195,6 +209,11 @@ MemoryMeasurement* ConvertResult(const WebMemoryMeasurementPtr& measurement) {
if (entry->memory) if (entry->memory)
breakdown.push_back(ConvertBreakdown(entry)); breakdown.push_back(ConvertBreakdown(entry));
} }
// Add breakdowns for memory that isn't attributed to an execution context.
breakdown.push_back(
CreateUnattributedBreakdown(measurement->detached_memory, "Detached"));
breakdown.push_back(
CreateUnattributedBreakdown(measurement->shared_memory, "Shared"));
// Add an empty breakdown entry as required by the spec. // Add an empty breakdown entry as required by the spec.
// See https://github.com/WICG/performance-measure-memory/issues/10. // See https://github.com/WICG/performance-measure-memory/issues/10.
breakdown.push_back(EmptyBreakdown()); breakdown.push_back(EmptyBreakdown());
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment