Skip to content

Commit

Permalink
Record field-trial information in stability file for crash analysis.
Browse files Browse the repository at this point in the history
BUG=620813

Review-Url: https://codereview.chromium.org/2666653002
Cr-Commit-Position: refs/heads/master@{#449013}
  • Loading branch information
bcwhite authored and Commit bot committed Feb 8, 2017
1 parent 013e6c4 commit 33d92f4
Show file tree
Hide file tree
Showing 7 changed files with 77 additions and 15 deletions.
2 changes: 1 addition & 1 deletion base/debug/activity_analyzer_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ TEST_F(ActivityAnalyzerTest, GlobalUserDataTest) {
GlobalActivityAnalyzer global_analyzer(MakeUnique<PersistentMemoryAllocator>(
const_cast<void*>(allocator->data()), allocator->size(), 0, 0, "", true));

ActivityUserData& global_data = GlobalActivityTracker::Get()->user_data();
ActivityUserData& global_data = GlobalActivityTracker::Get()->global_data();
global_data.Set("raw", "foo", 3);
global_data.SetString("string", "bar");
global_data.SetChar("char", '9');
Expand Down
34 changes: 29 additions & 5 deletions base/debug/activity_tracker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ const int kMinStackDepth = 2;

// The amount of memory set aside for holding arbitrary user data (key/value
// pairs) globally or associated with ActivityData entries.
const size_t kUserDataSize = 1024; // bytes
const size_t kGlobalDataSize = 4096; // bytes
const size_t kUserDataSize = 1 << 10; // 1 KiB
const size_t kGlobalDataSize = 16 << 10; // 16 KiB
const size_t kMaxUserDataNameLength =
static_cast<size_t>(std::numeric_limits<uint8_t>::max());

Expand Down Expand Up @@ -288,7 +288,6 @@ void ActivityUserData::Set(StringPiece name,
ValueType type,
const void* memory,
size_t size) {
DCHECK(thread_checker_.CalledOnValidThread());
DCHECK_GE(std::numeric_limits<uint8_t>::max(), name.length());
size = std::min(std::numeric_limits<uint16_t>::max() - (kMemoryAlignment - 1),
size);
Expand Down Expand Up @@ -1056,6 +1055,19 @@ ActivityUserData& GlobalActivityTracker::ScopedThreadActivity::user_data() {
return *user_data_;
}

GlobalActivityTracker::GlobalUserData::GlobalUserData(void* memory, size_t size)
: ActivityUserData(memory, size) {}

GlobalActivityTracker::GlobalUserData::~GlobalUserData() {}

void GlobalActivityTracker::GlobalUserData::Set(StringPiece name,
ValueType type,
const void* memory,
size_t size) {
AutoLock lock(data_lock_);
ActivityUserData::Set(name, type, memory, size);
}

GlobalActivityTracker::ManagedActivityTracker::ManagedActivityTracker(
PersistentMemoryAllocator::Reference mem_reference,
void* base,
Expand Down Expand Up @@ -1216,6 +1228,12 @@ void GlobalActivityTracker::RecordModuleInfo(const ModuleInfo& info) {
modules_.insert(std::make_pair(info.file, record));
}

void GlobalActivityTracker::RecordFieldTrial(const std::string& trial_name,
StringPiece group_name) {
const std::string key = std::string("FieldTrial.") + trial_name;
global_data_.SetString(key, group_name);
}

GlobalActivityTracker::GlobalActivityTracker(
std::unique_ptr<PersistentMemoryAllocator> allocator,
int stack_depth)
Expand All @@ -1235,7 +1253,7 @@ GlobalActivityTracker::GlobalActivityTracker(
kUserDataSize,
kCachedUserDataMemories,
/*make_iterable=*/false),
user_data_(
global_data_(
allocator_->GetAsArray<char>(
allocator_->Allocate(kGlobalDataSize, kTypeIdGlobalDataRecord),
kTypeIdGlobalDataRecord,
Expand All @@ -1251,7 +1269,13 @@ GlobalActivityTracker::GlobalActivityTracker(

// The global records must be iterable in order to be found by an analyzer.
allocator_->MakeIterable(allocator_->GetAsReference(
user_data_.GetBaseAddress(), kTypeIdGlobalDataRecord));
global_data_.GetBaseAddress(), kTypeIdGlobalDataRecord));

// Fetch and record all activated field trials.
FieldTrial::ActiveGroups active_groups;
FieldTrialList::GetActiveFieldTrialGroups(&active_groups);
for (auto& group : active_groups)
RecordFieldTrial(group.trial_name, group.group_name);
}

GlobalActivityTracker::~GlobalActivityTracker() {
Expand Down
40 changes: 34 additions & 6 deletions base/debug/activity_tracker.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include "base/gtest_prod_util.h"
#include "base/location.h"
#include "base/metrics/persistent_memory_allocator.h"
#include "base/strings/string_piece.h"
#include "base/strings/utf_string_conversions.h"
#include "base/threading/platform_thread.h"
#include "base/threading/thread_checker.h"
Expand Down Expand Up @@ -342,7 +343,7 @@ class BASE_EXPORT ActivityUserData {
using Snapshot = std::map<std::string, TypedValue>;

ActivityUserData(void* memory, size_t size);
~ActivityUserData();
virtual ~ActivityUserData();

// Gets the unique ID number for this user data. If this changes then the
// contents have been overwritten by another thread. The return value is
Expand Down Expand Up @@ -404,6 +405,12 @@ class BASE_EXPORT ActivityUserData {
// Gets the base memory address used for storing data.
const void* GetBaseAddress();

protected:
virtual void Set(StringPiece name,
ValueType type,
const void* memory,
size_t size);

private:
FRIEND_TEST_ALL_PREFIXES(ActivityTrackerTest, UserDataTest);

Expand Down Expand Up @@ -437,7 +444,6 @@ class BASE_EXPORT ActivityUserData {
size_t extent; // The total storage of the value,
}; // typically rounded up for alignment.

void Set(StringPiece name, ValueType type, const void* memory, size_t size);
void SetReference(StringPiece name,
ValueType type,
const void* memory,
Expand All @@ -461,8 +467,6 @@ class BASE_EXPORT ActivityUserData {
// A pointer to the unique ID for this instance.
std::atomic<uint32_t>* const id_;

base::ThreadChecker thread_checker_;

// This ID is used to create unique indentifiers for user data so that it's
// possible to tell if the information has been overwritten.
static StaticAtomicSequenceNumber next_id_;
Expand Down Expand Up @@ -784,8 +788,13 @@ class BASE_EXPORT GlobalActivityTracker {
// even with the same information.
void RecordModuleInfo(const ModuleInfo& info);

// Record field trial information. This call is thread-safe. In addition to
// this, construction of a GlobalActivityTracker will cause all existing
// active field trials to be fetched and recorded.
void RecordFieldTrial(const std::string& trial_name, StringPiece group_name);

// Accesses the global data record for storing arbitrary key/value pairs.
ActivityUserData& user_data() { return user_data_; }
ActivityUserData& global_data() { return global_data_; }

private:
friend class GlobalActivityAnalyzer;
Expand All @@ -800,6 +809,25 @@ class BASE_EXPORT GlobalActivityTracker {
kCachedUserDataMemories = 10,
};

// A wrapper around ActivityUserData that is thread-safe and thus can be used
// in the global scope without the requirement of being called from only one
// thread.
class GlobalUserData : public ActivityUserData {
public:
GlobalUserData(void* memory, size_t size);
~GlobalUserData() override;

private:
void Set(StringPiece name,
ValueType type,
const void* memory,
size_t size) override;

Lock data_lock_;

DISALLOW_COPY_AND_ASSIGN(GlobalUserData);
};

// State of a module as stored in persistent memory. This supports a single
// loading of a module only. If modules are loaded multiple times at
// different addresses, only the last will be recorded and an unload will
Expand Down Expand Up @@ -904,7 +932,7 @@ class BASE_EXPORT GlobalActivityTracker {

// An object for holding global arbitrary key value pairs. Values must always
// be written from the main UI thread.
ActivityUserData user_data_;
GlobalUserData global_data_;

// A map of global module information, keyed by module path.
std::map<const std::string, ModuleInfoRecord*> modules_;
Expand Down
10 changes: 10 additions & 0 deletions base/metrics/field_trial.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "base/base_switches.h"
#include "base/build_time.h"
#include "base/command_line.h"
#include "base/debug/activity_tracker.h"
#include "base/logging.h"
#include "base/metrics/field_trial_param_associator.h"
#include "base/process/memory.h"
Expand Down Expand Up @@ -982,6 +983,15 @@ void FieldTrialList::NotifyFieldTrialGroupSelection(FieldTrial* field_trial) {
ActivateFieldTrialEntryWhileLocked(field_trial);
}

// Recording for stability debugging has to be done inline as a task posted
// to an observer may not get executed before a crash.
base::debug::GlobalActivityTracker* tracker =
base::debug::GlobalActivityTracker::Get();
if (tracker) {
tracker->RecordFieldTrial(field_trial->trial_name(),
field_trial->group_name_internal());
}

global_->observer_list_->Notify(
FROM_HERE, &FieldTrialList::Observer::OnFieldTrialGroupFinalized,
field_trial->trial_name(), field_trial->group_name_internal());
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/chrome_browser_field_trials_desktop.cc
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ void SetupStabilityDebugging() {
&version_number, &special_build,
&channel_name);

base::debug::ActivityUserData& global_data = global_tracker->user_data();
base::debug::ActivityUserData& global_data = global_tracker->global_data();
global_data.SetString(browser_watcher::kStabilityProduct, product_name);
global_data.SetString(browser_watcher::kStabilityVersion, version_number);
global_data.SetString(browser_watcher::kStabilityChannel, channel_name);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -567,7 +567,7 @@ TEST_F(PostmortemReportCollectorCollectionFromGlobalTrackerTest,
// Record some global user data.
GlobalActivityTracker::CreateWithFile(debug_file_path(), kMemorySize, 0ULL,
"", 3);
ActivityUserData& global_data = GlobalActivityTracker::Get()->user_data();
ActivityUserData& global_data = GlobalActivityTracker::Get()->global_data();
global_data.Set("raw", "foo", 3);
global_data.SetString("string", "bar");
global_data.SetChar("char", '9');
Expand Down
2 changes: 1 addition & 1 deletion components/browser_watcher/stability_debugging.cc
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ void SetStabilityDataInt(base::StringPiece name, int64_t value) {
if (!global_tracker)
return; // Activity tracking isn't enabled.

global_tracker->user_data().SetInt(name, value);
global_tracker->global_data().SetInt(name, value);
}

} // namespace browser_watcher

0 comments on commit 33d92f4

Please sign in to comment.