Dict: gather dictionary values straight into a VarBinBuilder - #9098
Dict: gather dictionary values straight into a VarBinBuilder#9098robert3005 wants to merge 1 commit into
Conversation
| bits.for_each_set_index(|row| num_bytes += view(row).len() as usize); | ||
| builder.append_value_slices( | ||
| num_bytes, | ||
| bits.set_indices().map(|row| view(row).bytes(&buffers)), |
There was a problem hiding this comment.
this should be for_each_set_index
There was a problem hiding this comment.
Done — append_dict_to_varbin no longer builds an index iterator at all. The byte total is summed with for_each_set_index and the copy now goes through a new VarBinBuilder::append_valid_slices, which takes a FnMut(usize) -> &[u8] and walks the valid rows a word at a time internally. The iterator-based append_value_slices is gone (in the base PR), so set_indices() is out of this path entirely.
Appending a string dictionary to a builder went through `take_canonical`, which first materializes the values at full logical length — allocating and re-reading a views buffer proportional to the row count. The dictionary is usually far smaller than the column, so resolving each code against it in place skips that intermediate and leaves one `memcpy` per row. The fast path indexes the codes and the dictionary's views and data buffers as host slices, so `varbin_fast_path_is_host` gates it on residency; a device-resident dictionary still takes the canonical route, where a device kernel can serve it. Signed-off-by: Robert Kruszewski <[email protected]> Signed-off-by: Robert Kruszewski <[email protected]>
91142df to
c82aa58
Compare
Merging this PR will degrade performance by 10.02%
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing 
Footnotes
|
1 participant
Materialize dictionary values straight into VarBinBuilder instead of copying