From a68342541e74b8ecb35a6f84c3d056add7244935 Mon Sep 17 00:00:00 2001 From: Lalit Maganti Date: Fri, 9 Aug 2024 16:01:15 +0100 Subject: [PATCH] tp: remove external uses of private Table methods Migrate callers to other, proper public methods. Change-Id: I74a43aa997825dd3cee0ffb560af9277b8d644d1 --- .../db/query_executor_benchmark.cc | 58 +++--- src/trace_processor/db/table.h | 19 +- .../proto/proto_trace_parser_impl_unittest.cc | 191 ++++++++++-------- .../intrinsics/functions/BUILD.gn | 1 + .../intrinsics/functions/to_ftrace.cc | 88 ++++---- .../experimental_flat_slice_unittest.cc | 14 +- .../experimental_slice_layout_unittest.cc | 6 +- .../flamegraph_construction_algorithms.cc | 43 ++-- src/trace_processor/tables/BUILD.gn | 1 + src/trace_processor/tables/macros_internal.h | 1 + .../tables/py_tables_benchmark.cc | 38 ++-- .../tables/py_tables_unittest.cc | 52 ++--- test/gtest_and_gmock.h | 14 +- 13 files changed, 278 insertions(+), 248 deletions(-) diff --git a/src/trace_processor/db/query_executor_benchmark.cc b/src/trace_processor/db/query_executor_benchmark.cc index 95b64ac798..5fe039a896 100644 --- a/src/trace_processor/db/query_executor_benchmark.cc +++ b/src/trace_processor/db/query_executor_benchmark.cc @@ -103,6 +103,14 @@ std::vector ReadCSV(benchmark::State& state, return base::SplitString(table_csv, "\n"); } +template +double CountRows(It it) { + double i = 0; + for (; it; ++it, ++i) { + } + return i; +} + StringPool::Id StripAndIntern(StringPool& pool, const std::string& data) { std::string res = base::StripSuffix(base::StripPrefix(data, "\""), "\""); return pool.InternString(base::StringView(res)); @@ -237,16 +245,16 @@ void BenchmarkSliceTableFilter(benchmark::State& state, Query q; q.constraints = c; for (auto _ : state) { - benchmark::DoNotOptimize(table.table_.QueryToRowMap(q)); + benchmark::DoNotOptimize(table.table_.FilterToIterator(q)); } state.counters["s/row"] = benchmark::Counter(static_cast(table.table_.row_count()), benchmark::Counter::kIsIterationInvariantRate | benchmark::Counter::kInvert); - state.counters["s/out"] = benchmark::Counter( - static_cast(table.table_.QueryToRowMap(q).size()), - benchmark::Counter::kIsIterationInvariantRate | - benchmark::Counter::kInvert); + state.counters["s/out"] = + benchmark::Counter(CountRows(table.table_.FilterToIterator(q)), + benchmark::Counter::kIsIterationInvariantRate | + benchmark::Counter::kInvert); } void BenchmarkSliceTableSort(benchmark::State& state, @@ -266,32 +274,32 @@ void BenchmarkExpectedFrameTableQuery( ExpectedFrameTimelineTableForBenchmark& table, Query q) { for (auto _ : state) { - benchmark::DoNotOptimize(table.table_.QueryToRowMap(q)); + benchmark::DoNotOptimize(table.table_.FilterToIterator(q)); } state.counters["s/row"] = benchmark::Counter(static_cast(table.table_.row_count()), benchmark::Counter::kIsIterationInvariantRate | benchmark::Counter::kInvert); - state.counters["s/out"] = benchmark::Counter( - static_cast(table.table_.QueryToRowMap(q).size()), - benchmark::Counter::kIsIterationInvariantRate | - benchmark::Counter::kInvert); + state.counters["s/out"] = + benchmark::Counter(CountRows(table.table_.FilterToIterator(q)), + benchmark::Counter::kIsIterationInvariantRate | + benchmark::Counter::kInvert); } void BenchmarkFtraceEventTableQuery(benchmark::State& state, FtraceEventTableForBenchmark& table, Query q) { for (auto _ : state) { - benchmark::DoNotOptimize(table.table_.QueryToRowMap(q)); + benchmark::DoNotOptimize(table.table_.FilterToIterator(q)); } state.counters["s/row"] = benchmark::Counter(static_cast(table.table_.row_count()), benchmark::Counter::kIsIterationInvariantRate | benchmark::Counter::kInvert); - state.counters["s/out"] = benchmark::Counter( - static_cast(table.table_.QueryToRowMap(q).size()), - benchmark::Counter::kIsIterationInvariantRate | - benchmark::Counter::kInvert); + state.counters["s/out"] = + benchmark::Counter(CountRows(table.table_.FilterToIterator(q)), + benchmark::Counter::kIsIterationInvariantRate | + benchmark::Counter::kInvert); } void BenchmarkFtraceEventTableSort(benchmark::State& state, @@ -441,16 +449,16 @@ void BM_QEDenseNullFilter(benchmark::State& state) { Query q; q.constraints = {c}; for (auto _ : state) { - benchmark::DoNotOptimize(table.table_.QueryToRowMap(q)); + benchmark::DoNotOptimize(table.table_.FilterToIterator(q)); } state.counters["s/row"] = benchmark::Counter(static_cast(table.table_.row_count()), benchmark::Counter::kIsIterationInvariantRate | benchmark::Counter::kInvert); - state.counters["s/out"] = benchmark::Counter( - static_cast(table.table_.QueryToRowMap(q).size()), - benchmark::Counter::kIsIterationInvariantRate | - benchmark::Counter::kInvert); + state.counters["s/out"] = + benchmark::Counter(CountRows(table.table_.FilterToIterator(q)), + benchmark::Counter::kIsIterationInvariantRate | + benchmark::Counter::kInvert); } BENCHMARK(BM_QEDenseNullFilter); @@ -461,16 +469,16 @@ void BM_QEDenseNullFilterIsNull(benchmark::State& state) { Query q; q.constraints = {c}; for (auto _ : state) { - benchmark::DoNotOptimize(table.table_.QueryToRowMap(q)); + benchmark::DoNotOptimize(table.table_.FilterToIterator(q)); } state.counters["s/row"] = benchmark::Counter(static_cast(table.table_.row_count()), benchmark::Counter::kIsIterationInvariantRate | benchmark::Counter::kInvert); - state.counters["s/out"] = benchmark::Counter( - static_cast(table.table_.QueryToRowMap(q).size()), - benchmark::Counter::kIsIterationInvariantRate | - benchmark::Counter::kInvert); + state.counters["s/out"] = + benchmark::Counter(CountRows(table.table_.FilterToIterator(q)), + benchmark::Counter::kIsIterationInvariantRate | + benchmark::Counter::kInvert); } BENCHMARK(BM_QEDenseNullFilterIsNull); diff --git a/src/trace_processor/db/table.h b/src/trace_processor/db/table.h index 8b89139815..f05b7d9ca0 100644 --- a/src/trace_processor/db/table.h +++ b/src/trace_processor/db/table.h @@ -141,10 +141,9 @@ class Table { Table(Table&& other) noexcept { *this = std::move(other); } Table& operator=(Table&& other) noexcept; - // Return a chain corresponding to a given column. - const column::DataLayerChain& ChainForColumn(uint32_t col_idx) const { - return *chains_[col_idx]; - } + // Filters and sorts the tables with the arguments specified, returning the + // result as a RowMap. + RowMap QueryToRowMap(const Query&) const; // Applies the RowMap |rm| onto this table and returns an iterator over the // resulting rows. @@ -158,10 +157,6 @@ class Table { return Iterator(this, std::move(rm)); } - // Do not add any further uses. - // TODO(lalitm): make this private. - RowMap QueryToRowMap(const Query&) const; - std::optional GetIndex( const std::vector& cols) const { for (const auto& idx : indexes_) { @@ -206,8 +201,9 @@ class Table { } uint32_t row_count() const { return row_count_; } - StringPool* string_pool() const { return string_pool_; } const std::vector& columns() const { return columns_; } + StringPool* string_pool() const { return string_pool_; } + const std::vector>& storage_layers() const { return storage_layers_; } @@ -253,6 +249,7 @@ class Table { private: friend class ColumnLegacy; + friend class QueryExecutor; struct ColumnIndex { std::string name; @@ -272,6 +269,10 @@ class Table { RowMap ApplyIdJoinConstraints(const std::vector&, uint32_t& cs_offset) const; + const column::DataLayerChain& ChainForColumn(uint32_t col_idx) const { + return *chains_[col_idx]; + } + StringPool* string_pool_ = nullptr; uint32_t row_count_ = 0; std::vector overlays_; 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 5b63ce4109..cbff477a29 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 @@ -14,35 +14,51 @@ * limitations under the License. */ -#include "src/trace_processor/importers/proto/proto_trace_reader.h" +#include "src/trace_processor/importers/proto/proto_trace_parser_impl.h" +#include +#include +#include +#include +#include +#include +#include #include #include -#include "perfetto/base/logging.h" + +#include "perfetto/base/status.h" #include "perfetto/ext/base/string_view.h" #include "perfetto/protozero/scattered_heap_buffer.h" +#include "perfetto/trace_processor/basic_types.h" #include "perfetto/trace_processor/trace_blob.h" +#include "perfetto/trace_processor/trace_blob_view.h" +#include "src/trace_processor/db/column/types.h" #include "src/trace_processor/importers/common/args_tracker.h" #include "src/trace_processor/importers/common/args_translation_table.h" #include "src/trace_processor/importers/common/clock_tracker.h" #include "src/trace_processor/importers/common/cpu_tracker.h" #include "src/trace_processor/importers/common/event_tracker.h" #include "src/trace_processor/importers/common/flow_tracker.h" +#include "src/trace_processor/importers/common/global_args_tracker.h" #include "src/trace_processor/importers/common/machine_tracker.h" #include "src/trace_processor/importers/common/mapping_tracker.h" #include "src/trace_processor/importers/common/metadata_tracker.h" #include "src/trace_processor/importers/common/process_track_translation_table.h" #include "src/trace_processor/importers/common/process_tracker.h" #include "src/trace_processor/importers/common/slice_tracker.h" +#include "src/trace_processor/importers/common/slice_translation_table.h" #include "src/trace_processor/importers/common/stack_profile_tracker.h" #include "src/trace_processor/importers/common/track_tracker.h" #include "src/trace_processor/importers/ftrace/ftrace_sched_event_tracker.h" #include "src/trace_processor/importers/proto/additional_modules.h" #include "src/trace_processor/importers/proto/default_modules.h" -#include "src/trace_processor/importers/proto/proto_trace_parser_impl.h" +#include "src/trace_processor/importers/proto/proto_trace_reader.h" #include "src/trace_processor/sorter/trace_sorter.h" #include "src/trace_processor/storage/metadata.h" +#include "src/trace_processor/storage/stats.h" #include "src/trace_processor/storage/trace_storage.h" +#include "src/trace_processor/tables/metadata_tables_py.h" +#include "src/trace_processor/types/variadic.h" #include "src/trace_processor/util/descriptors.h" #include "test/gtest_and_gmock.h" @@ -79,8 +95,7 @@ #include "protos/perfetto/trace/track_event/track_descriptor.pbzero.h" #include "protos/perfetto/trace/track_event/track_event.pbzero.h" -namespace perfetto { -namespace trace_processor { +namespace perfetto::trace_processor { namespace { using ::std::make_pair; @@ -256,12 +271,13 @@ class ProtoTraceParserTest : public ::testing::Test { ProtoTraceParserTest() { storage_ = new TraceStorage(); context_.storage.reset(storage_); - context_.track_tracker.reset(new TrackTracker(&context_)); - context_.global_args_tracker.reset( - new GlobalArgsTracker(context_.storage.get())); + context_.track_tracker = std::make_unique(&context_); + context_.global_args_tracker = + std::make_unique(context_.storage.get()); context_.mapping_tracker.reset(new MappingTracker(&context_)); - context_.stack_profile_tracker.reset(new StackProfileTracker(&context_)); - context_.args_tracker.reset(new ArgsTracker(&context_)); + context_.stack_profile_tracker = + std::make_unique(&context_); + context_.args_tracker = std::make_unique(&context_); context_.args_translation_table.reset(new ArgsTranslationTable(storage_)); context_.metadata_tracker.reset( new MetadataTracker(context_.storage.get())); @@ -277,14 +293,16 @@ class ProtoTraceParserTest : public ::testing::Test { new ProcessTrackTranslationTable(storage_)); slice_ = new NiceMock(&context_); context_.slice_tracker.reset(slice_); - context_.slice_translation_table.reset(new SliceTranslationTable(storage_)); + context_.slice_translation_table = + std::make_unique(storage_); clock_ = new ClockTracker(&context_); context_.clock_tracker.reset(clock_); - context_.flow_tracker.reset(new FlowTracker(&context_)); - context_.proto_trace_parser.reset(new ProtoTraceParserImpl(&context_)); - context_.sorter.reset( - new TraceSorter(&context_, TraceSorter::SortingMode::kFullSort)); - context_.descriptor_pool_.reset(new DescriptorPool()); + context_.flow_tracker = std::make_unique(&context_); + context_.proto_trace_parser = + std::make_unique(&context_); + context_.sorter = std::make_shared( + &context_, TraceSorter::SortingMode::kFullSort); + context_.descriptor_pool_ = std::make_unique(); RegisterDefaultModules(&context_); RegisterAdditionalModules(&context_); @@ -294,7 +312,7 @@ class ProtoTraceParserTest : public ::testing::Test { void SetUp() override { ResetTraceBuffers(); } - util::Status Tokenize() { + base::Status Tokenize() { trace_->Finalize(); std::vector trace_bytes = trace_.SerializeAsArray(); std::unique_ptr raw_trace(new uint8_t[trace_bytes.size()]); @@ -312,12 +330,12 @@ class ProtoTraceParserTest : public ::testing::Test { const auto& args = storage_->arg_table(); Query q; q.constraints = {args.arg_set_id().eq(set_id)}; - RowMap rm = args.QueryToRowMap(q); + bool found = false; - for (auto it = rm.IterateRows(); it; it.Next()) { - if (args[it.index()].key() == key_id) { - EXPECT_EQ(args[it.index()].flat_key(), key_id); - if (storage_->GetArgValue(it.index()) == value) { + for (auto it = args.FilterToIterator(q); it; ++it) { + if (it.key() == key_id) { + EXPECT_EQ(it.flat_key(), key_id); + if (storage_->GetArgValue(it.row_number().row_number()) == value) { found = true; break; } @@ -462,18 +480,21 @@ TEST_F(ProtoTraceParserTest, LoadGenericFtrace) { const auto& args = storage_->arg_table(); Query q; q.constraints = {args.arg_set_id().eq(set_id)}; - RowMap rm = args.QueryToRowMap(q); - auto row = rm.Get(0); + auto it = args.FilterToIterator(q); + ASSERT_TRUE(it); - ASSERT_EQ(storage_->GetString(args[row].key()), "meta1"); - ASSERT_EQ(storage_->GetString(*args[row++].string_value()), "value1"); + ASSERT_EQ(storage_->GetString(it.key()), "meta1"); + ASSERT_EQ(storage_->GetString(*it.string_value()), "value1"); + ASSERT_TRUE(++it); - ASSERT_EQ(storage_->GetString(args[row].key()), "meta2"); - ASSERT_EQ(args[row++].int_value(), -2); + ASSERT_EQ(storage_->GetString(it.key()), "meta2"); + ASSERT_EQ(it.int_value(), -2); + ASSERT_TRUE(++it); - ASSERT_EQ(storage_->GetString(args[row].key()), "meta3"); - ASSERT_EQ(args[row++].int_value(), 3); + ASSERT_EQ(storage_->GetString(it.key()), "meta3"); + ASSERT_EQ(it.int_value(), 3); + ASSERT_FALSE(++it); } TEST_F(ProtoTraceParserTest, LoadMultipleEvents) { @@ -1074,10 +1095,10 @@ TEST_F(ProtoTraceParserTest, TrackEventWithInternedData) { legacy_event->set_phase('B'); auto* interned_data = packet->set_interned_data(); - auto cat1 = interned_data->add_event_categories(); + auto* cat1 = interned_data->add_event_categories(); cat1->set_iid(1); cat1->set_name("cat1"); - auto ev1 = interned_data->add_event_names(); + auto* ev1 = interned_data->add_event_names(); ev1->set_iid(1); ev1->set_name("ev1"); } @@ -1137,13 +1158,13 @@ TEST_F(ProtoTraceParserTest, TrackEventWithInternedData) { protos::pbzero::TrackEvent::LegacyEvent::FLOW_OUT); auto* interned_data = packet->set_interned_data(); - auto cat2 = interned_data->add_event_categories(); + auto* cat2 = interned_data->add_event_categories(); cat2->set_iid(2); cat2->set_name("cat2"); - auto cat3 = interned_data->add_event_categories(); + auto* cat3 = interned_data->add_event_categories(); cat3->set_iid(3); cat3->set_name("cat3"); - auto ev2 = interned_data->add_event_names(); + auto* ev2 = interned_data->add_event_names(); ev2->set_iid(4); ev2->set_name("ev2"); } @@ -1285,10 +1306,10 @@ TEST_F(ProtoTraceParserTest, TrackEventAsyncEvents) { legacy_event->set_use_async_tts(true); auto* interned_data = packet->set_interned_data(); - auto cat1 = interned_data->add_event_categories(); + auto* cat1 = interned_data->add_event_categories(); cat1->set_iid(1); cat1->set_name("cat1"); - auto ev1 = interned_data->add_event_names(); + auto* ev1 = interned_data->add_event_names(); ev1->set_iid(1); ev1->set_name("ev1"); } @@ -1318,7 +1339,7 @@ TEST_F(ProtoTraceParserTest, TrackEventAsyncEvents) { legacy_event->set_global_id(10); auto* interned_data = packet->set_interned_data(); - auto ev2 = interned_data->add_event_names(); + auto* ev2 = interned_data->add_event_names(); ev2->set_iid(2); ev2->set_name("ev2"); } @@ -1335,7 +1356,7 @@ TEST_F(ProtoTraceParserTest, TrackEventAsyncEvents) { legacy_event->set_global_id(15); auto* interned_data = packet->set_interned_data(); - auto cat2 = interned_data->add_event_categories(); + auto* cat2 = interned_data->add_event_categories(); cat2->set_iid(2); cat2->set_name("cat2"); } @@ -1452,10 +1473,10 @@ TEST_F(ProtoTraceParserTest, TrackEventWithTrackDescriptors) { legacy_event->set_use_async_tts(true); auto* interned_data = packet->set_interned_data(); - auto cat1 = interned_data->add_event_categories(); + auto* cat1 = interned_data->add_event_categories(); cat1->set_iid(1); cat1->set_name("cat1"); - auto ev1 = interned_data->add_event_names(); + auto* ev1 = interned_data->add_event_names(); ev1->set_iid(1); ev1->set_name("ev1"); } @@ -1472,10 +1493,10 @@ TEST_F(ProtoTraceParserTest, TrackEventWithTrackDescriptors) { event->set_type(protos::pbzero::TrackEvent::TYPE_INSTANT); auto* interned_data = packet->set_interned_data(); - auto cat1 = interned_data->add_event_categories(); + auto* cat1 = interned_data->add_event_categories(); cat1->set_iid(2); cat1->set_name("cat2"); - auto ev1 = interned_data->add_event_names(); + auto* ev1 = interned_data->add_event_names(); ev1->set_iid(2); ev1->set_name("ev2"); } @@ -1519,10 +1540,10 @@ TEST_F(ProtoTraceParserTest, TrackEventWithTrackDescriptors) { event->set_type(protos::pbzero::TrackEvent::TYPE_INSTANT); auto* interned_data = packet->set_interned_data(); - auto cat1 = interned_data->add_event_categories(); + auto* cat1 = interned_data->add_event_categories(); cat1->set_iid(1); cat1->set_name("cat3"); - auto ev1 = interned_data->add_event_names(); + auto* ev1 = interned_data->add_event_names(); ev1->set_iid(1); ev1->set_name("ev3"); } @@ -1922,10 +1943,10 @@ TEST_F(ProtoTraceParserTest, TrackEventMultipleSequences) { legacy_event->set_phase('B'); auto* interned_data = packet->set_interned_data(); - auto cat1 = interned_data->add_event_categories(); + auto* cat1 = interned_data->add_event_categories(); cat1->set_iid(1); cat1->set_name("cat1"); - auto ev1 = interned_data->add_event_names(); + auto* ev1 = interned_data->add_event_names(); ev1->set_iid(1); ev1->set_name("ev1"); } @@ -1949,10 +1970,10 @@ TEST_F(ProtoTraceParserTest, TrackEventMultipleSequences) { legacy_event->set_phase('B'); auto* interned_data = packet->set_interned_data(); - auto cat1 = interned_data->add_event_categories(); + auto* cat1 = interned_data->add_event_categories(); cat1->set_iid(1); cat1->set_name("cat1"); - auto ev2 = interned_data->add_event_names(); + auto* ev2 = interned_data->add_event_names(); ev2->set_iid(1); ev2->set_name("ev2"); } @@ -2057,16 +2078,16 @@ TEST_F(ProtoTraceParserTest, TrackEventWithDebugAnnotations) { legacy_event->set_phase('B'); auto* interned_data = packet->set_interned_data(); - auto cat1 = interned_data->add_event_categories(); + auto* cat1 = interned_data->add_event_categories(); cat1->set_iid(1); cat1->set_name("cat1"); - auto ev1 = interned_data->add_event_names(); + auto* ev1 = interned_data->add_event_names(); ev1->set_iid(1); ev1->set_name("ev1"); - auto an1 = interned_data->add_debug_annotation_names(); + auto* an1 = interned_data->add_debug_annotation_names(); an1->set_iid(1); an1->set_name("an1"); - auto an2 = interned_data->add_debug_annotation_names(); + auto* an2 = interned_data->add_debug_annotation_names(); an2->set_iid(2); an2->set_name("an2"); } @@ -2103,25 +2124,25 @@ TEST_F(ProtoTraceParserTest, TrackEventWithDebugAnnotations) { legacy_event->set_phase('E'); auto* interned_data = packet->set_interned_data(); - auto an3 = interned_data->add_debug_annotation_names(); + auto* an3 = interned_data->add_debug_annotation_names(); an3->set_iid(3); an3->set_name("an3"); - auto an4 = interned_data->add_debug_annotation_names(); + auto* an4 = interned_data->add_debug_annotation_names(); an4->set_iid(4); an4->set_name("an4"); - auto an5 = interned_data->add_debug_annotation_names(); + auto* an5 = interned_data->add_debug_annotation_names(); an5->set_iid(5); an5->set_name("an5"); - auto an6 = interned_data->add_debug_annotation_names(); + auto* an6 = interned_data->add_debug_annotation_names(); an6->set_iid(6); an6->set_name("an6"); - auto an7 = interned_data->add_debug_annotation_names(); + auto* an7 = interned_data->add_debug_annotation_names(); an7->set_iid(7); an7->set_name("an7"); - auto an8 = interned_data->add_debug_annotation_names(); + auto* an8 = interned_data->add_debug_annotation_names(); an8->set_iid(8); an8->set_name("an8"); - auto an9 = interned_data->add_debug_annotation_names(); + auto* an9 = interned_data->add_debug_annotation_names(); an9->set_iid(9); an9->set_name("an8.foo"); } @@ -2230,13 +2251,13 @@ TEST_F(ProtoTraceParserTest, TrackEventWithTaskExecution) { legacy_event->set_phase('B'); auto* interned_data = packet->set_interned_data(); - auto cat1 = interned_data->add_event_categories(); + auto* cat1 = interned_data->add_event_categories(); cat1->set_iid(1); cat1->set_name("cat1"); - auto ev1 = interned_data->add_event_names(); + auto* ev1 = interned_data->add_event_names(); ev1->set_iid(1); ev1->set_name("ev1"); - auto loc1 = interned_data->add_source_locations(); + auto* loc1 = interned_data->add_source_locations(); loc1->set_iid(1); loc1->set_file_name("file1"); loc1->set_function_name("func1"); @@ -2295,19 +2316,19 @@ TEST_F(ProtoTraceParserTest, TrackEventWithLogMessage) { legacy_event->set_phase('I'); auto* interned_data = packet->set_interned_data(); - auto cat1 = interned_data->add_event_categories(); + auto* cat1 = interned_data->add_event_categories(); cat1->set_iid(1); cat1->set_name("cat1"); - auto ev1 = interned_data->add_event_names(); + auto* ev1 = interned_data->add_event_names(); ev1->set_iid(1); ev1->set_name("ev1"); - auto body = interned_data->add_log_message_body(); + auto* body = interned_data->add_log_message_body(); body->set_iid(1); body->set_body("body1"); - auto loc1 = interned_data->add_source_locations(); + auto* loc1 = interned_data->add_source_locations(); loc1->set_iid(1); loc1->set_file_name("file1"); loc1->set_function_name("func1"); @@ -2383,13 +2404,13 @@ TEST_F(ProtoTraceParserTest, TrackEventParseLegacyEventIntoRawTable) { annotation1->set_uint_value(10u); auto* interned_data = packet->set_interned_data(); - auto cat1 = interned_data->add_event_categories(); + auto* cat1 = interned_data->add_event_categories(); cat1->set_iid(1); cat1->set_name("cat1"); - auto ev1 = interned_data->add_event_names(); + auto* ev1 = interned_data->add_event_names(); ev1->set_iid(1); ev1->set_name("ev1"); - auto an1 = interned_data->add_debug_annotation_names(); + auto* an1 = interned_data->add_debug_annotation_names(); an1->set_iid(1); an1->set_name("an1"); } @@ -2505,7 +2526,7 @@ TEST_F(ProtoTraceParserTest, ParseEventWithClockIdButWithoutClockSnapshot) { metadata->set_int_value(23); } - util::Status status = Tokenize(); + base::Status status = Tokenize(); EXPECT_TRUE(status.ok()); context_.sorter->ExtractEventsForced(); @@ -2625,13 +2646,12 @@ TEST_F(ProtoTraceParserTest, LoadChromeBenchmarkMetadata) { base::StringView tags = metadata::kNames[metadata::benchmark_story_tags]; context_.sorter->ExtractEventsForced(); - EXPECT_EQ(storage_->metadata_table().row_count(), 3u); std::vector> meta_entries; for (auto it = storage_->metadata_table().IterateRows(); it; ++it) { - meta_entries.emplace_back(std::make_pair( - storage_->GetString(it.name()), storage_->GetString(*it.str_value()))); + meta_entries.emplace_back(storage_->GetString(it.name()), + storage_->GetString(*it.str_value())); } EXPECT_THAT(meta_entries, UnorderedElementsAreArray({make_pair(benchmark, kName), @@ -2797,29 +2817,29 @@ TEST_F(ProtoTraceParserTest, ParseCPUProfileSamplesIntoTable) { auto* interned_data = packet->set_interned_data(); - auto mapping = interned_data->add_mappings(); + auto* mapping = interned_data->add_mappings(); mapping->set_iid(1); mapping->set_build_id(1); - auto build_id = interned_data->add_build_ids(); + auto* build_id = interned_data->add_build_ids(); build_id->set_iid(1); build_id->set_str("3BBCFBD372448A727265C3E7C4D954F91"); - auto frame = interned_data->add_frames(); + auto* frame = interned_data->add_frames(); frame->set_iid(1); frame->set_rel_pc(0x42); frame->set_mapping_id(1); - auto frame2 = interned_data->add_frames(); + auto* frame2 = interned_data->add_frames(); frame2->set_iid(2); frame2->set_rel_pc(0x4242); frame2->set_mapping_id(1); - auto callstack = interned_data->add_callstacks(); + auto* callstack = interned_data->add_callstacks(); callstack->set_iid(1); callstack->add_frame_ids(1); - auto callstack2 = interned_data->add_callstacks(); + auto* callstack2 = interned_data->add_callstacks(); callstack2->set_iid(42); callstack2->add_frame_ids(2); } @@ -2907,20 +2927,20 @@ TEST_F(ProtoTraceParserTest, CPUProfileSamplesTimestampsAreClockMonotonic) { auto* interned_data = packet->set_interned_data(); - auto mapping = interned_data->add_mappings(); + auto* mapping = interned_data->add_mappings(); mapping->set_iid(1); mapping->set_build_id(1); - auto build_id = interned_data->add_build_ids(); + auto* build_id = interned_data->add_build_ids(); build_id->set_iid(1); build_id->set_str("3BBCFBD372448A727265C3E7C4D954F91"); - auto frame = interned_data->add_frames(); + auto* frame = interned_data->add_frames(); frame->set_iid(1); frame->set_rel_pc(0x42); frame->set_mapping_id(1); - auto callstack = interned_data->add_callstacks(); + auto* callstack = interned_data->add_callstacks(); callstack->set_iid(1); callstack->add_frame_ids(1); } @@ -3010,5 +3030,4 @@ TEST_F(ProtoTraceParserTest, ConfigPbtxt) { } } // namespace -} // namespace trace_processor -} // namespace perfetto +} // namespace perfetto::trace_processor diff --git a/src/trace_processor/perfetto_sql/intrinsics/functions/BUILD.gn b/src/trace_processor/perfetto_sql/intrinsics/functions/BUILD.gn index 53df4f7b69..863c2e6f9c 100644 --- a/src/trace_processor/perfetto_sql/intrinsics/functions/BUILD.gn +++ b/src/trace_processor/perfetto_sql/intrinsics/functions/BUILD.gn @@ -76,6 +76,7 @@ source_set("functions") { "../../../perfetto_sql/intrinsics/table_functions", "../../../sqlite", "../../../storage", + "../../../tables", "../../../types", "../../../util", "../../../util:profile_builder", diff --git a/src/trace_processor/perfetto_sql/intrinsics/functions/to_ftrace.cc b/src/trace_processor/perfetto_sql/intrinsics/functions/to_ftrace.cc index efa7659d6c..7898fb926a 100644 --- a/src/trace_processor/perfetto_sql/intrinsics/functions/to_ftrace.cc +++ b/src/trace_processor/perfetto_sql/intrinsics/functions/to_ftrace.cc @@ -16,16 +16,31 @@ #include "src/trace_processor/perfetto_sql/intrinsics/functions/to_ftrace.h" -#include "perfetto/base/compiler.h" +#include +#include +#include +#include +#include +#include + +#include "perfetto/base/logging.h" #include "perfetto/base/status.h" -#include "perfetto/ext/base/string_utils.h" +#include "perfetto/ext/base/string_view.h" +#include "perfetto/ext/base/string_writer.h" +#include "perfetto/public/compiler.h" #include "perfetto/trace_processor/basic_types.h" +#include "src/trace_processor/containers/null_term_string_view.h" +#include "src/trace_processor/db/column/types.h" #include "src/trace_processor/importers/common/system_info_tracker.h" #include "src/trace_processor/importers/ftrace/ftrace_descriptors.h" -#include "src/trace_processor/sqlite/sqlite_utils.h" +#include "src/trace_processor/sqlite/bindings/sqlite_type.h" +#include "src/trace_processor/sqlite/bindings/sqlite_value.h" +#include "src/trace_processor/storage/trace_storage.h" +#include "src/trace_processor/tables/metadata_tables_py.h" #include "src/trace_processor/types/gfp_flags.h" #include "src/trace_processor/types/softirq_action.h" #include "src/trace_processor/types/task_state.h" +#include "src/trace_processor/types/trace_processor_context.h" #include "src/trace_processor/types/variadic.h" #include "protos/perfetto/trace/ftrace/binder.pbzero.h" @@ -43,9 +58,9 @@ #include "protos/perfetto/trace/ftrace/samsung.pbzero.h" #include "protos/perfetto/trace/ftrace/sched.pbzero.h" #include "protos/perfetto/trace/ftrace/workqueue.pbzero.h" +#include "src/trace_processor/types/version_number.h" -namespace perfetto { -namespace trace_processor { +namespace perfetto::trace_processor { namespace { @@ -57,6 +72,12 @@ struct FtraceTime { const int64_t micros; }; +Query GetArgQuery(const tables::ArgTable& table, uint32_t arg_set_id) { + Query q; + q.constraints = {table.arg_set_id().eq(arg_set_id)}; + return q; +} + class ArgsSerializer { public: ArgsSerializer(TraceProcessorContext*, @@ -72,7 +93,7 @@ class ArgsSerializer { using SerializerValueWriter = void (ArgsSerializer::*)(const Variadic&); // Arg writing functions. - void WriteArgForField(uint32_t field_id, ValueWriter writer) { + void WriteArgForField(uint32_t field_id, const ValueWriter& writer) { std::optional row = FieldIdToRow(field_id); if (!row) return; @@ -80,21 +101,23 @@ class ArgsSerializer { } void WriteArgForField(uint32_t field_id, base::StringView key, - ValueWriter writer) { + const ValueWriter& writer) { std::optional row = FieldIdToRow(field_id); if (!row) return; WriteArg(key, storage_->GetArgValue(*row), writer); } - void WriteArgAtRow(uint32_t arg_row, ValueWriter writer) { + void WriteArgAtRow(uint32_t arg_row, const ValueWriter& writer) { const auto& args = storage_->arg_table(); const auto& key = storage_->GetString(args.key()[arg_row]); WriteArg(key, storage_->GetArgValue(arg_row), writer); } - void WriteArg(base::StringView key, Variadic value, ValueWriter writer); + void WriteArg(base::StringView key, + Variadic value, + const ValueWriter& writer); // Value writing functions. - void WriteValueForField(uint32_t field_id, ValueWriter writer) { + void WriteValueForField(uint32_t field_id, const ValueWriter& writer) { std::optional row = FieldIdToRow(field_id); if (!row) return; @@ -109,7 +132,7 @@ class ArgsSerializer { PERFETTO_DFATAL("Invalid field type %d", static_cast(value.type)); } } - void WriteValue(const Variadic& variadic); + void WriteValue(const Variadic&); // The default value writer which uses the |WriteValue| function. ValueWriter DVW() { return Wrap(&ArgsSerializer::WriteValue); } @@ -130,11 +153,10 @@ class ArgsSerializer { const TraceStorage* storage_ = nullptr; TraceProcessorContext* context_ = nullptr; - ArgSetId arg_set_id_ = kInvalidArgSetId; NullTermStringView event_name_; std::vector>* field_id_to_arg_index_; - RowMap row_map_; + tables::ArgTable::ConstIterator it_; uint32_t start_row_ = 0; base::StringWriter* writer_ = nullptr; @@ -146,21 +168,16 @@ ArgsSerializer::ArgsSerializer( NullTermStringView event_name, std::vector>* field_id_to_arg_index, base::StringWriter* writer) - : context_(context), - arg_set_id_(arg_set_id), + : storage_(context->storage.get()), + context_(context), event_name_(event_name), field_id_to_arg_index_(field_id_to_arg_index), + it_(context->storage->arg_table().FilterToIterator( + GetArgQuery(context->storage->arg_table(), arg_set_id))), writer_(writer) { - storage_ = context_->storage.get(); - const auto& args = storage_->arg_table(); - const auto& set_ids = args.arg_set_id(); - // We assume that the row map is a contiguous range (which is always the case // because arg_set_ids are contiguous by definition). - Query q; - q.constraints = {set_ids.eq(arg_set_id_)}; - row_map_ = args.QueryToRowMap(q); - start_row_ = row_map_.empty() ? 0 : row_map_.Get(0); + start_row_ = it_ ? it_.row_number().row_number() : 0; // If the vector already has entries, we've previously cached the mapping // from field id to arg index. @@ -185,11 +202,13 @@ ArgsSerializer::ArgsSerializer( field_id_to_arg_index_->resize(max + 1); // Go through each field id and find the entry in the args table for that - for (uint32_t i = 1; i <= max; ++i) { - for (auto it = row_map_.IterateRows(); it; it.Next()) { - base::StringView key = args.key().GetString(it.index()); + auto it = storage_->arg_table().FilterToIterator( + GetArgQuery(context->storage->arg_table(), arg_set_id)); + for (uint32_t r = 0; it; ++it, ++r) { + for (uint32_t i = 1; i <= max; ++i) { + base::StringView key = context->storage->GetString(it.key()); if (key == descriptor->fields[i].name) { - (*field_id_to_arg_index)[i] = it.row(); + (*field_id_to_arg_index)[i] = r; break; } } @@ -197,7 +216,7 @@ ArgsSerializer::ArgsSerializer( } void ArgsSerializer::SerializeArgs() { - if (row_map_.empty()) + if (!it_) return; if (event_name_ == "sched_switch") { @@ -513,14 +532,14 @@ void ArgsSerializer::SerializeArgs() { WriteArgForField(CAT::kCommFieldNumber, DVW()); return; } - for (auto it = row_map_.IterateRows(); it; it.Next()) { - WriteArgAtRow(it.index(), DVW()); + for (; it_; ++it_) { + WriteArgAtRow(it_.row_number().row_number(), DVW()); } } void ArgsSerializer::WriteArg(base::StringView key, Variadic value, - ValueWriter writer) { + const ValueWriter& writer) { writer_->AppendChar(' '); writer_->AppendString(key.data(), key.size()); writer_->AppendChar('='); @@ -574,7 +593,7 @@ base::Status ToFtrace::Run(Context* context, sqlite3_value** argv, SqlValue& out, Destructors& destructors) { - if (argc != 1 || sqlite3_value_type(argv[0]) != SQLITE_INTEGER) { + if (argc != 1 || sqlite::value::Type(argv[0]) != sqlite::Type::kInteger) { return base::ErrStatus("Usage: to_ftrace(id)"); } uint32_t row = static_cast(sqlite3_value_int64(argv[0])); @@ -624,7 +643,7 @@ SystraceSerializer::ScopedCString SystraceSerializer::SerializeToString( &writer); serializer.SerializeArgs(); - return ScopedCString(writer.CreateStringCopy(), free); + return {writer.CreateStringCopy(), free}; } void SystraceSerializer::SerializePrefix(uint32_t raw_row, @@ -686,5 +705,4 @@ void SystraceSerializer::SerializePrefix(uint32_t raw_row, writer->AppendChar(':'); } -} // namespace trace_processor -} // namespace perfetto +} // namespace perfetto::trace_processor diff --git a/src/trace_processor/perfetto_sql/intrinsics/table_functions/experimental_flat_slice_unittest.cc b/src/trace_processor/perfetto_sql/intrinsics/table_functions/experimental_flat_slice_unittest.cc index 3a107d2fed..e3a645001c 100644 --- a/src/trace_processor/perfetto_sql/intrinsics/table_functions/experimental_flat_slice_unittest.cc +++ b/src/trace_processor/perfetto_sql/intrinsics/table_functions/experimental_flat_slice_unittest.cc @@ -57,20 +57,20 @@ class TableInseter { class TableAsserter { public: - explicit TableAsserter(Table::Iterator it) : iterator_(std::move(it)) {} + explicit TableAsserter(tables::ExperimentalFlatSliceTable::Iterator it) + : iterator_(std::move(it)) {} void NextSlice(int64_t ts, int64_t dur) { - using CI = tables::ExperimentalFlatSliceTable::ColumnIndex; ASSERT_TRUE(HasMoreSlices()); - ASSERT_EQ(iterator_.Get(CI::ts).AsLong(), ts); - ASSERT_EQ(iterator_.Get(CI::dur).AsLong(), dur); + ASSERT_EQ(iterator_.ts(), ts); + ASSERT_EQ(iterator_.dur(), dur); ++iterator_; } bool HasMoreSlices() { return bool(iterator_); } private: - Table::Iterator iterator_; + tables::ExperimentalFlatSliceTable::Iterator iterator_; }; TEST(ExperimentalFlatSlice, Smoke) { @@ -102,7 +102,7 @@ TEST(ExperimentalFlatSlice, Smoke) { auto out = ExperimentalFlatSlice::ComputeFlatSliceTable(table, &pool, 0, 400); Query q; q.orders = {out->track_id().ascending(), out->ts().ascending()}; - auto it = out->ApplyAndIterateRows(out->QueryToRowMap(q)); + auto it = out->FilterToIterator(q); TableAsserter asserter(std::move(it)); @@ -186,7 +186,7 @@ TEST(ExperimentalFlatSlice, Bounds) { ExperimentalFlatSlice::ComputeFlatSliceTable(table, &pool, start, end); Query q; q.orders = {out->track_id().ascending(), out->ts().ascending()}; - auto it = out->ApplyAndIterateRows(out->QueryToRowMap(q)); + auto it = out->FilterToIterator(q); TableAsserter asserter(std::move(it)); diff --git a/src/trace_processor/perfetto_sql/intrinsics/table_functions/experimental_slice_layout_unittest.cc b/src/trace_processor/perfetto_sql/intrinsics/table_functions/experimental_slice_layout_unittest.cc index 5df44c3c18..5735a1daf7 100644 --- a/src/trace_processor/perfetto_sql/intrinsics/table_functions/experimental_slice_layout_unittest.cc +++ b/src/trace_processor/perfetto_sql/intrinsics/table_functions/experimental_slice_layout_unittest.cc @@ -42,9 +42,9 @@ std::string ToVis(const Table& table) { using CI = tables::ExperimentalSliceLayoutTable::ColumnIndex; std::vector lines; for (auto it = table.IterateRows(); it; ++it) { - int64_t layout_depth = it.Get(CI::layout_depth).long_value; - int64_t ts = it.Get(CI::ts).long_value; - int64_t dur = it.Get(CI::dur).long_value; + int64_t layout_depth = it.Get(CI::layout_depth).AsLong(); + int64_t ts = it.Get(CI::ts).AsLong(); + int64_t dur = it.Get(CI::dur).AsLong(); const char* filter_track_ids = it.Get(CI::filter_track_ids).AsString(); if (std::string("") == filter_track_ids) { continue; diff --git a/src/trace_processor/perfetto_sql/intrinsics/table_functions/flamegraph_construction_algorithms.cc b/src/trace_processor/perfetto_sql/intrinsics/table_functions/flamegraph_construction_algorithms.cc index d47f834dca..9dd5564472 100644 --- a/src/trace_processor/perfetto_sql/intrinsics/table_functions/flamegraph_construction_algorithms.cc +++ b/src/trace_processor/perfetto_sql/intrinsics/table_functions/flamegraph_construction_algorithms.cc @@ -283,15 +283,21 @@ BuildFlamegraphTableHeapSizeAndCount( static std::unique_ptr BuildFlamegraphTableCallstackSizeAndCount( + const tables::PerfSampleTable& table, std::unique_ptr tbl, const std::vector& callsite_to_merged_callsite, - Table::Iterator it) { - for (; it; ++it) { - int64_t callsite_id = - it.Get(tables::PerfSampleTable::ColumnIndex::callsite_id).long_value; - int64_t ts = it.Get(tables::PerfSampleTable::ColumnIndex::ts).long_value; - uint32_t merged_idx = - callsite_to_merged_callsite[static_cast(callsite_id)]; + std::vector constraints, + const std::unordered_set& utids) { + Query q; + q.constraints = std::move(constraints); + for (auto it = table.FilterToIterator(q); it; ++it) { + if (utids.find(it.utid()) == utids.end()) { + continue; + } + + uint32_t callsite_id = it.callsite_id().value_or(CallsiteId(0u)).value; + int64_t ts = it.ts(); + uint32_t merged_idx = callsite_to_merged_callsite[callsite_id]; tbl->mutable_size()->Set(merged_idx, tbl->size()[merged_idx] + 1); tbl->mutable_count()->Set(merged_idx, tbl->count()[merged_idx] + 1); tbl->mutable_ts()->Set(merged_idx, ts); @@ -392,21 +398,6 @@ BuildNativeCallStackSamplingFlamegraph( cs.emplace_back(Constraint{tables::PerfSampleTable::ColumnIndex::ts, tc.op, SqlValue::Long(tc.value)}); } - std::vector cs_rows; - { - Query q; - q.constraints = cs; - auto it = storage->perf_sample_table().FilterToIterator(q); - for (; it; ++it) { - if (utids.find(it.utid()) != utids.end()) { - cs_rows.push_back(it.row_number().row_number()); - } - } - } - if (cs_rows.empty()) { - return std::make_unique( - storage->mutable_string_pool()); - } // The logic underneath is selecting a default timestamp to be used by all // frames which do not have a timestamp. The timestamp is taken from the @@ -415,7 +406,7 @@ BuildNativeCallStackSamplingFlamegraph( // the table ExperimentalFlamegraphTable in this class. int64_t default_timestamp = 0; if (!time_constraints.empty()) { - auto& tc = time_constraints[0]; + const auto& tc = time_constraints[0]; if (tc.op == FilterOp::kGt) { default_timestamp = tc.value + 1; } else if (tc.op == FilterOp::kLt) { @@ -431,10 +422,8 @@ BuildNativeCallStackSamplingFlamegraph( default_timestamp, storage->InternString("perf")); return BuildFlamegraphTableCallstackSizeAndCount( - std::move(table_and_callsites.tbl), - table_and_callsites.callsite_to_merged_callsite, - storage->perf_sample_table().ApplyAndIterateRows( - RowMap(std::move(cs_rows)))); + storage->perf_sample_table(), std::move(table_and_callsites.tbl), + table_and_callsites.callsite_to_merged_callsite, std::move(cs), utids); } } // namespace perfetto::trace_processor diff --git a/src/trace_processor/tables/BUILD.gn b/src/trace_processor/tables/BUILD.gn index 7f4f2e9b9c..72796e5507 100644 --- a/src/trace_processor/tables/BUILD.gn +++ b/src/trace_processor/tables/BUILD.gn @@ -65,6 +65,7 @@ source_set("unittests") { "../../../gn:gtest_and_gmock", "../containers", "../db", + "../db/column", ] } diff --git a/src/trace_processor/tables/macros_internal.h b/src/trace_processor/tables/macros_internal.h index 3420fb1b8f..994322d1f7 100644 --- a/src/trace_processor/tables/macros_internal.h +++ b/src/trace_processor/tables/macros_internal.h @@ -25,6 +25,7 @@ #include #include "perfetto/base/logging.h" +#include "perfetto/public/compiler.h" #include "perfetto/trace_processor/ref_counted.h" #include "src/trace_processor/containers/row_map.h" #include "src/trace_processor/containers/string_pool.h" diff --git a/src/trace_processor/tables/py_tables_benchmark.cc b/src/trace_processor/tables/py_tables_benchmark.cc index ed19da17eb..2491ca9212 100644 --- a/src/trace_processor/tables/py_tables_benchmark.cc +++ b/src/trace_processor/tables/py_tables_benchmark.cc @@ -108,7 +108,7 @@ static void BM_TableFilterRootId(benchmark::State& state) { root.Insert({}); for (auto _ : state) { - benchmark::DoNotOptimize(root.ApplyAndIterateRows(root.QueryToRowMap(q))); + benchmark::DoNotOptimize(root.FilterToIterator(q)); } } BENCHMARK(BM_TableFilterRootId)->Apply(TableFilterArgs); @@ -129,7 +129,7 @@ static void BM_TableFilterRootIdAndOther(benchmark::State& state) { } for (auto _ : state) { - benchmark::DoNotOptimize(root.ApplyAndIterateRows(root.QueryToRowMap(q))); + benchmark::DoNotOptimize(root.FilterToIterator(q)); } } BENCHMARK(BM_TableFilterRootIdAndOther)->Apply(TableFilterArgs); @@ -148,7 +148,7 @@ static void BM_TableFilterChildId(benchmark::State& state) { } for (auto _ : state) { - benchmark::DoNotOptimize(child.ApplyAndIterateRows(child.QueryToRowMap(q))); + benchmark::DoNotOptimize(child.FilterToIterator(q)); } } BENCHMARK(BM_TableFilterChildId)->Apply(TableFilterArgs); @@ -172,7 +172,7 @@ static void BM_TableFilterChildIdAndSortedInRoot(benchmark::State& state) { } for (auto _ : state) { - benchmark::DoNotOptimize(child.ApplyAndIterateRows(child.QueryToRowMap(q))); + benchmark::DoNotOptimize(child.FilterToIterator(q)); } } BENCHMARK(BM_TableFilterChildIdAndSortedInRoot)->Apply(TableFilterArgs); @@ -193,7 +193,7 @@ static void BM_TableFilterRootNonNullEqMatchMany(benchmark::State& state) { } for (auto _ : state) { - benchmark::DoNotOptimize(root.ApplyAndIterateRows(root.QueryToRowMap(q))); + benchmark::DoNotOptimize(root.FilterToIterator(q)); } } BENCHMARK(BM_TableFilterRootNonNullEqMatchMany)->Apply(TableFilterArgs); @@ -216,7 +216,7 @@ static void BM_TableFilterRootMultipleNonNull(benchmark::State& state) { } for (auto _ : state) { - benchmark::DoNotOptimize(root.ApplyAndIterateRows(root.QueryToRowMap(q))); + benchmark::DoNotOptimize(root.FilterToIterator(q)); } } BENCHMARK(BM_TableFilterRootMultipleNonNull)->Apply(TableFilterArgs); @@ -241,7 +241,7 @@ static void BM_TableFilterRootNullableEqMatchMany(benchmark::State& state) { } for (auto _ : state) { - benchmark::DoNotOptimize(root.ApplyAndIterateRows(root.QueryToRowMap(q))); + benchmark::DoNotOptimize(root.FilterToIterator(q)); } } BENCHMARK(BM_TableFilterRootNullableEqMatchMany)->Apply(TableFilterArgs); @@ -265,7 +265,7 @@ static void BM_TableFilterChildNonNullEqMatchMany(benchmark::State& state) { } for (auto _ : state) { - benchmark::DoNotOptimize(child.ApplyAndIterateRows(child.QueryToRowMap(q))); + benchmark::DoNotOptimize(child.FilterToIterator(q)); } } BENCHMARK(BM_TableFilterChildNonNullEqMatchMany)->Apply(TableFilterArgs); @@ -292,7 +292,7 @@ static void BM_TableFilterChildNullableEqMatchMany(benchmark::State& state) { } for (auto _ : state) { - benchmark::DoNotOptimize(child.ApplyAndIterateRows(child.QueryToRowMap(q))); + benchmark::DoNotOptimize(child.FilterToIterator(q)); } } BENCHMARK(BM_TableFilterChildNullableEqMatchMany)->Apply(TableFilterArgs); @@ -317,7 +317,7 @@ static void BM_TableFilterChildNonNullEqMatchManyInParent( } for (auto _ : state) { - benchmark::DoNotOptimize(child.ApplyAndIterateRows(child.QueryToRowMap(q))); + benchmark::DoNotOptimize(child.FilterToIterator(q)); } } BENCHMARK(BM_TableFilterChildNonNullEqMatchManyInParent) @@ -343,7 +343,7 @@ static void BM_TableFilterChildNullableEqMatchManyInParent( Query q; q.constraints = {child.root_nullable().eq(1)}; for (auto _ : state) { - benchmark::DoNotOptimize(child.ApplyAndIterateRows(child.QueryToRowMap(q))); + benchmark::DoNotOptimize(child.FilterToIterator(q)); } } BENCHMARK(BM_TableFilterChildNullableEqMatchManyInParent) @@ -364,7 +364,7 @@ static void BM_TableFilterParentSortedEq(benchmark::State& state) { Query q; q.constraints = {root.root_sorted().eq(22)}; for (auto _ : state) { - benchmark::DoNotOptimize(root.ApplyAndIterateRows(root.QueryToRowMap(q))); + benchmark::DoNotOptimize(root.FilterToIterator(q)); } } BENCHMARK(BM_TableFilterParentSortedEq)->Apply(TableFilterArgs); @@ -391,7 +391,7 @@ static void BM_TableFilterParentSortedAndOther(benchmark::State& state) { q.constraints = {root.root_sorted().eq(last_group), root.root_non_null().eq(size - 1)}; for (auto _ : state) { - benchmark::DoNotOptimize(root.ApplyAndIterateRows(root.QueryToRowMap(q))); + benchmark::DoNotOptimize(root.FilterToIterator(q)); } } BENCHMARK(BM_TableFilterParentSortedAndOther)->Apply(TableFilterArgs); @@ -413,7 +413,7 @@ static void BM_TableFilterChildSortedEq(benchmark::State& state) { Query q; q.constraints = {child.child_sorted().eq(22)}; for (auto _ : state) { - benchmark::DoNotOptimize(child.ApplyAndIterateRows(child.QueryToRowMap(q))); + benchmark::DoNotOptimize(child.FilterToIterator(q)); } } BENCHMARK(BM_TableFilterChildSortedEq)->Apply(TableFilterArgs); @@ -438,7 +438,7 @@ static void BM_TableFilterChildSortedEqInParent(benchmark::State& state) { Query q; q.constraints = {child.root_sorted().eq(22)}; for (auto _ : state) { - benchmark::DoNotOptimize(child.ApplyAndIterateRows(child.QueryToRowMap(q))); + benchmark::DoNotOptimize(child.FilterToIterator(q)); } } BENCHMARK(BM_TableFilterChildSortedEqInParent)->Apply(TableFilterArgs); @@ -461,7 +461,7 @@ static void BM_TableSortRootNonNull(benchmark::State& state) { Query q; q.orders = {root.root_non_null().ascending()}; for (auto _ : state) { - benchmark::DoNotOptimize(root.ApplyAndIterateRows(root.QueryToRowMap(q))); + benchmark::DoNotOptimize(root.FilterToIterator(q)); } } BENCHMARK(BM_TableSortRootNonNull)->Apply(TableSortArgs); @@ -485,7 +485,7 @@ static void BM_TableSortRootNullable(benchmark::State& state) { Query q; q.orders = {root.root_nullable().ascending()}; for (auto _ : state) { - benchmark::DoNotOptimize(root.ApplyAndIterateRows(root.QueryToRowMap(q))); + benchmark::DoNotOptimize(root.FilterToIterator(q)); } } BENCHMARK(BM_TableSortRootNullable)->Apply(TableSortArgs); @@ -515,7 +515,7 @@ static void BM_TableSortChildNonNullInParent(benchmark::State& state) { Query q; q.orders = {child.root_non_null().ascending()}; for (auto _ : state) { - benchmark::DoNotOptimize(child.ApplyAndIterateRows(child.QueryToRowMap(q))); + benchmark::DoNotOptimize(child.FilterToIterator(q)); } } BENCHMARK(BM_TableSortChildNonNullInParent)->Apply(TableSortArgs); @@ -547,7 +547,7 @@ static void BM_TableSortChildNullableInParent(benchmark::State& state) { Query q; q.orders = {child.root_nullable().ascending()}; for (auto _ : state) { - benchmark::DoNotOptimize(child.ApplyAndIterateRows(child.QueryToRowMap(q))); + benchmark::DoNotOptimize(child.FilterToIterator(q)); } } BENCHMARK(BM_TableSortChildNullableInParent)->Apply(TableSortArgs); diff --git a/src/trace_processor/tables/py_tables_unittest.cc b/src/trace_processor/tables/py_tables_unittest.cc index 4d2186fd62..fb128f0bf6 100644 --- a/src/trace_processor/tables/py_tables_unittest.cc +++ b/src/trace_processor/tables/py_tables_unittest.cc @@ -20,6 +20,7 @@ #include "src/trace_processor/containers/string_pool.h" #include "src/trace_processor/db/column.h" +#include "src/trace_processor/db/column/types.h" #include "src/trace_processor/db/column_storage.h" #include "src/trace_processor/tables/py_tables_unittest_py.h" @@ -245,56 +246,47 @@ TEST_F(PyTablesUnittest, SetIdColumns) { static constexpr uint32_t kFilterArgSetId = 1; Query q; q.constraints = {table.arg_set_id().eq(kFilterArgSetId)}; - auto res = table.QueryToRowMap(q); - ASSERT_TRUE(res.empty()); + auto res = table.FilterToIterator(q); + ASSERT_TRUE(!res); } { static constexpr uint32_t kFilterArgSetId = 9; Query q; q.constraints = {table.arg_set_id().eq(kFilterArgSetId)}; - auto res = table.QueryToRowMap(q); - ASSERT_TRUE(res.empty()); + auto it = table.FilterToIterator(q); + ASSERT_TRUE(!it); } - auto arg_set_id_col_idx = - static_cast(TestArgsTable::ColumnIndex::arg_set_id); - // Verify that filtering equality for real arg set ids works as expected. { static constexpr uint32_t kFilterArgSetId = 4; Query q; q.constraints = {table.arg_set_id().eq(kFilterArgSetId)}; - auto res = table.QueryToRowMap(q); - ASSERT_EQ(res.size(), 4u); - for (auto it = table.ApplyAndIterateRows(std::move(res)); it; ++it) { - auto arg_set_id = - static_cast(it.Get(arg_set_id_col_idx).AsLong()); - ASSERT_EQ(arg_set_id, kFilterArgSetId); + uint32_t cnt = 0; + for (auto it = table.FilterToIterator(q); it; ++it, ++cnt) { + ASSERT_EQ(it.arg_set_id(), kFilterArgSetId); } + ASSERT_EQ(cnt, 4u); } { static constexpr uint32_t kFilterArgSetId = 0; Query q; q.constraints = {table.arg_set_id().eq(kFilterArgSetId)}; - auto res = table.QueryToRowMap(q); - ASSERT_EQ(res.size(), 2u); - for (auto it = table.ApplyAndIterateRows(std::move(res)); it; ++it) { - auto arg_set_id = - static_cast(it.Get(arg_set_id_col_idx).AsLong()); - ASSERT_EQ(arg_set_id, kFilterArgSetId); + uint32_t cnt = 0; + for (auto it = table.FilterToIterator(q); it; ++it, ++cnt) { + ASSERT_EQ(it.arg_set_id(), kFilterArgSetId); } + ASSERT_EQ(cnt, 2u); } { static constexpr uint32_t kFilterArgSetId = 8; Query q; q.constraints = {table.arg_set_id().eq(kFilterArgSetId)}; - auto res = table.QueryToRowMap(q); - ASSERT_EQ(res.size(), 1u); - for (auto it = table.ApplyAndIterateRows(std::move(res)); it; ++it) { - auto arg_set_id = - static_cast(it.Get(arg_set_id_col_idx).AsLong()); - ASSERT_EQ(arg_set_id, kFilterArgSetId); + uint32_t cnt = 0; + for (auto it = table.FilterToIterator(q); it; ++it, ++cnt) { + ASSERT_EQ(it.arg_set_id(), kFilterArgSetId); } + ASSERT_EQ(cnt, 1u); } // Verify that filtering equality for arg set ids after filtering another @@ -304,13 +296,11 @@ TEST_F(PyTablesUnittest, SetIdColumns) { Query q; q.constraints = {table.int_value().eq(200), table.arg_set_id().eq(kFilterArgSetId)}; - auto res = table.QueryToRowMap(q); - ASSERT_EQ(res.size(), 2u); - for (auto it = table.ApplyAndIterateRows(std::move(res)); it; ++it) { - uint32_t arg_set_id = - static_cast(it.Get(arg_set_id_col_idx).AsLong()); - ASSERT_EQ(arg_set_id, kFilterArgSetId); + uint32_t cnt = 0; + for (auto it = table.FilterToIterator(q); it; ++it, ++cnt) { + ASSERT_EQ(it.arg_set_id(), kFilterArgSetId); } + ASSERT_EQ(cnt, 2u); } } diff --git a/test/gtest_and_gmock.h b/test/gtest_and_gmock.h index 24d8bedda8..69c112a56c 100644 --- a/test/gtest_and_gmock.h +++ b/test/gtest_and_gmock.h @@ -46,12 +46,14 @@ #endif // defined(__clang__) -#include // IWYU pragma: export -#include // IWYU pragma: export -#include // IWYU pragma: export -#include // IWYU pragma: export -#include // IWYU pragma: export -#include // IWYU pragma: export +#include // IWYU pragma: export +#include // IWYU pragma: export +#include // IWYU pragma: export +#include // IWYU pragma: export +#include // IWYU pragma: export +#include // IWYU pragma: export +#include // IWYU pragma: export +#include // IWYU pragma: export #if defined(__GNUC__) || defined(__clang__) #pragma GCC diagnostic pop