Skip to content

[chore](env) remove has_varlen_slots in sort #37162

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Mryange wants to merge 2 commits into from
Closed

[chore](env) remove has_varlen_slots in sort #37162

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b47dd20
upd
Mryange Jul 2, 2024
c16a61c
remove
Mryange Jul 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 2 additions & 6 deletions be/src/pipeline/exec/sort_sink_operator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,20 +105,16 @@ Status SortSinkOperatorX::init(const TPlanNode& tnode, RuntimeState* state) {
}

Status SortSinkOperatorX::prepare(RuntimeState* state) {
const auto& row_desc = _child_x->row_desc();

// If `limit` is smaller than HEAP_SORT_THRESHOLD, we consider using heap sort in priority.
// To do heap sorting, each income block will be filtered by heap-top row. There will be some
// `memcpy` operations. To ensure heap sort will not incur performance fallback, we should
// exclude cases which incoming blocks has string column which is sensitive to operations like
// `filter` and `memcpy`
if (_limit > 0 && _limit + _offset < vectorized::HeapSorter::HEAP_SORT_THRESHOLD &&
(_use_two_phase_read || state->get_query_ctx()->has_runtime_predicate(_node_id) ||
!row_desc.has_varlen_slots())) {
(_use_two_phase_read || state->get_query_ctx()->has_runtime_predicate(_node_id))) {
_algorithm = SortAlgorithm::HEAP_SORT;
_reuse_mem = false;
} else if (_limit > 0 && row_desc.has_varlen_slots() &&
_limit + _offset < vectorized::TopNSorter::TOPN_SORT_THRESHOLD) {
} else if (_limit > 0 && _limit + _offset < vectorized::TopNSorter::TOPN_SORT_THRESHOLD) {
_algorithm = SortAlgorithm::TOPN_SORT;
} else {
_algorithm = SortAlgorithm::FULL_SORT;
Expand Down
31 changes: 2 additions & 29 deletions be/src/runtime/descriptors.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,26 +292,16 @@ std::string JdbcTableDescriptor::debug_string() const {
}

TupleDescriptor::TupleDescriptor(const TTupleDescriptor& tdesc, bool own_slots)
: _id(tdesc.id),
_num_materialized_slots(0),
_has_varlen_slots(false),
_own_slots(own_slots) {}
: _id(tdesc.id), _num_materialized_slots(0), _own_slots(own_slots) {}

TupleDescriptor::TupleDescriptor(const PTupleDescriptor& pdesc, bool own_slots)
: _id(pdesc.id()),
_num_materialized_slots(0),
_has_varlen_slots(false),
_own_slots(own_slots) {}
: _id(pdesc.id()), _num_materialized_slots(0), _own_slots(own_slots) {}

void TupleDescriptor::add_slot(SlotDescriptor* slot) {
_slots.push_back(slot);

if (slot->is_materialized()) {
++_num_materialized_slots;

if (slot->type().is_string_type() || slot->type().is_collection_type()) {
_has_varlen_slots = true;
}
}
}

Expand All @@ -338,10 +328,6 @@ std::string TupleDescriptor::debug_string() const {
}
out << _slots[i]->debug_string();
}

out << "]";
out << " has_varlen_slots=" << _has_varlen_slots;
out << ")";
return out.str();
}

Expand All @@ -364,13 +350,11 @@ RowDescriptor::RowDescriptor(const DescriptorTbl& desc_tbl, const std::vector<TT
}

init_tuple_idx_map();
init_has_varlen_slots();
}

RowDescriptor::RowDescriptor(TupleDescriptor* tuple_desc, bool is_nullable)
: _tuple_desc_map(1, tuple_desc), _tuple_idx_nullable_map(1, is_nullable) {
init_tuple_idx_map();
init_has_varlen_slots();
_num_slots = tuple_desc->slots().size();
}

Expand All @@ -386,7 +370,6 @@ RowDescriptor::RowDescriptor(const RowDescriptor& lhs_row_desc, const RowDescrip
rhs_row_desc._tuple_idx_nullable_map.begin(),
rhs_row_desc._tuple_idx_nullable_map.end());
init_tuple_idx_map();
init_has_varlen_slots();

_num_slots = lhs_row_desc.num_slots() + rhs_row_desc.num_slots();
}
Expand All @@ -404,16 +387,6 @@ void RowDescriptor::init_tuple_idx_map() {
}
}

void RowDescriptor::init_has_varlen_slots() {
_has_varlen_slots = false;
for (auto& i : _tuple_desc_map) {
if (i->has_varlen_slots()) {
_has_varlen_slots = true;
break;
}
}
}

int RowDescriptor::get_tuple_idx(TupleId id) const {
// comment CHECK temporarily to make fuzzy test run smoothly
// DCHECK_LT(id, _tuple_idx_map.size()) << "RowDescriptor: " << debug_string();
Expand Down
16 changes: 1 addition & 15 deletions be/src/runtime/descriptors.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,6 @@ class TupleDescriptor {
int num_materialized_slots() const { return _num_materialized_slots; }
const std::vector<SlotDescriptor*>& slots() const { return _slots; }

bool has_varlen_slots() const { return _has_varlen_slots; }
const TableDescriptor* table_desc() const { return _table_desc; }

TupleId id() const { return _id; }
Expand All @@ -364,9 +363,6 @@ class TupleDescriptor {
int _num_materialized_slots;
std::vector<SlotDescriptor*> _slots; // contains all slots

// Provide quick way to check if there are variable length slots.
// True if _string_slots or _collection_slots have entries.
bool _has_varlen_slots;
bool _own_slots = false;

TupleDescriptor(const TTupleDescriptor& tdesc, bool own_slot = false);
Expand Down Expand Up @@ -435,8 +431,7 @@ class RowDescriptor {
RowDescriptor(const RowDescriptor& desc)
: _tuple_desc_map(desc._tuple_desc_map),
_tuple_idx_nullable_map(desc._tuple_idx_nullable_map),
_tuple_idx_map(desc._tuple_idx_map),
_has_varlen_slots(desc._has_varlen_slots) {
_tuple_idx_map(desc._tuple_idx_map) {
auto it = desc._tuple_desc_map.begin();
for (; it != desc._tuple_desc_map.end(); ++it) {
_num_materialized_slots += (*it)->num_materialized_slots();
Expand All @@ -460,9 +455,6 @@ class RowDescriptor {
// Returns INVALID_IDX if id not part of this row.
int get_tuple_idx(TupleId id) const;

// Return true if any Tuple has variable length slots.
bool has_varlen_slots() const { return _has_varlen_slots; }

// Return descriptors for all tuples in this row, in order of appearance.
const std::vector<TupleDescriptor*>& tuple_descriptors() const { return _tuple_desc_map; }

Expand All @@ -485,9 +477,6 @@ class RowDescriptor {
// Initializes tupleIdxMap during c'tor using the _tuple_desc_map.
void init_tuple_idx_map();

// Initializes _has_varlen_slots during c'tor using the _tuple_desc_map.
void init_has_varlen_slots();

// map from position of tuple w/in row to its descriptor
std::vector<TupleDescriptor*> _tuple_desc_map;

Expand All @@ -497,9 +486,6 @@ class RowDescriptor {
// map from TupleId to position of tuple w/in row
std::vector<int> _tuple_idx_map;

// Provide quick way to check if there are variable length slots.
bool _has_varlen_slots = false;

int _num_materialized_slots = 0;
int _num_slots = 0;
};
Expand Down