GH-50615: [C++] Reduce code generation for string kernels

[pitrou]

Rationale for this change

Some string kernels are generated twice for the corresponding binary and string type (for example binary/utf8, large_binary, large_utf8). Most of the time we can cut down on the code duplication to reduce compile times and binary size slightly.

What changes are included in this PR?

Only instantiate string kernels for base binary types, not the string derived types.

This makes libarrow_compute somehow smaller on my system:

• before:

$ size `find /build/ -name libarrow_compute.so.2600.0.0`
   text    data     bss     dec     hex filename
13551567         113096   46432 13711095         d136f7 /build/build-release/relwithdebinfo/libarrow_compute.so.2600.0.0

• after:

$ size `find /build/ -name libarrow_compute.so.2600.0.0`
   text    data     bss     dec     hex filename
13363474         112128   44968 13520570         ce4eba /build/build-release/relwithdebinfo/libarrow_compute.so.2600.0.0

Are these changes tested?

Yes, including additional tests to check that utf8-ness of the input is still propagated correctly.

Are there any user-facing changes?

No.

• GitHub Issue: [C++] Reduce code generation for string kernels #50615

[pitrou]

@fenfeng9 Would you like to review this?

[zanmato1984]

Overall this looks good to me. I found one small non-blocking test coverage gap that may be worth closing before merge.

This PR now relies on the runtime input type to choose UTF-8 vs Latin-1 semantics after string/binary kernels share the same physical binary implementation. The new tests cover this well for the regex-named kernels, but I don’t think they directly cover two related paths:

1. match_like, especially the new MatchLikeConstants::Instance(is_utf8) two-slot cache.
2. Literal ignore-case kernels such as match_substring, starts_with, ends_with, find_substring, and count_substring.

A small binary-vs-utf8 non-ASCII case would help prove that the runtime is_utf8 selection cannot cross wires. For example, pattern = "ÀB" or "%ÀB%" with ignore_case = true should not match lowercase àb on binary input, but should match it on utf8 input.

[zanmato1984]

Could we add a targeted binary-vs-utf8 test for this runtime selection? Since string and binary now share the same physical MatchLike implementation, this cache is the one place where accidentally selecting the wrong UTF-8 / Latin-1 slot would be hard to catch with ASCII-only inputs.

For example, pattern = "%ÀB%" with ignore_case = true should not match lowercase àb on binary input, but should match it on utf8 input.

[zanmato1984]

Optional readability nit: could we use default member initializers for these RE2 members so each regex literal stays next to the comment explaining it? The comments and regexes are tightly coupled because each one classifies a LIKE pattern that can be rewritten as substring / prefix / suffix search. Keeping them together would make these hard-to-read regexes easier to audit.

[pitrou]

Done.

[zanmato1984]

Optional nit: could init_status be a local in Make() instead of a MatchLikeConstants member? It appears to be construction-only state, so narrowing its scope would make the struct state tighter.

[pitrou]

Done.

[fenfeng9]

Thanks for the detailed review, @zanmato1984. I agree that the suggested additional test coverage would be useful. I don’t have any additional comments.

[pitrou]

@zanmato1984 I've added test for non-ASCII case-folding and addressed your other comments, do you want to take a look again?

[zanmato1984]

Thanks, this addresses my comments.

[conbench-apache-arrow]

After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 374db36.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 4 possible false positives for unstable benchmarks that are known to sometimes produce them.

[conbench-apache-arrow]

After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 374db36.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 4 possible false positives for unstable benchmarks that are known to sometimes produce them.