Merge "Fix segfault in trace processor" into main
diff --git a/src/trace_processor/db/column/dense_null_overlay.cc b/src/trace_processor/db/column/dense_null_overlay.cc
index e171ec0..55003fa 100644
--- a/src/trace_processor/db/column/dense_null_overlay.cc
+++ b/src/trace_processor/db/column/dense_null_overlay.cc
@@ -82,10 +82,10 @@
case SearchValidationResult::kNoData: {
// There is no need to search in underlying storage. It's enough to
// intersect the |non_null_|.
- BitVector res = non_null_->IntersectRange(in.start, in.end);
- res.Not();
+ BitVector res = non_null_->Copy();
res.Resize(in.end, false);
- return RangeOrBitVector(std::move(res));
+ res.Not();
+ return RangeOrBitVector(res.IntersectRange(in.start, in.end));
}
case SearchValidationResult::kAllData:
return RangeOrBitVector(in);
diff --git a/src/trace_processor/db/column/null_overlay.cc b/src/trace_processor/db/column/null_overlay.cc
index fa1e679..b1f8f45 100644
--- a/src/trace_processor/db/column/null_overlay.cc
+++ b/src/trace_processor/db/column/null_overlay.cc
@@ -129,10 +129,10 @@
case SearchValidationResult::kNoData: {
// There is no need to search in underlying storage. It's enough to
// intersect the |non_null_|.
- BitVector res = non_null_->IntersectRange(in.start, in.end);
- res.Not();
+ BitVector res = non_null_->Copy();
res.Resize(in.end, false);
- return RangeOrBitVector(std::move(res));
+ res.Not();
+ return RangeOrBitVector(res.IntersectRange(in.start, in.end));
}
case SearchValidationResult::kAllData:
return RangeOrBitVector(in);
diff --git a/src/trace_processor/db/column/numeric_storage.h b/src/trace_processor/db/column/numeric_storage.h
index f7cb51c..821fd40 100644
--- a/src/trace_processor/db/column/numeric_storage.h
+++ b/src/trace_processor/db/column/numeric_storage.h
@@ -100,21 +100,7 @@
SingleSearchResult SingleSearch(FilterOp op,
SqlValue sql_val,
uint32_t i) const override {
- if constexpr (std::is_same_v<T, double>) {
- if (sql_val.type != SqlValue::kDouble) {
- return SingleSearchResult::kNeedsFullSearch;
- }
- return utils::SingleSearchNumeric(op, (*vector_)[i],
- sql_val.double_value);
- } else {
- if (sql_val.type != SqlValue::kLong ||
- sql_val.long_value > std::numeric_limits<T>::max() ||
- sql_val.long_value < std::numeric_limits<T>::min()) {
- return SingleSearchResult::kNeedsFullSearch;
- }
- return utils::SingleSearchNumeric(op, (*vector_)[i],
- static_cast<T>(sql_val.long_value));
- }
+ return utils::SingleSearchNumeric(op, (*vector_)[i], sql_val);
}
void StableSort(SortToken* start,
diff --git a/src/trace_processor/db/column/numeric_storage_unittest.cc b/src/trace_processor/db/column/numeric_storage_unittest.cc
index 2b20306..46ef920 100644
--- a/src/trace_processor/db/column/numeric_storage_unittest.cc
+++ b/src/trace_processor/db/column/numeric_storage_unittest.cc
@@ -187,6 +187,11 @@
SingleSearchResult::kMatch);
ASSERT_EQ(chain->SingleSearch(FilterOp::kGe, SqlValue::Long(0), 5),
SingleSearchResult::kNoMatch);
+
+ ASSERT_EQ(chain->SingleSearch(FilterOp::kIsNull, SqlValue(), 0),
+ SingleSearchResult::kNoMatch);
+ ASSERT_EQ(chain->SingleSearch(FilterOp::kIsNotNull, SqlValue(), 0),
+ SingleSearchResult::kMatch);
}
TEST(NumericStorage, Search) {
diff --git a/src/trace_processor/db/column/set_id_storage.cc b/src/trace_processor/db/column/set_id_storage.cc
index 6e4719f..ae1c167 100644
--- a/src/trace_processor/db/column/set_id_storage.cc
+++ b/src/trace_processor/db/column/set_id_storage.cc
@@ -72,15 +72,7 @@
SingleSearchResult SetIdStorage::ChainImpl::SingleSearch(FilterOp op,
SqlValue sql_val,
uint32_t i) const {
- if (sql_val.type != SqlValue::kLong ||
- sql_val.long_value > std::numeric_limits<uint32_t>::max() ||
- sql_val.long_value < std::numeric_limits<uint32_t>::min()) {
- // Because of the large amount of code needing for handling comparisions
- // with doubles or out of range values, just defer to the full search.
- return SingleSearchResult::kNeedsFullSearch;
- }
- return utils::SingleSearchNumeric(op, (*values_)[i],
- static_cast<uint32_t>(sql_val.long_value));
+ return utils::SingleSearchNumeric(op, (*values_)[i], sql_val);
}
SearchValidationResult SetIdStorage::ChainImpl::ValidateSearchConstraints(
diff --git a/src/trace_processor/db/column/utils.h b/src/trace_processor/db/column/utils.h
index 14c6456..9533d8cd 100644
--- a/src/trace_processor/db/column/utils.h
+++ b/src/trace_processor/db/column/utils.h
@@ -19,7 +19,9 @@
#include <algorithm>
#include <cstdint>
#include <functional>
+#include <limits>
#include <optional>
+#include <type_traits>
#include <vector>
#include "perfetto/base/logging.h"
@@ -29,6 +31,36 @@
#include "src/trace_processor/db/column/types.h"
namespace perfetto::trace_processor::column::utils {
+namespace internal {
+
+template <typename T, typename Comparator>
+SingleSearchResult SingleSearchNumeric(T left, const SqlValue& right_v) {
+ if constexpr (std::is_same_v<T, double>) {
+ if (right_v.type != SqlValue::kDouble) {
+ // Because of the large amount of code needing for handling comparisons
+ // with integers, just defer to the full search.
+ return SingleSearchResult::kNeedsFullSearch;
+ }
+ return Comparator()(left, right_v.double_value)
+ ? SingleSearchResult::kMatch
+ : SingleSearchResult::kNoMatch;
+ } else if constexpr (std::is_integral_v<T>) {
+ if (right_v.type != SqlValue::kLong ||
+ right_v.long_value > std::numeric_limits<T>::max() ||
+ right_v.long_value < std::numeric_limits<T>::min()) {
+ // Because of the large amount of code needing for handling comparisons
+ // with doubles or out of range values, just defer to the full search.
+ return SingleSearchResult::kNeedsFullSearch;
+ }
+ return Comparator()(left, static_cast<T>(right_v.long_value))
+ ? SingleSearchResult::kMatch
+ : SingleSearchResult::kNoMatch;
+ } else {
+ static_assert(std::is_same_v<T, void>, "Illegal type");
+ }
+}
+
+} // namespace internal
template <typename Comparator, typename ValType, typename DataType>
void LinearSearchWithComparator(ValType val,
@@ -77,27 +109,25 @@
}
template <typename T>
-SingleSearchResult SingleSearchNumeric(FilterOp op, T left, T right) {
+SingleSearchResult SingleSearchNumeric(FilterOp op,
+ T left,
+ const SqlValue& right_v) {
switch (op) {
case FilterOp::kEq:
- return std::equal_to<T>()(left, right) ? SingleSearchResult::kMatch
- : SingleSearchResult::kNoMatch;
+ return internal::SingleSearchNumeric<T, std::equal_to<T>>(left, right_v);
case FilterOp::kNe:
- return std::not_equal_to<T>()(left, right) ? SingleSearchResult::kMatch
- : SingleSearchResult::kNoMatch;
+ return internal::SingleSearchNumeric<T, std::not_equal_to<T>>(left,
+ right_v);
case FilterOp::kGe:
- return std::greater_equal<T>()(left, right)
- ? SingleSearchResult::kMatch
- : SingleSearchResult::kNoMatch;
+ return internal::SingleSearchNumeric<T, std::greater_equal<T>>(left,
+ right_v);
case FilterOp::kGt:
- return std::greater<T>()(left, right) ? SingleSearchResult::kMatch
- : SingleSearchResult::kNoMatch;
+ return internal::SingleSearchNumeric<T, std::greater<T>>(left, right_v);
case FilterOp::kLe:
- return std::less_equal<T>()(left, right) ? SingleSearchResult::kMatch
- : SingleSearchResult::kNoMatch;
+ return internal::SingleSearchNumeric<T, std::less_equal<T>>(left,
+ right_v);
case FilterOp::kLt:
- return std::less<T>()(left, right) ? SingleSearchResult::kMatch
- : SingleSearchResult::kNoMatch;
+ return internal::SingleSearchNumeric<T, std::less<T>>(left, right_v);
case FilterOp::kIsNotNull:
return SingleSearchResult::kMatch;
case FilterOp::kGlob:
diff --git a/ui/src/common/actions.ts b/ui/src/common/actions.ts
index 4910cbd..92cad97 100644
--- a/ui/src/common/actions.ts
+++ b/ui/src/common/actions.ts
@@ -1162,13 +1162,6 @@
);
},
- setPivotTableArgumentNames(
- state: StateDraft,
- args: {argumentNames: string[]},
- ) {
- state.nonSerializableState.pivotTable.argumentNames = args.argumentNames;
- },
-
changePivotTablePivotOrder(
state: StateDraft,
args: {from: number; to: number; direction: DropDirection},
diff --git a/ui/src/common/empty_state.ts b/ui/src/common/empty_state.ts
index 5bf1f5a..f866914 100644
--- a/ui/src/common/empty_state.ts
+++ b/ui/src/common/empty_state.ts
@@ -83,7 +83,6 @@
],
constrainToArea: true,
queryRequested: false,
- argumentNames: [],
},
};
}
diff --git a/ui/src/common/state.ts b/ui/src/common/state.ts
index 3b27e16..d0af73b 100644
--- a/ui/src/common/state.ts
+++ b/ui/src/common/state.ts
@@ -424,9 +424,6 @@
// Set to true by frontend to request controller to perform the query to
// acquire the necessary data from the engine.
queryRequested: boolean;
-
- // Argument names in the current trace, used for autocompletion purposes.
- argumentNames: string[];
}
export interface LoadedConfigNone {
diff --git a/ui/src/controller/pivot_table_controller.ts b/ui/src/controller/pivot_table_controller.ts
index b1eb386..663fdf4 100644
--- a/ui/src/controller/pivot_table_controller.ts
+++ b/ui/src/controller/pivot_table_controller.ts
@@ -31,7 +31,7 @@
} from '../frontend/pivot_table_query_generator';
import {Aggregation, PivotTree} from '../frontend/pivot_table_types';
import {Engine} from '../trace_processor/engine';
-import {ColumnType, STR} from '../trace_processor/query_result';
+import {ColumnType} from '../trace_processor/query_result';
import {Controller} from './controller';
@@ -189,7 +189,6 @@
engine: Engine;
lastQueryAreaId = '';
lastQueryAreaTracks = new Set<string>();
- requestedArgumentNames = false;
constructor(args: {engine: Engine}) {
super({});
@@ -272,31 +271,11 @@
);
}
- async requestArgumentNames() {
- this.requestedArgumentNames = true;
- const result = await this.engine.query(`
- select distinct flat_key from args
- `);
- const it = result.iter({flat_key: STR});
-
- const argumentNames = [];
- while (it.valid()) {
- argumentNames.push(it.flat_key);
- it.next();
- }
-
- globals.dispatch(Actions.setPivotTableArgumentNames({argumentNames}));
- }
-
run() {
if (!PIVOT_TABLE_REDUX_FLAG.get()) {
return;
}
- if (!this.requestedArgumentNames) {
- this.requestArgumentNames();
- }
-
const pivotTableState = globals.state.nonSerializableState.pivotTable;
const selection = getLegacySelection(globals.state);
diff --git a/ui/src/frontend/pivot_table_argument_popup.ts b/ui/src/frontend/pivot_table_argument_popup.ts
index faa1579..949200e 100644
--- a/ui/src/frontend/pivot_table_argument_popup.ts
+++ b/ui/src/frontend/pivot_table_argument_popup.ts
@@ -20,21 +20,6 @@
interface ArgumentPopupArgs {
onArgumentChange: (arg: string) => void;
- knownArguments: string[];
-}
-
-function longestString(array: string[]): string {
- if (array.length === 0) {
- return '';
- }
-
- let answer = array[0];
- for (let i = 1; i < array.length; i++) {
- if (array[i].length > answer.length) {
- answer = array[i];
- }
- }
- return answer;
}
// Component rendering popup for entering an argument name to use as a pivot.
@@ -47,41 +32,6 @@
raf.scheduleFullRedraw();
}
- renderMatches(attrs: ArgumentPopupArgs): m.Child[] {
- const result: m.Child[] = [];
-
- for (const option of attrs.knownArguments) {
- // Would be great to have smarter fuzzy matching, but in the meantime
- // simple substring check should work fine.
- const index = option.indexOf(this.argument);
-
- if (index === -1) {
- continue;
- }
-
- if (result.length === 10) {
- break;
- }
-
- result.push(
- m(
- 'div',
- {
- onclick: () => {
- this.setArgument(attrs, option);
- },
- },
- option.substring(0, index),
- // Highlight the matching part with bold font
- m('strong', this.argument),
- option.substring(index + this.argument.length),
- ),
- );
- }
-
- return result;
- }
-
view({attrs}: m.Vnode<ArgumentPopupArgs>): m.Child {
return m(
'.name-completion',
@@ -94,8 +44,6 @@
},
value: this.argument,
}),
- m('.arguments-popup-sizer', longestString(attrs.knownArguments)),
- this.renderMatches(attrs),
);
}
}
diff --git a/ui/src/frontend/tables/attribute_modal_holder.ts b/ui/src/frontend/tables/attribute_modal_holder.ts
index 562a922..d154ab1 100644
--- a/ui/src/frontend/tables/attribute_modal_holder.ts
+++ b/ui/src/frontend/tables/attribute_modal_holder.ts
@@ -15,7 +15,6 @@
import m from 'mithril';
import {showModal} from '../../widgets/modal';
-import {globals} from '../globals';
import {ArgumentPopup} from '../pivot_table_argument_popup';
export class AttributeModalHolder {
@@ -45,8 +44,6 @@
private renderModalContents() {
return m(ArgumentPopup, {
- knownArguments:
- globals.state.nonSerializableState.pivotTable.argumentNames,
onArgumentChange: (arg) => {
this.typedArgument = arg;
},
