From dbf709c43e3df9b29a62ed491f5878f608a1634a Mon Sep 17 00:00:00 2001 From: Lalit Maganti Date: Mon, 25 Nov 2024 00:00:00 +0000 Subject: [PATCH] tp: switch from ucpu -> cpu on track tables ucpu on track does not makes sense because machine_id is also present. It makes reasoning about things more difficult than it needs to be for no reason. We might bring this back on the track tables in the future once more track refactorings are done. Change-Id: Ice4f04327b16d6a9ac46a0ab33ec5d0400163b44 --- .../importers/common/cpu_tracker.cc | 9 +-- .../importers/common/cpu_tracker.h | 11 +++- .../importers/common/track_tracker.cc | 21 ++++--- .../importers/common/track_tracker.h | 9 ++- src/trace_processor/importers/common/tracks.h | 6 +- .../importers/ftrace/ftrace_parser.cc | 7 +-- .../importers/ftrace/ftrace_parser.h | 1 - .../proto/proto_trace_parser_impl.cc | 2 +- .../proto/proto_trace_parser_impl_unittest.cc | 6 +- .../importers/proto/system_probes_parser.cc | 6 +- .../stdlib/linux/cpu/frequency.sql | 10 ++-- .../stdlib/linux/cpu/idle_time_in_state.sql | 17 +++--- .../stdlib/prelude/after_eof/tables_views.sql | 22 ++----- src/trace_processor/storage/trace_storage.h | 6 +- src/trace_processor/tables/track_tables.py | 10 ++-- src/trace_processor/types/variadic.h | 59 +++++++++---------- .../parser/parsing/tests_sys_stats.py | 2 +- .../diff_tests/tables/tests.py | 8 +-- .../diff_tests/tables/tests_counters.py | 3 +- .../suspend_resume_details.ts | 52 +++++++--------- 20 files changed, 126 insertions(+), 141 deletions(-) diff --git a/src/trace_processor/importers/common/cpu_tracker.cc b/src/trace_processor/importers/common/cpu_tracker.cc index b679eae768..4b6c55bca1 100644 --- a/src/trace_processor/importers/common/cpu_tracker.cc +++ b/src/trace_processor/importers/common/cpu_tracker.cc @@ -16,10 +16,13 @@ #include "src/trace_processor/importers/common/cpu_tracker.h" -#include "perfetto/base/logging.h" -#include "perfetto/public/compiler.h" +#include +#include +#include "perfetto/base/logging.h" +#include "perfetto/ext/base/string_view.h" #include "src/trace_processor/importers/common/machine_tracker.h" +#include "src/trace_processor/tables/metadata_tables_py.h" namespace perfetto::trace_processor { @@ -39,8 +42,6 @@ CpuTracker::CpuTracker(TraceProcessorContext* context) : context_(context) { } } -CpuTracker::~CpuTracker() = default; - tables::CpuTable::Id CpuTracker::SetCpuInfo(uint32_t cpu, base::StringView processor, uint32_t cluster_id, diff --git a/src/trace_processor/importers/common/cpu_tracker.h b/src/trace_processor/importers/common/cpu_tracker.h index 466fa92830..cbbd3ece71 100644 --- a/src/trace_processor/importers/common/cpu_tracker.h +++ b/src/trace_processor/importers/common/cpu_tracker.h @@ -43,7 +43,6 @@ class CpuTracker { static constexpr uint32_t kMaxCpusPerMachine = 4096; explicit CpuTracker(TraceProcessorContext*); - ~CpuTracker(); tables::CpuTable::Id GetOrCreateCpu(uint32_t cpu) { // CPU core number is in the range of 0..kMaxCpusPerMachine-1. @@ -59,6 +58,16 @@ class CpuTracker { return tables::CpuTable::Id(ucpu); } + void MarkCpuValid(uint32_t cpu) { + PERFETTO_CHECK(cpu < kMaxCpusPerMachine); + if (PERFETTO_LIKELY(cpu_ids_[cpu])) { + return; + } + auto ucpu = ucpu_offset_ + cpu; + auto& cpu_table = *context_->storage->mutable_cpu_table(); + cpu_table[ucpu].set_cpu(cpu); + } + // Sets or updates the information for the specified CPU in the CpuTable. tables::CpuTable::Id SetCpuInfo(uint32_t cpu, base::StringView processor, diff --git a/src/trace_processor/importers/common/track_tracker.cc b/src/trace_processor/importers/common/track_tracker.cc index 79bca1156a..3270ef2932 100644 --- a/src/trace_processor/importers/common/track_tracker.cc +++ b/src/trace_processor/importers/common/track_tracker.cc @@ -111,7 +111,7 @@ TrackTracker::TrackTracker(TraceProcessorContext* context) chrome_source_(context->storage->InternString("chrome")), utid_id_(context->storage->InternString("utid")), upid_id_(context->storage->InternString("upid")), - ucpu_id_(context->storage->InternString("ucpu")), + cpu_id_(context->storage->InternString("cpu")), uid_id_(context->storage->InternString("uid")), gpu_id_(context->storage->InternString("gpu")), name_id_(context->storage->InternString("name")), @@ -339,16 +339,16 @@ TrackId TrackTracker::LegacyInternThreadCounterTrack(StringId name, TrackId TrackTracker::InternCpuTrack(tracks::TrackClassification classification, uint32_t cpu, const TrackName& name) { - auto ucpu = context_->cpu_tracker->GetOrCreateCpu(cpu); - Dimensions dims_id = SingleDimension(ucpu_id_, Variadic::Integer(ucpu.value)); + MarkCpuValid(cpu); + Dimensions dims_id = SingleDimension(cpu_id_, Variadic::Integer(cpu)); auto* it = tracks_.Find({classification, dims_id}); if (it) { return *it; } tables::CpuTrackTable::Row row(StringIdFromTrackName(classification, name)); - row.ucpu = ucpu; + row.cpu = cpu; row.machine_id = context_->machine_id(); row.classification = context_->storage->InternString(tracks::ToString(classification)); @@ -435,14 +435,13 @@ TrackId TrackTracker::InternCpuCounterTrack( tracks::TrackClassification classification, uint32_t cpu, const TrackName& name) { - auto ucpu = context_->cpu_tracker->GetOrCreateCpu(cpu); StringId name_id = StringIdFromTrackName(classification, name); TrackMapKey key; key.classification = classification; DimensionsBuilder dims_builder = CreateDimensionsBuilder(); - dims_builder.AppendUcpu(ucpu); + dims_builder.AppendCpu(cpu); dims_builder.AppendName(name_id); key.dimensions = std::move(dims_builder).Build(); @@ -452,7 +451,7 @@ TrackId TrackTracker::InternCpuCounterTrack( } tables::CpuCounterTrackTable::Row row(name_id); - row.ucpu = ucpu; + row.cpu = cpu; row.machine_id = context_->machine_id(); row.classification = context_->storage->InternString(tracks::ToString(classification)); @@ -512,9 +511,8 @@ TrackId TrackTracker::LegacyCreatePerfCounterTrack( tables::PerfSessionTable::Id perf_session_id, uint32_t cpu, bool is_timebase) { - auto ucpu = context_->cpu_tracker->GetOrCreateCpu(cpu); DimensionsBuilder dims_builder = CreateDimensionsBuilder(); - dims_builder.AppendUcpu(ucpu); + dims_builder.AppendCpu(cpu); dims_builder.AppendDimension( context_->storage->InternString("perf_session_id"), Variadic::Integer(perf_session_id.value)); @@ -621,4 +619,9 @@ StringId TrackTracker::StringIdFromTrackName( PERFETTO_FATAL("For GCC"); } + +void TrackTracker::MarkCpuValid(uint32_t cpu) { + context_->cpu_tracker->MarkCpuValid(cpu); +} + } // namespace perfetto::trace_processor diff --git a/src/trace_processor/importers/common/track_tracker.h b/src/trace_processor/importers/common/track_tracker.h index 3736272da9..7c33095b7a 100644 --- a/src/trace_processor/importers/common/track_tracker.h +++ b/src/trace_processor/importers/common/track_tracker.h @@ -63,8 +63,9 @@ class TrackTracker { explicit DimensionsBuilder(TrackTracker* tt) : tt_(tt) {} // Append CPU dimension of a track. - void AppendUcpu(tables::CpuTable::Id ucpu) { - AppendDimension(tt_->ucpu_id_, Variadic::Integer(ucpu.value)); + void AppendCpu(uint32_t cpu) { + tt_->MarkCpuValid(cpu); + AppendDimension(tt_->cpu_id_, Variadic::Integer(cpu)); } // Append Utid (unique tid) dimension of a track. @@ -329,6 +330,8 @@ class TrackTracker { StringId StringIdFromTrackName(tracks::TrackClassification classification, const TrackTracker::TrackName& name); + void MarkCpuValid(uint32_t cpu); + Dimensions SingleDimension(StringId key, const Variadic& val) { std::array args{GlobalArgsTracker::CompactArg{key, key, val}}; return Dimensions{ @@ -352,7 +355,7 @@ class TrackTracker { const StringId utid_id_; const StringId upid_id_; - const StringId ucpu_id_; + const StringId cpu_id_; const StringId uid_id_; const StringId gpu_id_; const StringId name_id_; diff --git a/src/trace_processor/importers/common/tracks.h b/src/trace_processor/importers/common/tracks.h index c34a038276..16d07057f7 100644 --- a/src/trace_processor/importers/common/tracks.h +++ b/src/trace_processor/importers/common/tracks.h @@ -70,12 +70,16 @@ enum TrackClassification : size_t { PERFETTO_TP_TRACKS(PERFETTO_TP_TRACKS_CLASSIFICATION_ENUM) }; +namespace internal { + #define PERFETTO_TP_TRACKS_CLASSIFICATION_STR(name) #name, constexpr std::array kTrackClassificationStr{ PERFETTO_TP_TRACKS(PERFETTO_TP_TRACKS_CLASSIFICATION_STR)}; +} // namespace internal + constexpr const char* ToString(TrackClassification c) { - return kTrackClassificationStr[c]; + return internal::kTrackClassificationStr[c]; } } // namespace perfetto::trace_processor::tracks diff --git a/src/trace_processor/importers/ftrace/ftrace_parser.cc b/src/trace_processor/importers/ftrace/ftrace_parser.cc index d35f963ddb..bc8391b0ba 100644 --- a/src/trace_processor/importers/ftrace/ftrace_parser.cc +++ b/src/trace_processor/importers/ftrace/ftrace_parser.cc @@ -344,7 +344,6 @@ FtraceParser::FtraceParser(TraceProcessorContext* context) sched_wakeup_name_id_(context->storage->InternString("sched_wakeup")), sched_waking_name_id_(context->storage->InternString("sched_waking")), cpu_id_(context->storage->InternString("cpu")), - ucpu_id_(context->storage->InternString("ucpu")), linux_device_name_id_( context->storage->InternString("linux_device_name")), suspend_resume_name_id_( @@ -3518,8 +3517,7 @@ void FtraceParser::ParseSuspendResume(int64_t timestamp, context_->process_tracker->GetOrCreateThread(tid))); inserter->AddArg(suspend_resume_event_type_arg_name_, Variadic::String(suspend_resume_main_event_id_)); - auto ucpu = context_->cpu_tracker->GetOrCreateCpu(cpu); - inserter->AddArg(ucpu_id_, Variadic::UnsignedInteger(ucpu.value)); + inserter->AddArg(cpu_id_, Variadic::UnsignedInteger(cpu)); // These fields are set to null as this is not a device PM callback event. inserter->AddArg(suspend_resume_device_arg_name_, @@ -3777,8 +3775,7 @@ void FtraceParser::ParseDevicePmCallbackStart(int64_t ts, context_->process_tracker->GetOrCreateThread(tid))); inserter->AddArg(suspend_resume_event_type_arg_name_, Variadic::String(suspend_resume_device_pm_event_id_)); - auto ucpu = context_->cpu_tracker->GetOrCreateCpu(cpu); - inserter->AddArg(ucpu_id_, Variadic::UnsignedInteger(ucpu.value)); + inserter->AddArg(cpu_id_, Variadic::UnsignedInteger(cpu)); inserter->AddArg( suspend_resume_device_arg_name_, Variadic::String(context_->storage->InternString(device_name.c_str()))); diff --git a/src/trace_processor/importers/ftrace/ftrace_parser.h b/src/trace_processor/importers/ftrace/ftrace_parser.h index 9932e83740..bb6aa41f3b 100644 --- a/src/trace_processor/importers/ftrace/ftrace_parser.h +++ b/src/trace_processor/importers/ftrace/ftrace_parser.h @@ -335,7 +335,6 @@ class FtraceParser { const StringId sched_wakeup_name_id_; const StringId sched_waking_name_id_; const StringId cpu_id_; - const StringId ucpu_id_; const StringId linux_device_name_id_; const StringId suspend_resume_name_id_; const StringId suspend_resume_minimal_name_id_; diff --git a/src/trace_processor/importers/proto/proto_trace_parser_impl.cc b/src/trace_processor/importers/proto/proto_trace_parser_impl.cc index f4b6fca9b3..a409b9074a 100644 --- a/src/trace_processor/importers/proto/proto_trace_parser_impl.cc +++ b/src/trace_processor/importers/proto/proto_trace_parser_impl.cc @@ -176,7 +176,7 @@ void ProtoTraceParserImpl::ParseChromeEvents(int64_t ts, ConstBytes blob) { // table to JSON export. for (auto it = bundle.metadata(); it; ++it) { protos::pbzero::ChromeMetadata::Decoder metadata(*it); - Variadic value; + Variadic value = Variadic::Null(); if (metadata.has_string_value()) { value = Variadic::String(storage->InternString(metadata.string_value())); diff --git a/src/trace_processor/importers/proto/proto_trace_parser_impl_unittest.cc b/src/trace_processor/importers/proto/proto_trace_parser_impl_unittest.cc index 229a65888c..88abd3c542 100644 --- a/src/trace_processor/importers/proto/proto_trace_parser_impl_unittest.cc +++ b/src/trace_processor/importers/proto/proto_trace_parser_impl_unittest.cc @@ -649,7 +649,7 @@ TEST_F(ProtoTraceParserTest, LoadCpuFreq) { Tokenize(); context_.sorter->ExtractEventsForced(); - EXPECT_EQ(context_.storage->cpu_counter_track_table()[0].ucpu().value, 10u); + EXPECT_EQ(context_.storage->cpu_counter_track_table()[0].cpu(), 10u); } TEST_F(ProtoTraceParserTest, LoadCpuFreqKHz) { @@ -672,10 +672,10 @@ TEST_F(ProtoTraceParserTest, LoadCpuFreqKHz) { auto row = context_.storage->cpu_counter_track_table().FindById(TrackId(0)); EXPECT_EQ(context_.storage->GetString(row->name()), "cpufreq"); - EXPECT_EQ(row->ucpu().value, 0u); + EXPECT_EQ(row->cpu(), 0u); row = context_.storage->cpu_counter_track_table().FindById(TrackId(1)); - EXPECT_EQ(row->ucpu().value, 1u); + EXPECT_EQ(row->cpu(), 1u); } TEST_F(ProtoTraceParserTest, LoadCpuIdleStats) { diff --git a/src/trace_processor/importers/proto/system_probes_parser.cc b/src/trace_processor/importers/proto/system_probes_parser.cc index 574653aa2a..cd6b9fc910 100644 --- a/src/trace_processor/importers/proto/system_probes_parser.cc +++ b/src/trace_processor/importers/proto/system_probes_parser.cc @@ -378,14 +378,13 @@ void SystemProbesParser::ParseSysStats(int64_t ts, ConstBytes blob) { continue; } - auto ucpu = context_->cpu_tracker->GetOrCreateCpu(ct.cpu_id()); auto intern_track = [&, this](TrackTracker::LegacyCharArrayName name) -> TrackId { auto builder = context_->track_tracker->CreateDimensionsBuilder(); builder.AppendDimension( cpu_stat_counter_name_id_, Variadic::String(context_->storage->InternString(name.name))); - builder.AppendUcpu(ucpu); + builder.AppendCpu(ct.cpu_id()); return context_->track_tracker->InternCounterTrack( tracks::cpu_stat, std::move(builder).Build(), name); }; @@ -527,7 +526,6 @@ void SystemProbesParser::ParseSysStats(int64_t ts, ConstBytes blob) { void SystemProbesParser::ParseCpuIdleStats(int64_t ts, ConstBytes blob) { protos::pbzero::SysStats::CpuIdleState::Decoder cpuidle_state(blob); uint32_t cpu_id = cpuidle_state.cpu_id(); - auto ucpu = context_->cpu_tracker->GetOrCreateCpu(cpu_id); for (auto cpuidle_field = cpuidle_state.cpuidle_state_entry(); cpuidle_field; ++cpuidle_field) { protos::pbzero::SysStats::CpuIdleStateEntry::Decoder idle(*cpuidle_field); @@ -537,7 +535,7 @@ void SystemProbesParser::ParseCpuIdleStats(int64_t ts, ConstBytes blob) { dims_builder.AppendDimension( cpu_idle_state_id_, Variadic::String(context_->storage->InternString(idle.state()))); - dims_builder.AppendUcpu(ucpu); + dims_builder.AppendCpu(cpu_id); TrackId track = context_->track_tracker->InternTrack( tracks::cpu_idle_state, std::move(dims_builder).Build()); diff --git a/src/trace_processor/perfetto_sql/stdlib/linux/cpu/frequency.sql b/src/trace_processor/perfetto_sql/stdlib/linux/cpu/frequency.sql index a965b8e4c1..4d50ef9e09 100644 --- a/src/trace_processor/perfetto_sql/stdlib/linux/cpu/frequency.sql +++ b/src/trace_processor/perfetto_sql/stdlib/linux/cpu/frequency.sql @@ -40,14 +40,14 @@ SELECT count_w_dur.ts, count_w_dur.dur, cast_int!(count_w_dur.value) as freq, - cct.ucpu, + cpu.ucpu, cct.cpu -FROM -counter_leading_intervals!(( +FROM counter_leading_intervals!(( SELECT c.* FROM counter c JOIN cpu_counter_track cct ON cct.id = c.track_id AND cct.name = 'cpufreq' )) count_w_dur -JOIN cpu_counter_track cct -ON track_id = cct.id; +JOIN cpu_counter_track cct ON track_id = cct.id +JOIN cpu ON cct.machine_id IS cpu.machine_id + AND cct.cpu = cpu.cpu; diff --git a/src/trace_processor/perfetto_sql/stdlib/linux/cpu/idle_time_in_state.sql b/src/trace_processor/perfetto_sql/stdlib/linux/cpu/idle_time_in_state.sql index 192b5000d3..9e9d4b2d1a 100644 --- a/src/trace_processor/perfetto_sql/stdlib/linux/cpu/idle_time_in_state.sql +++ b/src/trace_processor/perfetto_sql/stdlib/linux/cpu/idle_time_in_state.sql @@ -41,8 +41,10 @@ idle_states AS ( SELECT c.ts, c.value, + c.track_id, + t.machine_id, EXTRACT_ARG(t.dimension_arg_set_id, 'cpu_idle_state') as state, - EXTRACT_ARG(t.dimension_arg_set_id, 'ucpu') as ucpu + EXTRACT_ARG(t.dimension_arg_set_id, 'cpu') as cpu FROM counter c JOIN track t on c.track_id = t.id WHERE t.classification = 'cpu_idle_state' @@ -51,14 +53,15 @@ residency_deltas AS ( SELECT ts, state, - ucpu, - value - (LAG(value) OVER (PARTITION BY state, ucpu ORDER BY ts)) as delta + cpu, + machine_id, + value - (LAG(value) OVER (PARTITION BY track_id ORDER BY ts)) as delta FROM idle_states ), total_residency_calc AS ( SELECT ts, - cpu.machine_id, + residency_deltas.machine_id, state AS state_name, SUM(delta) as total_residency, -- Perfetto timestamp is in nanoseconds whereas sysfs cpuidle time @@ -68,11 +71,11 @@ total_residency_calc AS ( (time_to_us(ts - LAG(ts, 1) over (PARTITION BY state ORDER BY ts))) ) as time_slice FROM residency_deltas - JOIN cpu USING (ucpu) -- The use of `IS` instead of `=` is intentional because machine_id can be -- null and we still want this join to work in that case. - JOIN cpu_counts_per_machine ON cpu.machine_id IS cpu_counts_per_machine.machine_id - GROUP BY ts, cpu.machine_id, state + JOIN cpu_counts_per_machine + ON residency_deltas.machine_id IS cpu_counts_per_machine.machine_id + GROUP BY ts, residency_deltas.machine_id, state ) SELECT ts, diff --git a/src/trace_processor/perfetto_sql/stdlib/prelude/after_eof/tables_views.sql b/src/trace_processor/perfetto_sql/stdlib/prelude/after_eof/tables_views.sql index 777e2a410f..2047c684f5 100644 --- a/src/trace_processor/perfetto_sql/stdlib/prelude/after_eof/tables_views.sql +++ b/src/trace_processor/perfetto_sql/stdlib/prelude/after_eof/tables_views.sql @@ -417,12 +417,8 @@ CREATE PERFETTO VIEW cpu_track ( source_arg_set_id LONG, -- Machine identifier, non-null for tracks on a remote machine. machine_id LONG, - -- The CPU that the track is associated with (meaningful only in single - -- machine traces). For multi-machine, join with the `cpu` table on `ucpu` to - -- get the CPU identifier of each machine. - cpu LONG, - -- The unique CPU identifier that this track is associated with. - ucpu LONG + -- The CPU that the track is associated with. + cpu LONG ) AS SELECT id, @@ -431,8 +427,7 @@ SELECT parent_id, source_arg_set_id, machine_id, - ucpu AS cpu, - ucpu + cpu FROM __intrinsic_cpu_track; @@ -457,12 +452,8 @@ CREATE PERFETTO VIEW cpu_counter_track ( unit STRING, -- The description for this track. For debugging purposes only. description STRING, - -- The CPU that the track is associated with (meaningful only in single - -- machine traces). For multi-machine, join with the `cpu` table on `ucpu` to - -- get the CPU identifier of each machine. - cpu LONG, - -- The unique CPU identifier that this track is associated with. - ucpu LONG + -- The CPU that the track is associated with. + cpu LONG ) AS SELECT id, @@ -473,7 +464,6 @@ SELECT machine_id, unit, description, - ucpu AS cpu, - ucpu + cpu FROM __intrinsic_cpu_counter_track; diff --git a/src/trace_processor/storage/trace_storage.h b/src/trace_processor/storage/trace_storage.h index 43172b5701..89d4d0b84f 100644 --- a/src/trace_processor/storage/trace_storage.h +++ b/src/trace_processor/storage/trace_storage.h @@ -953,12 +953,8 @@ class TraceStorage { Variadic GetArgValue(uint32_t row) const { auto rr = arg_table_[row]; - Variadic v; + Variadic v = Variadic::Null(); v.type = *GetVariadicTypeForId(rr.value_type()); - - // Force initialization of union to stop GCC complaining. - v.int_value = 0; - switch (v.type) { case Variadic::Type::kBool: v.bool_value = static_cast(*rr.int_value()); diff --git a/src/trace_processor/tables/track_tables.py b/src/trace_processor/tables/track_tables.py index 1ed6df8074..5dcf89544a 100644 --- a/src/trace_processor/tables/track_tables.py +++ b/src/trace_processor/tables/track_tables.py @@ -27,7 +27,7 @@ from python.generators.trace_processor_table.public import TableDoc from python.generators.trace_processor_table.public import WrappingSqlView -from src.trace_processor.tables.metadata_tables import CPU_TABLE, MACHINE_TABLE +from src.trace_processor.tables.metadata_tables import MACHINE_TABLE TRACK_TABLE = Table( python_module=__file__, @@ -138,7 +138,7 @@ class_name='CpuTrackTable', sql_name='__intrinsic_cpu_track', columns=[ - C('ucpu', CppTableId(CPU_TABLE)), + C('cpu', CppUint32()), ], wrapping_sql_view=WrappingSqlView('cpu_track'), parent=TRACK_TABLE, @@ -146,7 +146,7 @@ doc='Tracks which are associated to a single CPU', group='Tracks', columns={ - 'ucpu': 'The unique CPU identifier associated with this track.', + 'cpu': 'The CPU associated with this track.', })) GPU_TRACK_TABLE = Table( @@ -238,7 +238,7 @@ class_name='CpuCounterTrackTable', sql_name='__intrinsic_cpu_counter_track', columns=[ - C('ucpu', CppTableId(CPU_TABLE)), + C('cpu', CppUint32()), ], wrapping_sql_view=WrappingSqlView('cpu_counter_track'), parent=COUNTER_TRACK_TABLE, @@ -246,7 +246,7 @@ doc='Tracks containing counter-like events associated to a CPU.', group='Counter Tracks', columns={ - 'ucpu': 'The unique CPU identifier associated with this track.' + 'cpu': 'The unique CPU identifier associated with this track.' })) GPU_COUNTER_TRACK_TABLE = Table( diff --git a/src/trace_processor/types/variadic.h b/src/trace_processor/types/variadic.h index 8cf7cac065..2a9510aa83 100644 --- a/src/trace_processor/types/variadic.h +++ b/src/trace_processor/types/variadic.h @@ -17,10 +17,14 @@ #ifndef SRC_TRACE_PROCESSOR_TYPES_VARIADIC_H_ #define SRC_TRACE_PROCESSOR_TYPES_VARIADIC_H_ +#include +#include +#include +#include + #include "src/trace_processor/containers/string_pool.h" -namespace perfetto { -namespace trace_processor { +namespace perfetto::trace_processor { // Variadic type representing value of different possible types. struct Variadic { @@ -35,13 +39,12 @@ struct Variadic { kNull, kMaxType = kNull, }; - static constexpr const char* const kTypeNames[] = { - "int", "uint", "string", "real", "pointer", "bool", "json", "null"}; + "int", "uint", "string", "real", "pointer", "bool", "json", "null", + }; - static Variadic Integer(int64_t int_value) { - Variadic variadic; - variadic.type = Type::kInt; + static constexpr Variadic Integer(int64_t int_value) { + Variadic variadic(Type::kInt); variadic.int_value = int_value; return variadic; } @@ -50,57 +53,47 @@ struct Variadic { // SQLite for built-in SQL operators. This variadic type is used to // distinguish between int64 and uint64 for correct JSON export of TrackEvent // arguments. - static Variadic UnsignedInteger(uint64_t uint_value) { - Variadic variadic; - variadic.type = Type::kUint; + static constexpr Variadic UnsignedInteger(uint64_t uint_value) { + Variadic variadic(Type::kUint); variadic.uint_value = uint_value; return variadic; } - static Variadic String(StringPool::Id string_id) { - Variadic variadic; - variadic.type = Type::kString; + static constexpr Variadic String(StringPool::Id string_id) { + Variadic variadic(Type::kString); variadic.string_value = string_id; return variadic; } - static Variadic Real(double real_value) { - Variadic variadic; - variadic.type = Type::kReal; + static constexpr Variadic Real(double real_value) { + Variadic variadic(Type::kReal); variadic.real_value = real_value; return variadic; } // This variadic type is used to distinguish between integers and pointer // values for correct JSON export of TrackEvent arguments. - static Variadic Pointer(uint64_t pointer_value) { - Variadic variadic; - variadic.type = Type::kPointer; + static constexpr Variadic Pointer(uint64_t pointer_value) { + Variadic variadic(Type::kPointer); variadic.pointer_value = pointer_value; return variadic; } - static Variadic Boolean(bool bool_value) { - Variadic variadic; - variadic.type = Type::kBool; + static constexpr Variadic Boolean(bool bool_value) { + Variadic variadic(Type::kBool); variadic.bool_value = bool_value; return variadic; } // This variadic type is used to distinguish between regular string and JSON // string values for correct JSON export of TrackEvent arguments. - static Variadic Json(StringPool::Id json_value) { - Variadic variadic; - variadic.type = Type::kJson; + static constexpr Variadic Json(StringPool::Id json_value) { + Variadic variadic(Type::kJson); variadic.json_value = json_value; return variadic; } - static Variadic Null() { - Variadic variadic; - variadic.type = Type::kNull; - return variadic; - } + static constexpr Variadic Null() { return Variadic(Type::kNull); } // Used in tests. bool operator==(const Variadic& other) const { @@ -137,9 +130,11 @@ struct Variadic { bool bool_value; StringPool::Id json_value; }; + + private: + constexpr explicit Variadic(Type t) : type(t), int_value(0) {} }; -} // namespace trace_processor -} // namespace perfetto +} // namespace perfetto::trace_processor #endif // SRC_TRACE_PROCESSOR_TYPES_VARIADIC_H_ diff --git a/test/trace_processor/diff_tests/parser/parsing/tests_sys_stats.py b/test/trace_processor/diff_tests/parser/parsing/tests_sys_stats.py index ab23e0beab..2891d74f16 100644 --- a/test/trace_processor/diff_tests/parser/parsing/tests_sys_stats.py +++ b/test/trace_processor/diff_tests/parser/parsing/tests_sys_stats.py @@ -54,7 +54,7 @@ def test_cpuidle_stats(self): ts, EXTRACT_ARG(t.dimension_arg_set_id, 'cpu_idle_state') as state, value, - EXTRACT_ARG(t.dimension_arg_set_id, 'ucpu') as cpu + EXTRACT_ARG(t.dimension_arg_set_id, 'cpu') as cpu FROM counter c JOIN track t on c.track_id = t.id ORDER BY ts; diff --git a/test/trace_processor/diff_tests/tables/tests.py b/test/trace_processor/diff_tests/tables/tests.py index e6ab9a4f66..e7024a92d7 100644 --- a/test/trace_processor/diff_tests/tables/tests.py +++ b/test/trace_processor/diff_tests/tables/tests.py @@ -461,11 +461,11 @@ def test_cpu_track_table_machine_id(self): query=""" SELECT ct.type, - ct.ucpu, - c.cpu, - ct.machine_id + c.ucpu, + ct.cpu, + c.machine_id FROM cpu_track AS ct - JOIN cpu AS c ON ct.ucpu = c.id + JOIN cpu AS c ON ct.machine_id IS c.machine_id AND ct.cpu = c.cpu ORDER BY ct.type, c.cpu """, out=Csv(""" diff --git a/test/trace_processor/diff_tests/tables/tests_counters.py b/test/trace_processor/diff_tests/tables/tests_counters.py index d5b3bcf53b..c14051ef89 100644 --- a/test/trace_processor/diff_tests/tables/tests_counters.py +++ b/test/trace_processor/diff_tests/tables/tests_counters.py @@ -191,8 +191,7 @@ def test_counters_where_cpu_counters_where_cpu_machine_id(self): value FROM counter c JOIN cpu_counter_track t ON c.track_id = t.id - JOIN cpu ON t.ucpu = cpu.id - WHERE cpu.cpu = 1; + WHERE t.cpu = 1; """, out=Csv(""" "ts","dur","value" diff --git a/ui/src/plugins/org.kernel.SuspendResumeLatency/suspend_resume_details.ts b/ui/src/plugins/org.kernel.SuspendResumeLatency/suspend_resume_details.ts index d43f6997d2..98a84790bd 100644 --- a/ui/src/plugins/org.kernel.SuspendResumeLatency/suspend_resume_details.ts +++ b/ui/src/plugins/org.kernel.SuspendResumeLatency/suspend_resume_details.ts @@ -135,17 +135,18 @@ async function loadSuspendResumeEventDetails( id: number, ): Promise { const suspendResumeDetailsQuery = ` - SELECT ts, - dur, - EXTRACT_ARG(arg_set_id, 'utid') as utid, - EXTRACT_ARG(arg_set_id, 'ucpu') as ucpu, - EXTRACT_ARG(arg_set_id, 'event_type') as event_type, - EXTRACT_ARG(arg_set_id, 'device_name') as device_name, - EXTRACT_ARG(arg_set_id, 'driver_name') as driver_name, - EXTRACT_ARG(arg_set_id, 'callback_phase') as callback_phase - FROM slice - WHERE slice_id = ${id}; - `; + SELECT + ts, + dur, + EXTRACT_ARG(arg_set_id, 'utid') as utid, + EXTRACT_ARG(arg_set_id, 'cpu') as cpu, + EXTRACT_ARG(arg_set_id, 'event_type') as event_type, + EXTRACT_ARG(arg_set_id, 'device_name') as device_name, + EXTRACT_ARG(arg_set_id, 'driver_name') as driver_name, + EXTRACT_ARG(arg_set_id, 'callback_phase') as callback_phase + FROM slice + WHERE slice_id = ${id}; + `; const suspendResumeDetailsResult = await engine.query( suspendResumeDetailsQuery, @@ -154,7 +155,7 @@ async function loadSuspendResumeEventDetails( ts: LONG, dur: LONG, utid: NUM, - ucpu: NUM, + cpu: NUM, event_type: STR_NULL, device_name: STR_NULL, driver_name: STR_NULL, @@ -175,11 +176,12 @@ async function loadSuspendResumeEventDetails( } const threadStateQuery = ` - SELECT t.id as threadStateId - FROM thread_state t - WHERE t.utid = ${suspendResumeEventRow.utid} - AND t.ts <= ${suspendResumeEventRow.ts} - AND t.ts + t.dur > ${suspendResumeEventRow.ts}; + SELECT t.id as threadStateId + FROM thread_state t + WHERE + t.utid = ${suspendResumeEventRow.utid} + AND t.ts <= ${suspendResumeEventRow.ts} + AND t.ts + t.dur > ${suspendResumeEventRow.ts}; `; const threadStateResult = await engine.query(threadStateQuery); let threadStateId = 0; @@ -190,25 +192,11 @@ async function loadSuspendResumeEventDetails( threadStateId = threadStateRow.threadStateId; } - const cpuQuery = ` - SELECT cpu - FROM cpu - WHERE cpu.id = ${suspendResumeEventRow.ucpu} - `; - const cpuResult = await engine.query(cpuQuery); - let cpu = 0; - if (cpuResult.numRows() > 0) { - const cpuRow = cpuResult.firstRow({ - cpu: NUM, - }); - cpu = cpuRow.cpu; - } - return { ts: Time.fromRaw(suspendResumeEventRow.ts), dur: Duration.fromRaw(suspendResumeEventRow.dur), utid: suspendResumeEventRow.utid, - cpu: cpu, + cpu: suspendResumeEventRow.cpu, event_type: suspendResumeEventRow.event_type !== null ? suspendResumeEventRow.event_type