Skip to content

Commit

Permalink
trace_processor: add helper methods for adding constraints on typed cols
Browse files Browse the repository at this point in the history 
This CL reduces the boilerplate of setting up a constraint when we know
the type of the column. Instead of going through SqlValue, since we know
the type, we can provide a type-safe method to generate the constraint.

Change-Id: If7677ca52cbc919ee21d39de363b929abd661e2f
  • Loading branch information
@LalitMaganti
LalitMaganti committed Dec 2, 2019
1 parent 7724a94 commit a5287b0
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 60 deletions.
22 changes: 16 additions & 6 deletions src/trace_processor/db/column.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -269,22 +269,22 @@ class Column {
}

// Returns a Constraint for each type of filter operation for this Column.
Constraint eq(SqlValue value) const {
Constraint eq_value(SqlValue value) const {
return Constraint{col_idx_, FilterOp::kEq, value};
}
Constraint gt(SqlValue value) const {
Constraint gt_value(SqlValue value) const {
return Constraint{col_idx_, FilterOp::kGt, value};
}
Constraint lt(SqlValue value) const {
Constraint lt_value(SqlValue value) const {
return Constraint{col_idx_, FilterOp::kLt, value};
}
Constraint ne(SqlValue value) const {
Constraint ne_value(SqlValue value) const {
return Constraint{col_idx_, FilterOp::kNe, value};
}
Constraint ge(SqlValue value) const {
Constraint ge_value(SqlValue value) const {
return Constraint{col_idx_, FilterOp::kGe, value};
}
Constraint le(SqlValue value) const {
Constraint le_value(SqlValue value) const {
return Constraint{col_idx_, FilterOp::kLe, value};
}
Constraint is_not_null() const {
Expand Down Expand Up @@ -318,6 +318,16 @@ class Column {
return *static_cast<const SparseVector<T>*>(sparse_vector_);
}

template <typename T>
static SqlValue NumericToSqlValue(T value) {
if (std::is_same<T, double>::value) {
return SqlValue::Double(value);
} else if (std::is_convertible<T, int64_t>::value) {
return SqlValue::Long(value);
}
PERFETTO_FATAL("Invalid type");
}

private:
enum class ColumnType {
// Standard primitive types.
Expand Down
57 changes: 42 additions & 15 deletions src/trace_processor/db/typed_column.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,16 @@
namespace perfetto {
namespace trace_processor {

// Represents a column containing ids.
struct IdColumn : public Column {
Constraint eq(uint32_t v) const { return eq_value(SqlValue::Long(v)); }
Constraint gt(uint32_t v) const { return gt_value(SqlValue::Long(v)); }
Constraint lt(uint32_t v) const { return lt_value(SqlValue::Long(v)); }
Constraint ne(uint32_t v) const { return ne_value(SqlValue::Long(v)); }
Constraint ge(uint32_t v) const { return ge_value(SqlValue::Long(v)); }
Constraint le(uint32_t v) const { return le_value(SqlValue::Long(v)); }
};

// Represents a column containing data with the given type T.
//
// This class exists as a memberless subclass of Column (i.e. sizeof(Column) ==
Expand All @@ -35,10 +45,17 @@ struct TypedColumn : public Column {
T operator[](uint32_t row) const {
return sparse_vector<T>().GetNonNull(row_map().Get(row));
}
void Set(uint32_t row, T value) {
mutable_sparse_vector<T>()->Set(row_map().Get(row), value);
void Set(uint32_t row, T v) {
mutable_sparse_vector<T>()->Set(row_map().Get(row), v);
}
void Append(T value) { mutable_sparse_vector<T>()->Append(value); }
void Append(T v) { mutable_sparse_vector<T>()->Append(v); }

Constraint eq(T v) const { return eq_value(NumericToSqlValue(v)); }
Constraint gt(T v) const { return gt_value(NumericToSqlValue(v)); }
Constraint lt(T v) const { return lt_value(NumericToSqlValue(v)); }
Constraint ne(T v) const { return ne_value(NumericToSqlValue(v)); }
Constraint ge(T v) const { return ge_value(NumericToSqlValue(v)); }
Constraint le(T v) const { return le_value(NumericToSqlValue(v)); }

static constexpr uint32_t default_flags() { return Flag::kNonNull; }
};
Expand All @@ -50,13 +67,17 @@ struct TypedColumn<base::Optional<T>> : public Column {
base::Optional<T> operator[](uint32_t row) const {
return sparse_vector<T>().Get(row_map().Get(row));
}
void Set(uint32_t row, T value) {
mutable_sparse_vector<T>()->Set(row_map().Get(row), value);
void Set(uint32_t row, T v) {
mutable_sparse_vector<T>()->Set(row_map().Get(row), v);
}
void Append(base::Optional<T> v) { mutable_sparse_vector<T>()->Append(v); }

void Append(base::Optional<T> value) {
mutable_sparse_vector<T>()->Append(value);
}
Constraint eq(T v) const { return eq_value(NumericToSqlValue(v)); }
Constraint gt(T v) const { return gt_value(NumericToSqlValue(v)); }
Constraint lt(T v) const { return lt_value(NumericToSqlValue(v)); }
Constraint ne(T v) const { return ne_value(NumericToSqlValue(v)); }
Constraint ge(T v) const { return ge_value(NumericToSqlValue(v)); }
Constraint le(T v) const { return le_value(NumericToSqlValue(v)); }

static constexpr uint32_t default_flags() { return Flag::kNoFlag; }
};
Expand All @@ -71,20 +92,27 @@ struct TypedColumn<StringPool::Id> : public Column {
NullTermStringView GetString(uint32_t row) const {
return GetStringPoolStringAtIdx(row_map().Get(row));
}
void Set(uint32_t row, StringPool::Id value) {
mutable_sparse_vector<StringPool::Id>()->Set(row_map().Get(row), value);
void Set(uint32_t row, StringPool::Id v) {
mutable_sparse_vector<StringPool::Id>()->Set(row_map().Get(row), v);
}
void Append(StringPool::Id value) {
mutable_sparse_vector<StringPool::Id>()->Append(value);
void Append(StringPool::Id v) {
mutable_sparse_vector<StringPool::Id>()->Append(v);
}

Constraint eq(const char* v) const { return eq_value(SqlValue::String(v)); }
Constraint gt(const char* v) const { return gt_value(SqlValue::String(v)); }
Constraint lt(const char* v) const { return lt_value(SqlValue::String(v)); }
Constraint ne(const char* v) const { return ne_value(SqlValue::String(v)); }
Constraint ge(const char* v) const { return ge_value(SqlValue::String(v)); }
Constraint le(const char* v) const { return le_value(SqlValue::String(v)); }

static constexpr uint32_t default_flags() { return Flag::kNonNull; }
};

template <>
struct TypedColumn<base::Optional<StringPool::Id>>
: public TypedColumn<StringPool::Id> {
void Append(base::Optional<StringPool::Id> value) {
void Append(base::Optional<StringPool::Id> v) {
// Since StringPool::Id == 0 is always treated as null, rewrite
// base::nullopt -> 0 to remove an extra check at filter time for
// base::nullopt. Instead, that code can assume that the SparseVector layer
Expand All @@ -93,8 +121,7 @@ struct TypedColumn<base::Optional<StringPool::Id>>
// TODO(lalitm): remove this special casing if we migrate all tables over
// to macro tables and find that we can remove support for null stringids
// in the stringpool.
return TypedColumn<StringPool::Id>::Append(value ? *value
: StringPool::Id(0u));
return TypedColumn<StringPool::Id>::Append(v ? *v : StringPool::Id(0u));
}

static constexpr uint32_t default_flags() { return Flag::kNonNull; }
Expand Down
29 changes: 10 additions & 19 deletions src/trace_processor/tables/macros_benchmark.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ static void BM_TableFilterIdColumn(benchmark::State& state) {
root.Insert({});

for (auto _ : state) {
benchmark::DoNotOptimize(root.Filter({root.id().eq(SqlValue::Long(30))}));
benchmark::DoNotOptimize(root.Filter({root.id().eq(30)}));
}
}
BENCHMARK(BM_TableFilterIdColumn)->Apply(TableFilterArgs);
Expand All @@ -136,8 +136,7 @@ static void BM_TableFilterRootNonNullEqMatchMany(benchmark::State& state) {
}

for (auto _ : state) {
benchmark::DoNotOptimize(
root.Filter({root.root_non_null().eq(SqlValue::Long(0))}));
benchmark::DoNotOptimize(root.Filter({root.root_non_null().eq(0)}));
}
}
BENCHMARK(BM_TableFilterRootNonNullEqMatchMany)->Apply(TableFilterArgs);
Expand All @@ -160,8 +159,7 @@ static void BM_TableFilterRootNullableEqMatchMany(benchmark::State& state) {
}

for (auto _ : state) {
benchmark::DoNotOptimize(
root.Filter({root.root_nullable().eq(SqlValue::Long(1))}));
benchmark::DoNotOptimize(root.Filter({root.root_nullable().eq(1)}));
}
}
BENCHMARK(BM_TableFilterRootNullableEqMatchMany)->Apply(TableFilterArgs);
Expand All @@ -183,8 +181,7 @@ static void BM_TableFilterChildNonNullEqMatchMany(benchmark::State& state) {
}

for (auto _ : state) {
benchmark::DoNotOptimize(
child.Filter({child.child_non_null().eq(SqlValue::Long(0))}));
benchmark::DoNotOptimize(child.Filter({child.child_non_null().eq(0)}));
}
}
BENCHMARK(BM_TableFilterChildNonNullEqMatchMany)->Apply(TableFilterArgs);
Expand All @@ -209,8 +206,7 @@ static void BM_TableFilterChildNullableEqMatchMany(benchmark::State& state) {
}

for (auto _ : state) {
benchmark::DoNotOptimize(
child.Filter({child.child_nullable().eq(SqlValue::Long(1))}));
benchmark::DoNotOptimize(child.Filter({child.child_nullable().eq(1)}));
}
}
BENCHMARK(BM_TableFilterChildNullableEqMatchMany)->Apply(TableFilterArgs);
Expand All @@ -233,8 +229,7 @@ static void BM_TableFilterChildNonNullEqMatchManyInParent(
}

for (auto _ : state) {
benchmark::DoNotOptimize(
child.Filter({child.root_non_null().eq(SqlValue::Long(0))}));
benchmark::DoNotOptimize(child.Filter({child.root_non_null().eq(0)}));
}
}
BENCHMARK(BM_TableFilterChildNonNullEqMatchManyInParent)
Expand All @@ -258,8 +253,7 @@ static void BM_TableFilterChildNullableEqMatchManyInParent(
}

for (auto _ : state) {
benchmark::DoNotOptimize(
child.Filter({child.root_nullable().eq(SqlValue::Long(1))}));
benchmark::DoNotOptimize(child.Filter({child.root_nullable().eq(1)}));
}
}
BENCHMARK(BM_TableFilterChildNullableEqMatchManyInParent)
Expand All @@ -278,8 +272,7 @@ static void BM_TableFilterParentSortedEq(benchmark::State& state) {
}

for (auto _ : state) {
benchmark::DoNotOptimize(
root.Filter({root.root_sorted().eq(SqlValue::Long(22))}));
benchmark::DoNotOptimize(root.Filter({root.root_sorted().eq(22)}));
}
}
BENCHMARK(BM_TableFilterParentSortedEq)->Apply(TableFilterArgs);
Expand All @@ -299,8 +292,7 @@ static void BM_TableFilterChildSortedEq(benchmark::State& state) {
}

for (auto _ : state) {
benchmark::DoNotOptimize(
child.Filter({child.child_sorted().eq(SqlValue::Long(22))}));
benchmark::DoNotOptimize(child.Filter({child.child_sorted().eq(22)}));
}
}
BENCHMARK(BM_TableFilterChildSortedEq)->Apply(TableFilterArgs);
Expand All @@ -323,8 +315,7 @@ static void BM_TableFilterChildSortedEqInParent(benchmark::State& state) {
}

for (auto _ : state) {
benchmark::DoNotOptimize(
child.Filter({child.root_sorted().eq(SqlValue::Long(22))}));
benchmark::DoNotOptimize(child.Filter({child.root_sorted().eq(22)}));
}
}
BENCHMARK(BM_TableFilterChildSortedEqInParent)->Apply(TableFilterArgs);
Expand Down
5 changes: 3 additions & 2 deletions src/trace_processor/tables/macros_internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -282,8 +282,9 @@ class MacroTable : public Table {
return id; \
} \
\
const Column& id() const { \
return columns_[static_cast<uint32_t>(ColumnIndex::id)]; \
const IdColumn& id() const { \
return static_cast<const IdColumn&>( \
columns_[static_cast<uint32_t>(ColumnIndex::id)]); \
} \
\
const TypedColumn<StringPool::Id>& type() const { \
Expand Down
35 changes: 17 additions & 18 deletions src/trace_processor/tables/macros_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,34 +148,34 @@ TEST_F(TableMacrosUnittest, NullableLongComparision) {
ASSERT_EQ(dur->Get(1).long_value, 101);
ASSERT_EQ(dur->Get(2).long_value, 200);

out = slice_.Filter({slice_.dur().lt(SqlValue::Long(101))});
out = slice_.Filter({slice_.dur().lt(101)});
dur = &out.GetColumn(*out.FindColumnIdxByName("dur"));
ASSERT_EQ(out.size(), 1u);
ASSERT_EQ(dur->Get(0).long_value, 100);

out = slice_.Filter({slice_.dur().eq(SqlValue::Long(101))});
out = slice_.Filter({slice_.dur().eq(101)});
dur = &out.GetColumn(*out.FindColumnIdxByName("dur"));
ASSERT_EQ(out.size(), 1u);
ASSERT_EQ(dur->Get(0).long_value, 101);

out = slice_.Filter({slice_.dur().gt(SqlValue::Long(101))});
out = slice_.Filter({slice_.dur().gt(101)});
dur = &out.GetColumn(*out.FindColumnIdxByName("dur"));
ASSERT_EQ(out.size(), 1u);
ASSERT_EQ(dur->Get(0).long_value, 200);

out = slice_.Filter({slice_.dur().ne(SqlValue::Long(100))});
out = slice_.Filter({slice_.dur().ne(100)});
dur = &out.GetColumn(*out.FindColumnIdxByName("dur"));
ASSERT_EQ(out.size(), 2u);
ASSERT_EQ(dur->Get(0).long_value, 101);
ASSERT_EQ(dur->Get(1).long_value, 200);

out = slice_.Filter({slice_.dur().le(SqlValue::Long(101))});
out = slice_.Filter({slice_.dur().le(101)});
dur = &out.GetColumn(*out.FindColumnIdxByName("dur"));
ASSERT_EQ(out.size(), 2u);
ASSERT_EQ(dur->Get(0).long_value, 100);
ASSERT_EQ(dur->Get(1).long_value, 101);

out = slice_.Filter({slice_.dur().ge(SqlValue::Long(101))});
out = slice_.Filter({slice_.dur().ge(101)});
dur = &out.GetColumn(*out.FindColumnIdxByName("dur"));
ASSERT_EQ(out.size(), 2u);
ASSERT_EQ(dur->Get(0).long_value, 101);
Expand All @@ -197,13 +197,13 @@ TEST_F(TableMacrosUnittest, NullableLongCompareWrongType) {

slice_.Insert({});

Table out = slice_.Filter({slice_.dur().ne(SqlValue())});
Table out = slice_.Filter({slice_.dur().ne_value(SqlValue())});
ASSERT_EQ(out.size(), 0u);

out = slice_.Filter({slice_.dur().eq(SqlValue::String("100"))});
out = slice_.Filter({slice_.dur().eq_value(SqlValue::String("100"))});
ASSERT_EQ(out.size(), 0u);

out = slice_.Filter({slice_.dur().eq(SqlValue::Double(100.0))});
out = slice_.Filter({slice_.dur().eq_value(SqlValue::Double(100.0))});
ASSERT_EQ(out.size(), 0u);
}

Expand Down Expand Up @@ -231,33 +231,33 @@ TEST_F(TableMacrosUnittest, StringComparision) {
ASSERT_STREQ(end_state->Get(0).string_value, "R");
ASSERT_STREQ(end_state->Get(1).string_value, "D");

out = cpu_slice_.Filter({cpu_slice_.end_state().lt(SqlValue::String("R"))});
out = cpu_slice_.Filter({cpu_slice_.end_state().lt("R")});
end_state = &out.GetColumn(*out.FindColumnIdxByName("end_state"));
ASSERT_EQ(out.size(), 1u);
ASSERT_STREQ(end_state->Get(0).string_value, "D");

out = cpu_slice_.Filter({cpu_slice_.end_state().eq(SqlValue::String("D"))});
out = cpu_slice_.Filter({cpu_slice_.end_state().eq("D")});
end_state = &out.GetColumn(*out.FindColumnIdxByName("end_state"));
ASSERT_EQ(out.size(), 1u);
ASSERT_STREQ(end_state->Get(0).string_value, "D");

out = cpu_slice_.Filter({cpu_slice_.end_state().gt(SqlValue::String("D"))});
out = cpu_slice_.Filter({cpu_slice_.end_state().gt("D")});
end_state = &out.GetColumn(*out.FindColumnIdxByName("end_state"));
ASSERT_EQ(out.size(), 1u);
ASSERT_STREQ(end_state->Get(0).string_value, "R");

out = cpu_slice_.Filter({cpu_slice_.end_state().ne(SqlValue::String("D"))});
out = cpu_slice_.Filter({cpu_slice_.end_state().ne("D")});
end_state = &out.GetColumn(*out.FindColumnIdxByName("end_state"));
ASSERT_EQ(out.size(), 1u);
ASSERT_STREQ(end_state->Get(0).string_value, "R");

out = cpu_slice_.Filter({cpu_slice_.end_state().le(SqlValue::String("R"))});
out = cpu_slice_.Filter({cpu_slice_.end_state().le("R")});
end_state = &out.GetColumn(*out.FindColumnIdxByName("end_state"));
ASSERT_EQ(out.size(), 2u);
ASSERT_STREQ(end_state->Get(0).string_value, "R");
ASSERT_STREQ(end_state->Get(1).string_value, "D");

out = cpu_slice_.Filter({cpu_slice_.end_state().ge(SqlValue::String("D"))});
out = cpu_slice_.Filter({cpu_slice_.end_state().ge("D")});
end_state = &out.GetColumn(*out.FindColumnIdxByName("end_state"));
ASSERT_EQ(out.size(), 2u);
ASSERT_STREQ(end_state->Get(0).string_value, "R");
Expand All @@ -274,9 +274,8 @@ TEST_F(TableMacrosUnittest, FilterIdThenOther) {
cpu_slice_.Insert(row);

auto out =
cpu_slice_.Filter({cpu_slice_.id().eq(SqlValue::Long(0)),
cpu_slice_.end_state().eq(SqlValue::String("D")),
cpu_slice_.cpu().eq(SqlValue::Long(1))});
cpu_slice_.Filter({cpu_slice_.id().eq(0), cpu_slice_.end_state().eq("D"),
cpu_slice_.cpu().eq(1)});
const auto& end_state = out.GetColumn(*out.FindColumnIdxByName("end_state"));
const auto& cpu = out.GetColumn(*out.FindColumnIdxByName("cpu"));

Expand Down

0 comments on commit a5287b0

Please sign in to comment.