Index: components/metrics/persistent_system_profile.cc |
diff --git a/components/metrics/persistent_system_profile.cc b/components/metrics/persistent_system_profile.cc |
index 88b2a76bb257365096a06b86d0113837d1597007..4063bf3f2fb1be3a6f267db3fe0558e100ee3fce 100644 |
--- a/components/metrics/persistent_system_profile.cc |
+++ b/components/metrics/persistent_system_profile.cc |
@@ -4,10 +4,14 @@ |
#include "components/metrics/persistent_system_profile.h" |
+#include <set> |
+ |
#include "base/atomicops.h" |
#include "base/memory/singleton.h" |
#include "base/metrics/persistent_memory_allocator.h" |
+#include "base/pickle.h" |
#include "base/stl_util.h" |
+#include "components/variations/active_field_trials.h" |
namespace metrics { |
@@ -82,9 +86,8 @@ void PersistentSystemProfile::RecordAllocator::Reset() { |
end_offset_ = 0; |
} |
-bool PersistentSystemProfile::RecordAllocator::Write( |
- RecordType type, |
- const std::string& record) { |
+bool PersistentSystemProfile::RecordAllocator::Write(RecordType type, |
+ base::StringPiece record) { |
const char* data = record.data(); |
size_t remaining_size = record.size(); |
@@ -198,8 +201,9 @@ bool PersistentSystemProfile::RecordAllocator::WriteData( |
reinterpret_cast<base::subtle::Atomic32*>(block + end_offset_), 0); |
} |
memcpy(block + offset + sizeof(header), *data, write_size); |
- base::subtle::Release_Store(reinterpret_cast<base::subtle::Atomic32*>(block), |
- header.as_atomic); |
+ base::subtle::Release_Store( |
+ reinterpret_cast<base::subtle::Atomic32*>(block + offset), |
Alexei Svitkine (slow)
2017/06/28 14:59:00
Wait, so this is now writing the header at block+o
bcwhite
2017/06/28 16:43:30
It was an error where each additional record had i
|
+ header.as_atomic); |
// Account for what was stored and prepare for follow-on records with any |
// remaining data. |
@@ -239,15 +243,14 @@ bool PersistentSystemProfile::RecordAllocator::ReadData( |
return false; |
} |
size_t read_size = header.as_parts.amount; |
- if (read_size < sizeof(header) || |
- end_offset_ + sizeof(header) + read_size > alloc_size_) { |
+ if (end_offset_ + sizeof(header) + read_size > alloc_size_) { |
NOTREACHED(); // Invalid header amount. |
*type = kUnusedSpace; |
return true; // Don't try again. |
} |
// Append the record data to the output string. |
- record->append(block + sizeof(header), read_size); |
+ record->append(block + end_offset_ + sizeof(header), read_size); |
end_offset_ += CalculateRecordSize(read_size); |
DCHECK_GE(alloc_size_, end_offset_); |
@@ -292,8 +295,10 @@ void PersistentSystemProfile::SetSystemProfile( |
// Don't overwrite a complete profile with an incomplete one. |
if (!complete && allocator.has_complete_profile()) |
continue; |
- // A full system profile always starts fresh. |
- allocator.Reset(); |
+ // A full system profile always starts fresh. Incomplete keeps existing |
+ // records for merging. |
+ if (complete) |
+ allocator.Reset(); |
// Write out the serialized profile. |
allocator.Write(kSystemProfileProto, serialized_profile); |
// Indicate if this is a complete profile. |
@@ -319,6 +324,22 @@ void PersistentSystemProfile::SetSystemProfile( |
SetSystemProfile(serialized_profile, complete); |
} |
+void PersistentSystemProfile::SetFieldTrial(base::StringPiece trial, |
+ base::StringPiece group) { |
+ DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); |
+ |
+ if (trial.empty() || group.empty()) |
Alexei Svitkine (slow)
2017/06/28 14:59:00
This shouldn't be possible. Can these be DCHECKs i
bcwhite
2017/06/28 16:43:30
Done.
|
+ return; |
+ |
+ base::Pickle pickler; |
+ if (!pickler.WriteString(trial) || !pickler.WriteString(group)) |
+ return; |
+ |
+ WriteToAll(kFieldTrialInfo, |
+ base::StringPiece(static_cast<const char*>(pickler.data()), |
+ pickler.size())); |
+} |
+ |
// static |
bool PersistentSystemProfile::HasSystemProfile( |
const base::PersistentMemoryAllocator& memory_allocator) { |
@@ -334,12 +355,73 @@ bool PersistentSystemProfile::GetSystemProfile( |
RecordType type; |
std::string record; |
- if (!records.Read(&type, &record)) |
- return false; |
- if (type != kSystemProfileProto) |
+ do { |
+ if (!records.Read(&type, &record)) |
+ return false; |
+ } while (type != kSystemProfileProto); |
+ |
+ if (!system_profile->ParseFromString(record)) |
return false; |
- return system_profile->ParseFromString(record); |
+ MergeUpdateRecords(memory_allocator, system_profile); |
+ return true; |
+} |
+ |
+// static |
+void PersistentSystemProfile::MergeUpdateRecords( |
+ const base::PersistentMemoryAllocator& memory_allocator, |
+ SystemProfileProto* system_profile) { |
+ const RecordAllocator records(&memory_allocator); |
+ |
+ RecordType type; |
+ std::string record; |
+ std::set<uint32_t> known_field_trial_ids; |
+ |
+ while (records.Read(&type, &record)) { |
Alexei Svitkine (slow)
2017/06/28 14:59:01
Maybe add a comment about why this can't be done i
bcwhite
2017/06/28 16:43:31
Mostly it just made a nice, independent block of c
|
+ switch (type) { |
+ case kUnusedSpace: |
+ // These should never be returned. |
+ NOTREACHED(); |
+ break; |
+ |
+ case kSystemProfileProto: |
+ // Profile was passed in; ignore this one. |
+ break; |
+ |
+ case kFieldTrialInfo: { |
+ // Get the set of known trial IDs so duplicates don't get added. |
+ if (known_field_trial_ids.empty()) { |
+ for (int i = 0; i < system_profile->field_trial_size(); ++i) { |
+ const SystemProfileProto::FieldTrial& field_trial = |
+ system_profile->field_trial(i); |
Alexei Svitkine (slow)
2017/06/28 14:59:00
Nit: Inline this into the line below.
bcwhite
2017/06/28 16:43:31
Done.
|
+ known_field_trial_ids.insert(field_trial.name_id()); |
+ } |
+ } |
+ |
+ base::Pickle pickler(record.data(), record.size()); |
+ base::PickleIterator iter(pickler); |
+ base::StringPiece trial; |
+ base::StringPiece group; |
+ if (iter.ReadStringPiece(&trial) && iter.ReadStringPiece(&group)) { |
+ variations::ActiveGroupId field_ids = |
+ variations::MakeActiveGroupId(trial, group); |
Alexei Svitkine (slow)
2017/06/28 14:59:00
Hmm, maybe we can just store ActiveGroupId in the
bcwhite
2017/06/28 16:43:31
Yes. However, that means calculating the two SHA1
|
+ if (!base::ContainsKey(known_field_trial_ids, field_ids.name)) { |
+ SystemProfileProto::FieldTrial* field_trial = |
+ system_profile->add_field_trial(); |
+ field_trial->set_name_id(field_ids.name); |
+ field_trial->set_group_id(field_ids.group); |
+ known_field_trial_ids.insert(field_ids.name); |
+ } |
+ } |
+ } break; |
+ } |
+ } |
+} |
+ |
+void PersistentSystemProfile::WriteToAll(RecordType type, |
+ base::StringPiece record) { |
+ for (auto& allocator : allocators_) |
+ allocator.Write(type, record); |
} |
GlobalPersistentSystemProfile* GlobalPersistentSystemProfile::GetInstance() { |