Skip to content

Commit

Permalink
tp: remove external uses of private Table methods
Browse files Browse the repository at this point in the history
Migrate callers to other, proper public methods.

Change-Id: I74a43aa997825dd3cee0ffb560af9277b8d644d1
  • Loading branch information
LalitMaganti committed Aug 9, 2024
1 parent a3c586f commit a683425
Show file tree
Hide file tree
Showing 13 changed files with 278 additions and 248 deletions.
58 changes: 33 additions & 25 deletions src/trace_processor/db/query_executor_benchmark.cc
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,14 @@ std::vector<std::string> ReadCSV(benchmark::State& state,
return base::SplitString(table_csv, "\n");
}

template <typename It>
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));
Expand Down Expand Up @@ -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<double>(table.table_.row_count()),
benchmark::Counter::kIsIterationInvariantRate |
benchmark::Counter::kInvert);
state.counters["s/out"] = benchmark::Counter(
static_cast<double>(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,
Expand All @@ -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<double>(table.table_.row_count()),
benchmark::Counter::kIsIterationInvariantRate |
benchmark::Counter::kInvert);
state.counters["s/out"] = benchmark::Counter(
static_cast<double>(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<double>(table.table_.row_count()),
benchmark::Counter::kIsIterationInvariantRate |
benchmark::Counter::kInvert);
state.counters["s/out"] = benchmark::Counter(
static_cast<double>(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,
Expand Down Expand Up @@ -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<double>(table.table_.row_count()),
benchmark::Counter::kIsIterationInvariantRate |
benchmark::Counter::kInvert);
state.counters["s/out"] = benchmark::Counter(
static_cast<double>(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);

Expand All @@ -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<double>(table.table_.row_count()),
benchmark::Counter::kIsIterationInvariantRate |
benchmark::Counter::kInvert);
state.counters["s/out"] = benchmark::Counter(
static_cast<double>(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);

Expand Down
19 changes: 10 additions & 9 deletions src/trace_processor/db/table.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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<OrderedIndices> GetIndex(
const std::vector<uint32_t>& cols) const {
for (const auto& idx : indexes_) {
Expand Down Expand Up @@ -206,8 +201,9 @@ class Table {
}

uint32_t row_count() const { return row_count_; }
StringPool* string_pool() const { return string_pool_; }
const std::vector<ColumnLegacy>& columns() const { return columns_; }
StringPool* string_pool() const { return string_pool_; }

const std::vector<RefPtr<column::StorageLayer>>& storage_layers() const {
return storage_layers_;
}
Expand Down Expand Up @@ -253,6 +249,7 @@ class Table {

private:
friend class ColumnLegacy;
friend class QueryExecutor;

struct ColumnIndex {
std::string name;
Expand All @@ -272,6 +269,10 @@ class Table {
RowMap ApplyIdJoinConstraints(const std::vector<Constraint>&,
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<ColumnStorageOverlay> overlays_;
Expand Down
Loading

0 comments on commit a683425

Please sign in to comment.