waynexia commented on PR #16877: URL: https://github.com/apache/datafusion/pull/16877#issuecomment-3124163124
I do find revert the null checking logic on non-ascii branch will eliminate regression on those two cases, but it would slow down other cases in the same time. Still not sure why the get value operation is slower only for 32-length UTF-8... Maybe we can make it faster by branching using offsets (like check if start equals to end), but `StringArrayType` doesn't provide such APIs (the same as https://github.com/apache/datafusion/pull/16877#discussion_r2228560541). However I find checking if the value is empty (zero length) in advance will make it a bit faster, the regression changes from 10% to 5% while keeping other cases unchanged: ``` character_length_StringArray_ascii_str_len_8 time: [1.4314 µs 1.4336 µs 1.4369 µs] change: [-73.924% -73.891% -73.859%] (p = 0.00 < 0.05) Performance has improved. Found 3 outliers among 100 measurements (3.00%) 1 (1.00%) high mild 2 (2.00%) high severe character_length_StringArray_utf8_str_len_8 time: [31.946 µs 31.979 µs 32.018 µs] change: [-11.242% -11.106% -10.965%] (p = 0.00 < 0.05) Performance has improved. Found 6 outliers among 100 measurements (6.00%) 3 (3.00%) high mild 3 (3.00%) high severe character_length_StringViewArray_ascii_str_len_8 time: [15.377 µs 15.390 µs 15.404 µs] change: [-30.680% -30.603% -30.527%] (p = 0.00 < 0.05) Performance has improved. Found 3 outliers among 100 measurements (3.00%) 2 (2.00%) high mild 1 (1.00%) high severe character_length_StringViewArray_utf8_str_len_8 time: [32.590 µs 32.675 µs 32.767 µs] change: [-8.6176% -8.4165% -8.1910%] (p = 0.00 < 0.05) Performance has improved. Found 8 outliers among 100 measurements (8.00%) 4 (4.00%) high mild 4 (4.00%) high severe character_length_StringArray_ascii_str_len_32 time: [3.9932 µs 3.9959 µs 3.9982 µs] change: [-50.365% -50.323% -50.281%] (p = 0.00 < 0.05) Performance has improved. character_length_StringArray_utf8_str_len_32 time: [38.780 µs 38.815 µs 38.847 µs] change: [+5.2960% +5.3832% +5.4714%] (p = 0.00 < 0.05) Performance has regressed. character_length_StringViewArray_ascii_str_len_32 time: [11.412 µs 11.427 µs 11.443 µs] change: [-27.836% -27.742% -27.636%] (p = 0.00 < 0.05) Performance has improved. Found 4 outliers among 100 measurements (4.00%) 3 (3.00%) high mild 1 (1.00%) high severe character_length_StringViewArray_utf8_str_len_32 time: [41.017 µs 41.067 µs 41.114 µs] change: [+6.4789% +6.6189% +6.7539%] (p = 0.00 < 0.05) Performance has regressed. Found 1 outliers among 100 measurements (1.00%) 1 (1.00%) high mild character_length_StringArray_ascii_str_len_128 time: [14.538 µs 14.575 µs 14.612 µs] change: [-23.194% -22.995% -22.793%] (p = 0.00 < 0.05) Performance has improved. Found 1 outliers among 100 measurements (1.00%) 1 (1.00%) high mild character_length_StringArray_utf8_str_len_128 time: [53.908 µs 53.982 µs 54.065 µs] change: [-5.0270% -4.8019% -4.5793%] (p = 0.00 < 0.05) Performance has improved. Found 7 outliers among 100 measurements (7.00%) 1 (1.00%) low mild 1 (1.00%) high mild 5 (5.00%) high severe character_length_StringViewArray_ascii_str_len_128 time: [22.863 µs 22.970 µs 23.072 µs] change: [-13.846% -13.226% -12.614%] (p = 0.00 < 0.05) Performance has improved. character_length_StringViewArray_utf8_str_len_128 time: [56.301 µs 56.404 µs 56.515 µs] change: [-4.7315% -4.5431% -4.3550%] (p = 0.00 < 0.05) Performance has improved. Found 5 outliers among 100 measurements (5.00%) 5 (5.00%) low mild character_length_StringArray_ascii_str_len_4096 time: [500.96 µs 502.81 µs 504.84 µs] change: [-3.3507% -2.7881% -2.2107%] (p = 0.00 < 0.05) Performance has improved. Found 7 outliers among 100 measurements (7.00%) 4 (4.00%) high mild 3 (3.00%) high severe Benchmarking character_length_StringArray_utf8_str_len_4096: Warming up for 3.0000 s Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 7.2s, enable flat sampling, or reduce sample count to 50. character_length_StringArray_utf8_str_len_4096 time: [1.4200 ms 1.4257 ms 1.4321 ms] change: [-4.4737% -3.1885% -1.9979%] (p = 0.00 < 0.05) Performance has improved. Found 8 outliers among 100 measurements (8.00%) 7 (7.00%) high mild 1 (1.00%) high severe character_length_StringViewArray_ascii_str_len_4096 time: [448.59 µs 450.82 µs 453.06 µs] change: [-1.7833% -1.1598% -0.5034%] (p = 0.00 < 0.05) Change within noise threshold. Found 4 outliers among 100 measurements (4.00%) 4 (4.00%) high mild Benchmarking character_length_StringViewArray_utf8_str_len_4096: Warming up for 3.0000 s Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 7.6s, enable flat sampling, or reduce sample count to 50. character_length_StringViewArray_utf8_str_len_4096 time: [1.4845 ms 1.4914 ms 1.4994 ms] change: [-2.4781% -1.9180% -1.2655%] (p = 0.00 < 0.05) Performance has improved. Found 10 outliers among 100 measurements (10.00%) 7 (7.00%) high mild 3 (3.00%) high severe ``` -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org