liaoxin01 commented on PR #66545: URL: https://github.com/apache/doris/pull/66545#issuecomment-5219743046
Splitting this in two, since the memory question and the sort question are independent and the memory one deserves to be judged on its own. This PR is now only the row diet: `RowInBlock` goes from a `shared_ptr` with a `make_shared` per row to a 24 byte value stored in the vector, which is 564 MB -> 169 MB for a 7.05M row memtable. It no longer touches the sort at all, so it is a straight reduction against master rather than something that has to be netted against a regression. The `ColumnSorter` change moved to #66588, stacked on this one. Its inline permutation is what costs 8-32 bytes per row depending on key width, and it only comes out ahead of master once this PR has landed -- that ordering is now explicit in its description, along with the before/after peak for each key type. Two things in here worth your eye, both consequences of dropping the `shared_ptr` rather than of the sort: - `_aggregate()` now adopts `temp_row_in_blocks` unconditionally. Without that, a memtable that aggregates across a `shrink_memtable_by_agg()` round and then again in `to_block()` releases the same aggregate states twice. I confirmed it by instrumenting `~MemTable` rather than reasoning about it -- it destroyed two states that `_finalize_one_row<true>()` had already released. The new shrink-round unit tests reach that path, but a release build does not reliably die on a double free, so ASAN is what would actually catch a regression there. - The flexible-partial-update paths are the largest part of the diff and have no unit coverage, before or after. I went through them line by line against the previous semantics, but that is a reading, not a test. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
