Skip to content

GitLab

T
tangled
Commit fbbeb3c8 authored by Xi Cheng's avatar Xi Cheng Committed by Commit Bot
Browse files
Options

Serialize all profiles from browser process to save memory 

Bug: 888716, 851163
Change-Id: I4ac176bf42d716f163717022352793dee763399a
Reviewed-on: https://chromium-review.googlesource.com/c/1327704
Commit-Queue: Xi Cheng <chengx@chromium.org>
Reviewed-by: default avatarMike Wittman <wittman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606962}
parent 59703d87
Show whitespace changes
Inline Side-by-side
Showing with 26 additions and 119 deletions
components/metrics/call_stack_profile_metrics_provider.cc
View file @ fbbeb3c8
......@@ -4,8 +4,6 @@
#include "components/metrics/call_stack_profile_metrics_provider.h"
#include <algorithm>
#include <iterator>
#include <utility>
#include <vector>
......@@ -24,55 +22,12 @@ namespace metrics {
namespace {
// Cap the number of pending profiles to avoid excessive memory usage when
// profile uploads are delayed (e.g. due to being offline). 1250 profiles
// corresponds to 80MB of storage. Capping at this threshold loses approximately
// 0.5% of profiles on canary and dev.
// profile uploads are delayed (e.g. due to being offline). Capping at this
// threshold loses approximately 0.5% of profiles on canary and dev.
// TODO(chengx): Remove this threshold after moving to a more memory-efficient
// profile representation.
const size_t kMaxPendingProfiles = 1250;
// Cap the number of pending unserialized profiles to avoid excessive memory
// usage when profile uploads are delayed (e.g., due to being offline). When the
// number of pending unserialized profiles exceeds this cap, serialize all
// additional unserialized profiles to save memory. Since profile serialization
// and deserialization (required later for uploads) are expensive, we choose 250
// as the capacity to balance speed and memory. 250 unserialized profiles
// corresponds to 16MB of storage.
// TODO(chengx): Remove this threshold after moving to a more memory-efficient
// profile representation.
constexpr size_t kMaxPendingUnserializedProfiles = 250;
// Merges the serialized profiles into the unserialized ones, by appending them.
// The params are not const& because they should be passed using std::move.
// Implementation note: In order to maintain the invariant that profiles are
// reported in correct temporal sequence, it's important to order the serialized
// profiles to follow the unserialized profiles. This way, profiles that were
// serialized simply for space efficiency will end up ordered correctly.
std::vector<SampledProfile> MergeProfiles(
std::vector<SampledProfile> profiles,
std::vector<std::string> serialized_profiles) {
// Deserialize all serialized profiles, skipping over any that fail to parse.
std::vector<SampledProfile> deserialized_profiles;
deserialized_profiles.reserve(serialized_profiles.size());
for (const auto& serialized_profile : serialized_profiles) {
base::ElapsedTimer timer;
SampledProfile profile;
if (profile.ParseFromArray(serialized_profile.data(),
serialized_profile.size())) {
UMA_HISTOGRAM_TIMES("StackSamplingProfiler.ProfileDeserializationTime",
timer.Elapsed());
deserialized_profiles.push_back(std::move(profile));
}
}
// Merge the profiles.
profiles.reserve(profiles.size() + deserialized_profiles.size());
std::move(deserialized_profiles.begin(), deserialized_profiles.end(),
std::back_inserter(profiles));
return profiles;
}
// PendingProfiles ------------------------------------------------------------
// Singleton class responsible for retaining profiles received from
......@@ -95,10 +50,9 @@ class PendingProfiles {
// profiles provided to future invocations of MaybeCollect*Profile.
void SetCollectionEnabled(bool enabled);
// Collects |profile|. It may be stored as it is, or in a serialized form, or
// ignored, depending on the pre-defined storage capacity and whether
// collection is enabled. |profile| is not const& because it must be passed
// with std::move.
// Collects |profile|. It may be stored in a serialized form, or ignored,
// depending on the pre-defined storage capacity and whether collection is
// enabled. |profile| is not const& because it must be passed with std::move.
void MaybeCollectProfile(base::TimeTicks profile_start_time,
SampledProfile profile);
......@@ -124,10 +78,6 @@ class PendingProfiles {
bool IsCollectionEnabledForProfile(base::TimeTicks profile_start_time) const
EXCLUSIVE_LOCKS_REQUIRED(lock_);
// Whether there is spare capacity to store an additional profile.
// The |lock_| must be held prior to calling this method.
bool HasSpareCapacity() const EXCLUSIVE_LOCKS_REQUIRED(lock_);
mutable base::Lock lock_;
// If true, profiles provided to MaybeCollect*Profile should be collected.
......@@ -142,9 +92,6 @@ class PendingProfiles {
// enabled at any point since a profile was started.
base::TimeTicks last_collection_enable_time_ GUARDED_BY(lock_);
// The set of completed unserialized profiles that should be reported.
std::vector<SampledProfile> unserialized_profiles_ GUARDED_BY(lock_);
// The set of completed serialized profiles that should be reported.
std::vector<std::string> serialized_profiles_ GUARDED_BY(lock_);
......@@ -159,19 +106,28 @@ PendingProfiles* PendingProfiles::GetInstance() {
}
std::vector<SampledProfile> PendingProfiles::RetrieveProfiles() {
std::vector<SampledProfile> profiles;
std::vector<std::string> serialized_profiles;
{
base::AutoLock scoped_lock(lock_);
profiles.swap(unserialized_profiles_);
serialized_profiles.swap(serialized_profiles_);
}
// Merge the serialized profiles by deserializing them. Note that this work is
// performed without holding the lock, to avoid blocking the lock for an
// extended period of time.
return MergeProfiles(std::move(profiles), std::move(serialized_profiles));
// Deserialize all serialized profiles, skipping over any that fail to parse.
std::vector<SampledProfile> profiles;
profiles.reserve(serialized_profiles.size());
for (const auto& serialized_profile : serialized_profiles) {
base::ElapsedTimer timer;
SampledProfile profile;
if (profile.ParseFromArray(serialized_profile.data(),
serialized_profile.size())) {
UMA_HISTOGRAM_TIMES("StackSamplingProfiler.ProfileDeserializationTime",
timer.Elapsed());
profiles.push_back(std::move(profile));
}
}
return profiles;
}
void PendingProfiles::SetCollectionEnabled(bool enabled) {
......@@ -180,7 +136,6 @@ void PendingProfiles::SetCollectionEnabled(bool enabled) {
collection_enabled_ = enabled;
if (!collection_enabled_) {
unserialized_profiles_.clear();
serialized_profiles_.clear();
last_collection_disable_time_ = base::TimeTicks::Now();
} else {
......@@ -215,12 +170,6 @@ bool PendingProfiles::IsCollectionEnabledForProfile(
return true;
}
bool PendingProfiles::HasSpareCapacity() const {
lock_.AssertAcquired();
return (unserialized_profiles_.size() + serialized_profiles_.size()) <
kMaxPendingProfiles;
}
void PendingProfiles::MaybeCollectProfile(base::TimeTicks profile_start_time,
SampledProfile profile) {
{
......@@ -228,24 +177,9 @@ void PendingProfiles::MaybeCollectProfile(base::TimeTicks profile_start_time,
if (!IsCollectionEnabledForProfile(profile_start_time))
return;
// Store the unserialized profile directly if there's room.
if (unserialized_profiles_.size() < kMaxPendingUnserializedProfiles) {
unserialized_profiles_.push_back(std::move(profile));
return;
}
// This early return is strictly a performance optimization to avoid doing
// unnecessary serialization below. For correctness, since the
// serialization happens without holding the lock, it's necessary to check
// this condition again prior to actually collecting the serialized profile.
if (!HasSpareCapacity())
return;
}
// There was no room to store the unserialized profile directly, but there was
// room to store it in serialized form. Serialize the profile without holding
// the lock, then try again to store it.
// Serialize the profile without holding the lock.
base::ElapsedTimer timer;
std::string serialized_profile;
profile.SerializeToString(&serialized_profile);
......@@ -261,7 +195,11 @@ void PendingProfiles::MaybeCollectSerializedProfile(
std::string serialized_profile) {
base::AutoLock scoped_lock(lock_);
if (IsCollectionEnabledForProfile(profile_start_time) && HasSpareCapacity())
// There is no room for additional profiles.
if (serialized_profiles_.size() >= kMaxPendingProfiles)
return;
if (IsCollectionEnabledForProfile(profile_start_time))
serialized_profiles_.push_back(std::move(serialized_profile));
}
......@@ -271,7 +209,6 @@ void PendingProfiles::ResetToDefaultStateForTesting() {
collection_enabled_ = true;
last_collection_disable_time_ = base::TimeTicks();
last_collection_enable_time_ = base::TimeTicks();
unserialized_profiles_.clear();
serialized_profiles_.clear();
}
......
components/metrics/call_stack_profile_metrics_provider_unittest.cc
View file @ fbbeb3c8
......@@ -105,36 +105,6 @@ TEST_F(CallStackProfileMetricsProviderTest,
uma_proto.sampled_profile(1).trigger_event());
}
// Checks that the unserialized pending profiles whose number exceeds the
// associated cap are still encoded in the session data.
TEST_F(CallStackProfileMetricsProviderTest,
ProvideCurrentSessionDataExceedUnserializedCap) {
// The value must be consistent with that in
// call_stack_profile_metrics_provider.cc so that this test is meaningful.
constexpr int kMaxPendingUnserializedProfiles = 250;
CallStackProfileMetricsProvider provider;
provider.OnRecordingEnabled();
// Receive (kMaxPendingUnserializedProfiles + 1) unserialized profiles.
for (int i = 0; i < kMaxPendingUnserializedProfiles + 1; ++i) {
SampledProfile profile;
profile.set_trigger_event(SampledProfile::PERIODIC_COLLECTION);
CallStackProfileMetricsProvider::ReceiveProfile(base::TimeTicks::Now(),
std::move(profile));
}
ChromeUserMetricsExtension uma_proto;
provider.ProvideCurrentSessionData(&uma_proto);
ASSERT_EQ(kMaxPendingUnserializedProfiles + 1,
uma_proto.sampled_profile().size());
for (int i = 0; i < kMaxPendingUnserializedProfiles + 1; ++i) {
EXPECT_EQ(SampledProfile::PERIODIC_COLLECTION,
uma_proto.sampled_profile(i).trigger_event());
}
}
// Checks that the pending profiles above the total cap are dropped therefore
// not encoded in the session data.
TEST_F(CallStackProfileMetricsProviderTest,
......
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