neilconway opened a new pull request, #26039: URL: https://github.com/apache/datafusion/pull/26039
## Which issue does this PR close? - N/A ## Rationale for this change #23762 added an ASCII fast-path for `left` and `right`. That improves performance in many scenarios, but the implementation calls `is_ascii` on every input string. That is expensive, particularly for the common case that `left` and `right` are used to fetch a small prefix/suffix from a much longer string. That check is also overly conservative: for example, we can take the ASCII fast-path for `left(s, k)` if the first `k` bytes in the string are ASCII, even if there are multibyte characters elsewhere in the string. This PR implements two optimizations: 1. Only call `is_ascii` on the bytes necessary to determine if we can take the fast-path, not the entire input string, as described above. 2. Benchmarking identified that for `Utf8View` inputs, the first optimization regressed some benchmark cases (e.g., `negative_n`) on both ARM and x86. Claude's theory is that `make_view` is out-of-line and does an indirect jump on the length of the result string; it seems that after implementing the first optimization, this jump was not well-handled by the branch predictor. Instead, we add a helper `sub_view` that returns a view that is a substring of an existing view. This can be inlined and avoids the indirect jump incurred by `make_view`; it can also construct the new view from the old view with bitwise ops, rather than building the new view on the stack. Benchmarks: (separate PR #26026) x86 (AMD EPYC Milan) - left Utf8 long_result: 101.7 → 93.4 (−8%) - left Utf8 n_exceeds_len: 91.7 → 93.8 (+2%) - left Utf8 negative_n: 90.4 → 83.2 (−8%) - left Utf8 per_row_n: 93.9 → 89.5 (−5%) - left Utf8 short_result: 91.0 → 82.6 (−9%) - left Utf8 short_result_long_input: 310.4 → 89.4 (−71%) - left Utf8View long_result: 53.6 → 40.9 (−24%) - left Utf8View n_exceeds_len: 55.9 → 50.7 (−9%) - left Utf8View negative_n: 58.8 → 45.2 (−23%) - left Utf8View per_row_n: 89.0 → 45.2 (−49%) - left Utf8View short_result: 54.2 → 47.5 (−12%) - left Utf8View short_result_long_input: 249.1 → 52.7 (−79%) - right Utf8 long_result: 109.4 → 91.8 (−16%) - right Utf8 n_exceeds_len: 97.9 → 98.5 (+1%) - right Utf8 negative_n: 91.5 → 84.7 (−7%) - right Utf8 per_row_n: 100.7 → 88.1 (−13%) - right Utf8 short_result: 96.8 → 87.2 (−10%) - right Utf8 short_result_long_input: 261.2 → 95.5 (−63%) - right Utf8View long_result: 64.2 → 53.8 (−16%) - right Utf8View n_exceeds_len: 63.9 → 56.0 (−12%) - right Utf8View negative_n: 62.3 → 55.8 (−10%) - right Utf8View per_row_n: 101.0 → 54.9 (−46%) - right Utf8View short_result: 62.9 → 58.7 (−7%) - right Utf8View short_result_long_input: 202.2 → 67.7 (−67%) ARM (Apple M4 Max) - left Utf8 long_result: 91.3 → 62.6 (−31%) - left Utf8 n_exceeds_len: 69.4 → 71.4 (+3%) - left Utf8 negative_n: 66.8 → 69.5 (+4%) - left Utf8 per_row_n: 66.6 → 60.6 (−9%) - left Utf8 short_result: 69.1 → 62.7 (−9%) - left Utf8 short_result_long_input: 91.1 → 59.6 (−35%) - left Utf8View long_result: 45.4 → 26.0 (−43%) - left Utf8View n_exceeds_len: 37.6 → 34.4 (−9%) - left Utf8View negative_n: 37.8 → 25.9 (−31%) - left Utf8View per_row_n: 68.1 → 29.4 (−57%) - left Utf8View short_result: 37.9 → 25.0 (−34%) - left Utf8View short_result_long_input: 61.1 → 28.9 (−53%) - right Utf8 long_result: 104.2 → 64.7 (−38%) - right Utf8 n_exceeds_len: 80.1 → 79.1 (−1%) - right Utf8 negative_n: 71.4 → 66.0 (−8%) - right Utf8 per_row_n: 74.0 → 64.3 (−13%) - right Utf8 short_result: 80.2 → 65.4 (−18%) - right Utf8 short_result_long_input: 101.1 → 63.4 (−37%) - right Utf8View long_result: 62.7 → 32.0 (−49%) - right Utf8View n_exceeds_len: 42.0 → 38.5 (−8%) - right Utf8View negative_n: 41.3 → 28.5 (−31%) - right Utf8View per_row_n: 72.0 → 33.5 (−53%) - right Utf8View short_result: 43.9 → 30.0 (−32%) - right Utf8View short_result_long_input: 63.7 → 33.3 (−48%) ## What changes are included in this PR? See above. ## What is the testing strategy for this PR? Existing tests pass; no functional changes. ## Are there any user-facing changes? No. -- 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]
