HappenLee commented on PR #67951: URL: https://github.com/apache/doris/pull/67951#issuecomment-5676042402
For this correctness fix, I would keep `std::all_of` here. It directly expresses that every row in the merged NULL map must already be NULL. Please also retain the `input_rows_count > 0` guard, since `all_of` returns true for an empty range. For reference, I benchmarked the current predicate against `!simd::contain_zero(...)`, including the same nonempty guard in both. Environment: Xeon Platinum 8457C, Clang 21.1.8 / libstdc++ 15, `-O3 -mavx2`, no sanitizer, pinned CPU, seven-sample medians over a warm corpus of 64 maps. | Input | `std::all_of` | `!simd::contain_zero` | |---|---:|---:| | 31 rows, all NULL | 11.21 ns | 15.87 ns | | 4096 rows, first row non-NULL | 1.57 ns | 1.29 ns | | 4096 rows, all NULL | 1563.31 ns | 83.30 ns | SIMD clearly wins on long scans: the generated AVX2 code checks 32 bytes at a time, while `all_of` remains scalar. For inputs shorter than 32 bytes, this helper uses its scalar tail and can be slower; when the first row is non-NULL, both exit quickly. The shared host also introduces timing variation. Keeping `std::all_of` is therefore a simplicity choice, not a claim that it is faster. The long-scan improvement is real, but these measurements cover only the NULL-map predicate, not the complete date function or SQL execution. We can evaluate that optimization separately with representative workloads. -- 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]
