YQL-18356: Refactor WideTopSort to be able use LLVM compilation #5109
Conversation
|
⚪ |
|
⚪ |
|
⚪
|
|
⚪
|
Darych
force-pushed
the
yql-18356-refactor-widetopsort
branch
from
d2ee9e1 to
a537476
Compare
|
⚪
|
|
⚪
|
|
⚪
|
|
⚪
|
| // Remove placeholder for new data | ||
| Storage.resize(Storage.size() - Indexes.size()); | ||
|
|
||
| Full.reserve(Storage.size() / Indexes.size()); |
There was a problem hiding this comment.
Do we have guarantees that Indexes.size() is not 0?
There was a problem hiding this comment.
Yes. Indexes size is the number of input columns to the callable.
| } | ||
|
|
||
| Free.clear(); | ||
| Free.shrink_to_fit(); |
There was a problem hiding this comment.
Cppref says that shrink_to_fit is not obliged to actually shrink:
https://en.cppreference.com/w/cpp/container/vector/shrink_to_fit
As far as I know the only way to completely clear container is to swap it with empty container
There was a problem hiding this comment.
Hm, I kept the previous implementation.
I think we should be fine for now: https://stackoverflow.com/questions/23502291/is-shrink-to-fit-the-proper-way-of-reducing-the-capacity-a-stdvector-to-its/23503995#23503995
2 participants
Changelog entry
...
Changelog category
Additional information
...