Skip to content

Commit

Permalink
Enable HistogramName interning
Browse files Browse the repository at this point in the history
This change depends on
https://android-review.googlesource.com/c/platform/external/perfetto/+/1438933

Bug: 1119834
Change-Id: I55bc44de601479704fd35c2f12ccad2459a61fb5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2431566
Reviewed-by: Sami Kyöstilä <skyostil@chromium.org>
Reviewed-by: Mikhail Khokhlov <khokhlov@google.com>
Commit-Queue: Julia Semavina <nuwanda@google.com>
Cr-Commit-Position: refs/heads/master@{#813810}
  • Loading branch information
nuwanda authored and Commit Bot committed Oct 5, 2020
1 parent 79ffa15 commit 97c6f39
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 7 deletions.
22 changes: 21 additions & 1 deletion services/tracing/public/cpp/perfetto/trace_event_data_source.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
#include "third_party/perfetto/include/perfetto/ext/tracing/core/shared_memory_arbiter.h"
#include "third_party/perfetto/include/perfetto/ext/tracing/core/trace_writer.h"
#include "third_party/perfetto/include/perfetto/tracing/track.h"
#include "third_party/perfetto/include/perfetto/tracing/track_event_interned_data_index.h"
#include "third_party/perfetto/protos/perfetto/trace/chrome/chrome_metadata.pbzero.h"
#include "third_party/perfetto/protos/perfetto/trace/chrome/chrome_trace_event.pbzero.h"
#include "third_party/perfetto/protos/perfetto/trace/trace_packet.pbzero.h"
Expand Down Expand Up @@ -1041,6 +1042,24 @@ void TraceEventDataSource::FlushCurrentThread() {
}
}

namespace {

struct InternedHistogramName
: public perfetto::TrackEventInternedDataIndex<
InternedHistogramName,
perfetto::protos::pbzero::InternedData::kHistogramNamesFieldNumber,
std::string> {
static void Add(perfetto::protos::pbzero::InternedData* interned_data,
size_t iid,
const std::string& histogram_name) {
auto* msg = interned_data->add_histogram_names();
msg->set_iid(iid);
msg->set_name(histogram_name);
}
};

} // namespace

// static
void TraceEventDataSource::OnMetricsSampleCallback(
const char* histogram_name,
Expand All @@ -1056,7 +1075,8 @@ void TraceEventDataSource::OnMetricsSampleCallback(
new_sample->set_name_hash(name_hash);
new_sample->set_sample(sample);
if (!privacy_filtering_enabled) {
new_sample->set_name(histogram_name);
size_t iid = InternedHistogramName::Get(&ctx, histogram_name);
new_sample->set_name_iid(iid);
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1917,9 +1917,11 @@ TEST_F(TraceEventDataSourceTest, HistogramSampleTraceConfigEmpty) {
{{1u, TRACE_DISABLED_BY_DEFAULT("histogram_samples")}});
ExpectEventNames(e_packet, {{1u, "HistogramSample"}});
ASSERT_TRUE(e_packet->track_event().has_chrome_histogram_sample());
EXPECT_EQ(e_packet->track_event().chrome_histogram_sample().name_hash(),
base::HashMetricName("Foo.Bar"));
EXPECT_EQ(e_packet->track_event().chrome_histogram_sample().sample(), 1u);
ASSERT_TRUE(e_packet->has_interned_data());
EXPECT_EQ(e_packet->track_event().chrome_histogram_sample().name_iid(),
e_packet->interned_data().histogram_names()[0].iid());
EXPECT_EQ(e_packet->interned_data().histogram_names()[0].name(), "Foo.Bar");
}

TEST_F(TraceEventDataSourceTest, HistogramSampleTraceConfigNotEmpty) {
Expand All @@ -1946,9 +1948,11 @@ TEST_F(TraceEventDataSourceTest, HistogramSampleTraceConfigNotEmpty) {
ASSERT_TRUE(e_packet->track_event().has_chrome_histogram_sample());
EXPECT_EQ(e_packet->track_event().chrome_histogram_sample().name_hash(),
base::HashMetricName("Foo1.Bar1"));
EXPECT_EQ(e_packet->track_event().chrome_histogram_sample().name(),
"Foo1.Bar1");
EXPECT_EQ(e_packet->track_event().chrome_histogram_sample().sample(), 1u);
ASSERT_TRUE(e_packet->has_interned_data());
EXPECT_EQ(e_packet->track_event().chrome_histogram_sample().name_iid(),
e_packet->interned_data().histogram_names()[0].iid());
EXPECT_EQ(e_packet->interned_data().histogram_names()[0].name(), "Foo1.Bar1");

e_packet = producer_client()->GetFinalizedPacket(packet_index++);

Expand All @@ -1957,9 +1961,11 @@ TEST_F(TraceEventDataSourceTest, HistogramSampleTraceConfigNotEmpty) {
ASSERT_TRUE(e_packet->track_event().has_chrome_histogram_sample());
EXPECT_EQ(e_packet->track_event().chrome_histogram_sample().name_hash(),
base::HashMetricName("Foo3.Bar3"));
EXPECT_EQ(e_packet->track_event().chrome_histogram_sample().name(),
"Foo3.Bar3");
EXPECT_EQ(e_packet->track_event().chrome_histogram_sample().sample(), 1u);
ASSERT_TRUE(e_packet->has_interned_data());
EXPECT_EQ(e_packet->track_event().chrome_histogram_sample().name_iid(),
e_packet->interned_data().histogram_names()[0].iid());
EXPECT_EQ(e_packet->interned_data().histogram_names()[0].name(), "Foo3.Bar3");

EXPECT_EQ(packet_index, producer_client()->GetFinalizedPacketCount());
}
Expand Down

0 comments on commit 97c6f39

Please sign in to comment.