Merge "[ui] Pass canvas size into Track.render() functions." into main
diff --git a/Android.bp b/Android.bp
index ef5737b..cebcd7b 100644
--- a/Android.bp
+++ b/Android.bp
@@ -10983,6 +10983,7 @@
"src/trace_processor/db/storage/set_id_storage.cc",
"src/trace_processor/db/storage/storage.cc",
"src/trace_processor/db/storage/string_storage.cc",
+ "src/trace_processor/db/storage/utils.cc",
],
}
diff --git a/BUILD b/BUILD
index 5c8fbe6..f3d4584 100644
--- a/BUILD
+++ b/BUILD
@@ -1321,6 +1321,7 @@
"src/trace_processor/db/storage/string_storage.cc",
"src/trace_processor/db/storage/string_storage.h",
"src/trace_processor/db/storage/types.h",
+ "src/trace_processor/db/storage/utils.cc",
"src/trace_processor/db/storage/utils.h",
],
)
diff --git a/src/trace_processor/db/query_executor.cc b/src/trace_processor/db/query_executor.cc
index d0ef7ed..cb9a2cf 100644
--- a/src/trace_processor/db/query_executor.cc
+++ b/src/trace_processor/db/query_executor.cc
@@ -178,9 +178,11 @@
col.type() == SqlValue::kLong && c.value.type == SqlValue::kDouble;
bool double_with_int =
col.type() == SqlValue::kDouble && c.value.type == SqlValue::kLong;
- use_legacy = use_legacy ||
- (c.op != FilterOp::kIsNull && c.op != FilterOp::kIsNotNull &&
- (int_with_double || double_with_int));
+ bool double_int_enabled_col_type = col.IsId() || col.IsSetId();
+ use_legacy =
+ use_legacy ||
+ (!double_int_enabled_col_type && c.op != FilterOp::kIsNull &&
+ c.op != FilterOp::kIsNotNull && (int_with_double || double_with_int));
// Extrinsically sorted columns.
use_legacy = use_legacy ||
diff --git a/src/trace_processor/db/query_executor_benchmark.cc b/src/trace_processor/db/query_executor_benchmark.cc
index eb23903..38feabd 100644
--- a/src/trace_processor/db/query_executor_benchmark.cc
+++ b/src/trace_processor/db/query_executor_benchmark.cc
@@ -403,6 +403,24 @@
}
BENCHMARK(BM_QEDenseNullFilterIsNull)->ArgsProduct({{DB::V1, DB::V2}});
+static void BM_QEIdColumnWithIntAsDouble(benchmark::State& state) {
+ SliceTableForBenchmark table(state);
+ Constraint c{table.table_.track_id().index_in_table(), FilterOp::kEq,
+ SqlValue::Double(100)};
+ BenchmarkSliceTable(state, table, {c});
+}
+
+BENCHMARK(BM_QEIdColumnWithIntAsDouble)->ArgsProduct({{DB::V1, DB::V2}});
+
+static void BM_QEIdColumnWithDouble(benchmark::State& state) {
+ SliceTableForBenchmark table(state);
+ Constraint c{table.table_.track_id().index_in_table(), FilterOp::kEq,
+ SqlValue::Double(100.5)};
+ BenchmarkSliceTable(state, table, {c});
+}
+
+BENCHMARK(BM_QEIdColumnWithDouble)->ArgsProduct({{DB::V1, DB::V2}});
+
} // namespace
} // namespace trace_processor
} // namespace perfetto
diff --git a/src/trace_processor/db/query_executor_unittest.cc b/src/trace_processor/db/query_executor_unittest.cc
index 1f73314..7f2dfcc 100644
--- a/src/trace_processor/db/query_executor_unittest.cc
+++ b/src/trace_processor/db/query_executor_unittest.cc
@@ -560,6 +560,29 @@
ASSERT_EQ(res.size(), 0u);
}
+TEST(QueryExecutor, MismatchedTypeIdWithDouble) {
+ IdStorage storage(5);
+
+ // Filter.
+ Constraint c{0, FilterOp::kGe, SqlValue::Double(1.5)};
+ QueryExecutor exec({&storage}, 5);
+ RowMap res = exec.Filter({c});
+
+ ASSERT_EQ(res.size(), 3u);
+}
+
+TEST(QueryExecutor, MismatchedTypeSetIdWithDouble) {
+ std::vector<uint32_t> storage_data{0, 0, 0, 3, 3, 3, 6, 6, 6, 9, 9, 9};
+ SetIdStorage storage(&storage_data);
+
+ // Filter.
+ Constraint c{0, FilterOp::kGe, SqlValue::Double(1.5)};
+ QueryExecutor exec({&storage}, storage.size());
+ RowMap res = exec.Filter({c});
+
+ ASSERT_EQ(res.size(), 9u);
+}
+
#if !PERFETTO_BUILDFLAG(PERFETTO_OS_WIN)
TEST(QueryExecutor, StringBinarySearchRegex) {
StringPool pool;
diff --git a/src/trace_processor/db/storage/BUILD.gn b/src/trace_processor/db/storage/BUILD.gn
index 72d8d67..d13b530 100644
--- a/src/trace_processor/db/storage/BUILD.gn
+++ b/src/trace_processor/db/storage/BUILD.gn
@@ -37,6 +37,7 @@
"string_storage.cc",
"string_storage.h",
"types.h",
+ "utils.cc",
"utils.h",
]
deps = [
diff --git a/src/trace_processor/db/storage/id_storage.cc b/src/trace_processor/db/storage/id_storage.cc
index 68cfb2f..0b9adee 100644
--- a/src/trace_processor/db/storage/id_storage.cc
+++ b/src/trace_processor/db/storage/id_storage.cc
@@ -15,7 +15,6 @@
*/
#include "src/trace_processor/db/storage/id_storage.h"
-
#include <optional>
#include "perfetto/base/logging.h"
@@ -124,17 +123,18 @@
return SearchValidationResult::kNoData;
}
- // TODO(b/307482437): Remove after adding support for double
- PERFETTO_CHECK(val.type != SqlValue::kDouble);
-
// Bounds of the value.
- if (PERFETTO_UNLIKELY(val.AsLong() > std::numeric_limits<uint32_t>::max())) {
+ double_t num_val = val.type == SqlValue::kLong
+ ? static_cast<double_t>(val.AsLong())
+ : val.AsDouble();
+
+ if (PERFETTO_UNLIKELY(num_val > std::numeric_limits<uint32_t>::max())) {
if (op == FilterOp::kLe || op == FilterOp::kLt || op == FilterOp::kNe) {
return SearchValidationResult::kAllData;
}
return SearchValidationResult::kNoData;
}
- if (PERFETTO_UNLIKELY(val.AsLong() < std::numeric_limits<uint32_t>::min())) {
+ if (PERFETTO_UNLIKELY(num_val < std::numeric_limits<uint32_t>::min())) {
if (op == FilterOp::kGe || op == FilterOp::kGt || op == FilterOp::kNe) {
return SearchValidationResult::kAllData;
}
@@ -157,7 +157,21 @@
PERFETTO_DCHECK(search_range.end <= size_);
+ // It's a valid filter operation if |sql_val| is a double, although it
+ // requires special logic.
+ if (sql_val.type == SqlValue::kDouble) {
+ switch (utils::CompareIntColumnWithDouble(&sql_val, op)) {
+ case SearchValidationResult::kOk:
+ break;
+ case SearchValidationResult::kAllData:
+ return RangeOrBitVector(Range(0, search_range.end));
+ case SearchValidationResult::kNoData:
+ return RangeOrBitVector(Range());
+ }
+ }
+
uint32_t val = static_cast<uint32_t>(sql_val.AsLong());
+
if (op == FilterOp::kNe) {
BitVector ret(search_range.start, false);
ret.Resize(search_range.end, true);
@@ -180,6 +194,19 @@
std::to_string(static_cast<uint32_t>(op)));
});
+ // It's a valid filter operation if |sql_val| is a double, although it
+ // requires special logic.
+ if (sql_val.type == SqlValue::kDouble) {
+ switch (utils::CompareIntColumnWithDouble(&sql_val, op)) {
+ case SearchValidationResult::kOk:
+ break;
+ case SearchValidationResult::kAllData:
+ return RangeOrBitVector(Range(0, indices_size));
+ case SearchValidationResult::kNoData:
+ return RangeOrBitVector(Range());
+ }
+ }
+
uint32_t val = static_cast<uint32_t>(sql_val.AsLong());
switch (op) {
diff --git a/src/trace_processor/db/storage/id_storage_unittest.cc b/src/trace_processor/db/storage/id_storage_unittest.cc
index 91cc37b..ab04d86 100644
--- a/src/trace_processor/db/storage/id_storage_unittest.cc
+++ b/src/trace_processor/db/storage/id_storage_unittest.cc
@@ -28,9 +28,28 @@
return std::tie(a.start, a.end) == std::tie(b.start, b.end);
}
+inline bool operator==(const BitVector& a, const BitVector& b) {
+ return a.size() == b.size() && a.CountSetBits() == b.CountSetBits();
+}
+
namespace storage {
namespace {
+using testing::ElementsAre;
+using testing::IsEmpty;
+using Range = RowMap::Range;
+
+std::vector<uint32_t> ToIndexVector(RangeOrBitVector& r_or_bv) {
+ RowMap rm;
+ if (r_or_bv.IsBitVector()) {
+ rm = RowMap(std::move(r_or_bv).TakeIfBitVector());
+ } else {
+ Range range = std::move(r_or_bv).TakeIfRange();
+ rm = RowMap(range.start, range.end);
+ }
+ return rm.GetAllIndices();
+}
+
using Range = RowMap::Range;
TEST(IdStorageUnittest, InvalidSearchConstraints) {
@@ -55,6 +74,11 @@
FilterOp::kGe),
SearchValidationResult::kNoData);
+ // With double
+ ASSERT_EQ(
+ storage.ValidateSearchConstraints(SqlValue::Double(-1), FilterOp::kGe),
+ SearchValidationResult::kAllData);
+
// Value bounds
SqlValue max_val = SqlValue::Long(
static_cast<int64_t>(std::numeric_limits<uint32_t>::max()) + 10);
@@ -72,8 +96,7 @@
ASSERT_EQ(storage.ValidateSearchConstraints(max_val, FilterOp::kNe),
SearchValidationResult::kAllData);
- SqlValue min_val = SqlValue::Long(
- static_cast<int64_t>(std::numeric_limits<uint32_t>::min()) - 1);
+ SqlValue min_val = SqlValue::Long(-1);
ASSERT_EQ(storage.ValidateSearchConstraints(min_val, FilterOp::kGe),
SearchValidationResult::kAllData);
ASSERT_EQ(storage.ValidateSearchConstraints(min_val, FilterOp::kGt),
@@ -259,6 +282,61 @@
ASSERT_TRUE(bv.IsSet(5));
}
+TEST(IdStorageUnittest, SearchWithIdAsDoubleSimple) {
+ IdStorage storage(100);
+ SqlValue double_val = SqlValue::Double(15.0);
+ SqlValue long_val = SqlValue::Long(15);
+ Range range(10, 20);
+
+ auto res_double = storage.Search(FilterOp::kEq, double_val, range);
+ auto res_long = storage.Search(FilterOp::kEq, long_val, range);
+ ASSERT_EQ(ToIndexVector(res_double), ToIndexVector(res_long));
+
+ res_double = storage.Search(FilterOp::kNe, double_val, range);
+ res_long = storage.Search(FilterOp::kNe, long_val, range);
+ ASSERT_EQ(ToIndexVector(res_double), ToIndexVector(res_long));
+
+ res_double = storage.Search(FilterOp::kLe, double_val, range);
+ res_long = storage.Search(FilterOp::kLe, long_val, range);
+ ASSERT_EQ(ToIndexVector(res_double), ToIndexVector(res_long));
+
+ res_double = storage.Search(FilterOp::kLt, double_val, range);
+ res_long = storage.Search(FilterOp::kLt, long_val, range);
+ ASSERT_EQ(ToIndexVector(res_double), ToIndexVector(res_long));
+
+ res_double = storage.Search(FilterOp::kGe, double_val, range);
+ res_long = storage.Search(FilterOp::kGe, long_val, range);
+ ASSERT_EQ(ToIndexVector(res_double), ToIndexVector(res_long));
+
+ res_double = storage.Search(FilterOp::kGt, double_val, range);
+ res_long = storage.Search(FilterOp::kGt, long_val, range);
+ ASSERT_EQ(ToIndexVector(res_double), ToIndexVector(res_long));
+}
+
+TEST(IdStorageUnittest, SearchWithIdAsDouble) {
+ IdStorage storage(100);
+ Range range(10, 20);
+ SqlValue val = SqlValue::Double(15.5);
+
+ auto res = storage.Search(FilterOp::kEq, val, range);
+ ASSERT_THAT(ToIndexVector(res), IsEmpty());
+
+ res = storage.Search(FilterOp::kNe, val, range);
+ ASSERT_EQ(ToIndexVector(res).size(), 20u);
+
+ res = storage.Search(FilterOp::kLe, val, range);
+ ASSERT_THAT(ToIndexVector(res), ElementsAre(10, 11, 12, 13, 14, 15));
+
+ res = storage.Search(FilterOp::kLt, val, range);
+ ASSERT_THAT(ToIndexVector(res), ElementsAre(10, 11, 12, 13, 14, 15));
+
+ res = storage.Search(FilterOp::kGe, val, range);
+ ASSERT_THAT(ToIndexVector(res), ElementsAre(16, 17, 18, 19));
+
+ res = storage.Search(FilterOp::kGt, val, range);
+ ASSERT_THAT(ToIndexVector(res), ElementsAre(16, 17, 18, 19));
+}
+
TEST(IdStorageUnittest, Sort) {
std::vector<uint32_t> order{4, 3, 6, 1, 5};
IdStorage storage(10);
diff --git a/src/trace_processor/db/storage/set_id_storage.cc b/src/trace_processor/db/storage/set_id_storage.cc
index 5efc45f..dd0af25 100644
--- a/src/trace_processor/db/storage/set_id_storage.cc
+++ b/src/trace_processor/db/storage/set_id_storage.cc
@@ -108,17 +108,18 @@
return SearchValidationResult::kNoData;
}
- // TODO(b/307482437): Remove after adding support for double
- PERFETTO_CHECK(val.type != SqlValue::kDouble);
-
// Bounds of the value.
- if (PERFETTO_UNLIKELY(val.AsLong() > std::numeric_limits<uint32_t>::max())) {
+ double_t num_val = val.type == SqlValue::kLong
+ ? static_cast<double_t>(val.AsLong())
+ : val.AsDouble();
+
+ if (PERFETTO_UNLIKELY(num_val > std::numeric_limits<uint32_t>::max())) {
if (op == FilterOp::kLe || op == FilterOp::kLt || op == FilterOp::kNe) {
return SearchValidationResult::kAllData;
}
return SearchValidationResult::kNoData;
}
- if (PERFETTO_UNLIKELY(val.AsLong() < std::numeric_limits<uint32_t>::min())) {
+ if (PERFETTO_UNLIKELY(num_val < std::numeric_limits<uint32_t>::min())) {
if (op == FilterOp::kGe || op == FilterOp::kGt || op == FilterOp::kNe) {
return SearchValidationResult::kAllData;
}
@@ -131,6 +132,8 @@
RangeOrBitVector SetIdStorage::Search(FilterOp op,
SqlValue sql_val,
RowMap::Range search_range) const {
+ PERFETTO_DCHECK(search_range.end <= size());
+
PERFETTO_TP_TRACE(metatrace::Category::DB, "SetIdStorage::Search",
[&search_range, op](metatrace::Record* r) {
r->AddArg("Start", std::to_string(search_range.start));
@@ -139,7 +142,18 @@
std::to_string(static_cast<uint32_t>(op)));
});
- PERFETTO_DCHECK(search_range.end <= size());
+ // It's a valid filter operation if |sql_val| is a double, although it
+ // requires special logic.
+ if (sql_val.type == SqlValue::kDouble) {
+ switch (utils::CompareIntColumnWithDouble(&sql_val, op)) {
+ case SearchValidationResult::kOk:
+ break;
+ case SearchValidationResult::kAllData:
+ return RangeOrBitVector(Range(0, search_range.end));
+ case SearchValidationResult::kNoData:
+ return RangeOrBitVector(Range());
+ }
+ }
uint32_t val = static_cast<uint32_t>(sql_val.AsLong());
@@ -169,6 +183,19 @@
std::to_string(static_cast<uint32_t>(op)));
});
+ // It's a valid filter operation if |sql_val| is a double, although it
+ // requires special logic.
+ if (sql_val.type == SqlValue::kDouble) {
+ switch (utils::CompareIntColumnWithDouble(&sql_val, op)) {
+ case SearchValidationResult::kOk:
+ break;
+ case SearchValidationResult::kAllData:
+ return RangeOrBitVector(Range(0, indices_size));
+ case SearchValidationResult::kNoData:
+ return RangeOrBitVector(Range());
+ }
+ }
+
uint32_t val = static_cast<uint32_t>(sql_val.AsLong());
BitVector::Builder builder(indices_size);
diff --git a/src/trace_processor/db/storage/set_id_storage_unittest.cc b/src/trace_processor/db/storage/set_id_storage_unittest.cc
index 562db61..b7cbfc7 100644
--- a/src/trace_processor/db/storage/set_id_storage_unittest.cc
+++ b/src/trace_processor/db/storage/set_id_storage_unittest.cc
@@ -24,11 +24,26 @@
return std::tie(a.start, a.end) == std::tie(b.start, b.end);
}
+inline bool operator==(const BitVector& a, const BitVector& b) {
+ return a.size() == b.size() && a.CountSetBits() == b.CountSetBits();
+}
+
namespace storage {
namespace {
using Range = RowMap::Range;
+std::vector<uint32_t> ToIndexVector(RangeOrBitVector r_or_bv) {
+ RowMap rm;
+ if (r_or_bv.IsBitVector()) {
+ rm = RowMap(std::move(r_or_bv).TakeIfBitVector());
+ } else {
+ Range range = std::move(r_or_bv).TakeIfRange();
+ rm = RowMap(range.start, range.end);
+ }
+ return rm.GetAllIndices();
+}
+
TEST(SetIdStorageUnittest, InvalidSearchConstraints) {
std::vector<uint32_t> storage_data{0, 0, 0, 3, 3, 3, 6, 6, 6, 9, 9, 9};
SetIdStorage storage(&storage_data);
@@ -281,6 +296,52 @@
ASSERT_EQ(bv.CountSetBits(), 2u);
}
+TEST(SetIdStorageUnittest, SearchWithIdAsDoubleSimple) {
+ std::vector<uint32_t> storage_data{0, 0, 0, 3, 3, 3, 6, 6, 6, 9, 9, 9};
+ SetIdStorage storage(&storage_data);
+ SqlValue double_val = SqlValue::Double(7.0);
+ SqlValue long_val = SqlValue::Long(7);
+ Range range(1, 9);
+
+ ASSERT_EQ(ToIndexVector(storage.Search(FilterOp::kEq, double_val, range)),
+ ToIndexVector(storage.Search(FilterOp::kEq, long_val, range)));
+ ASSERT_EQ(ToIndexVector(storage.Search(FilterOp::kNe, double_val, range)),
+ ToIndexVector(storage.Search(FilterOp::kNe, long_val, range)));
+ ASSERT_EQ(ToIndexVector(storage.Search(FilterOp::kLe, double_val, range)),
+ ToIndexVector(storage.Search(FilterOp::kLe, long_val, range)));
+ ASSERT_EQ(ToIndexVector(storage.Search(FilterOp::kLt, double_val, range)),
+ ToIndexVector(storage.Search(FilterOp::kLt, long_val, range)));
+ ASSERT_EQ(ToIndexVector(storage.Search(FilterOp::kGe, double_val, range)),
+ ToIndexVector(storage.Search(FilterOp::kGe, long_val, range)));
+ ASSERT_EQ(ToIndexVector(storage.Search(FilterOp::kGt, double_val, range)),
+ ToIndexVector(storage.Search(FilterOp::kGt, long_val, range)));
+}
+
+TEST(SetIdStorageUnittest, SearchWithIdAsDouble) {
+ std::vector<uint32_t> storage_data{0, 0, 0, 3, 3, 3, 6, 6, 6, 9, 9, 9};
+ SetIdStorage storage(&storage_data);
+ SqlValue val = SqlValue::Double(7.5);
+ Range range(5, 10);
+
+ Range res = storage.Search(FilterOp::kEq, val, range).TakeIfRange();
+ ASSERT_EQ(res, Range());
+
+ res = storage.Search(FilterOp::kNe, val, range).TakeIfRange();
+ ASSERT_EQ(res, Range(0, 10));
+
+ res = storage.Search(FilterOp::kLe, val, range).TakeIfRange();
+ ASSERT_EQ(res, Range(5, 9));
+
+ res = storage.Search(FilterOp::kLt, val, range).TakeIfRange();
+ ASSERT_EQ(res, Range(5, 9));
+
+ res = storage.Search(FilterOp::kGe, val, range).TakeIfRange();
+ ASSERT_EQ(res, Range(9, 10));
+
+ res = storage.Search(FilterOp::kGt, val, range).TakeIfRange();
+ ASSERT_EQ(res, Range(9, 10));
+}
+
} // namespace
} // namespace storage
} // namespace trace_processor
diff --git a/src/trace_processor/db/storage/utils.cc b/src/trace_processor/db/storage/utils.cc
new file mode 100644
index 0000000..ba12e40
--- /dev/null
+++ b/src/trace_processor/db/storage/utils.cc
@@ -0,0 +1,64 @@
+/*
+ * Copyright (C) 2023 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "src/trace_processor/db/storage/utils.h"
+
+namespace perfetto {
+namespace trace_processor {
+namespace storage {
+namespace utils {
+
+SearchValidationResult CompareIntColumnWithDouble(SqlValue* sql_val,
+ FilterOp op) {
+ double double_val = sql_val->AsDouble();
+ if (std::equal_to<double>()(
+ double_val, static_cast<double>(static_cast<uint32_t>(double_val)))) {
+ // If double is the same as uint32_t, we should just "cast" the |sql_val|
+ // to be treated as long.
+ *sql_val = SqlValue::Long(static_cast<int64_t>(double_val));
+ return SearchValidationResult::kOk;
+ }
+ // Logic for when the value is a real double.
+ switch (op) {
+ case FilterOp::kEq:
+ return SearchValidationResult::kNoData;
+ case FilterOp::kNe:
+ return SearchValidationResult::kAllData;
+
+ case FilterOp::kLe:
+ case FilterOp::kGt:
+ *sql_val = SqlValue::Long(static_cast<int64_t>(std::floor(double_val)));
+ return SearchValidationResult::kOk;
+
+ case FilterOp::kLt:
+ case FilterOp::kGe:
+ *sql_val = SqlValue::Long(static_cast<int64_t>(std::ceil(double_val)));
+ return SearchValidationResult::kOk;
+
+ case FilterOp::kIsNotNull:
+ case FilterOp::kIsNull:
+ case FilterOp::kGlob:
+ case FilterOp::kRegex:
+ PERFETTO_FATAL("Invalid filter operation");
+ }
+ PERFETTO_FATAL("For GCC");
+}
+
+} // namespace utils
+
+} // namespace storage
+} // namespace trace_processor
+} // namespace perfetto
