Skip to content

perf: Optimize find_in_set NULL handling#21464

Open
neilconway wants to merge 3 commits intoapache:mainfrom
neilconway:neilc/perf-find-in-set-nulls
Open

perf: Optimize find_in_set NULL handling#21464
neilconway wants to merge 3 commits intoapache:mainfrom
neilconway:neilc/perf-find-in-set-nulls

Conversation

@neilconway
Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

find_in_set uses PrimitiveArray::<T>::builder to construct its results, which means building the internal null buffer in an iterative fashion (via repeated append_null calls). It is more efficient to construct the null buffer directly, via NullBuffer::union (when multiple arguments might be NULL) or just cloning the input null buffer (when passed a single argument).

Benchmarks (ARM64):

  - find_in_set/string_len_8: 589.0 µs → 529.0 µs (-10.2%)
  - find_in_set/string_len_32: 736.2 µs → 660.5 µs (-10.3%)
  - find_in_set/string_len_1024: 7.5 ms → 7.4 ms (-1.3%)
  - find_in_set/string_view_len_8: 616.0 µs → 579.9 µs (-5.9%)
  - find_in_set/string_view_len_32: 748.0 µs → 701.9 µs (-6.2%)
  - find_in_set/string_view_len_1024: 7.6 ms → 7.6 ms (0.0%)
  - find_in_set_scalar/string_len_8: 76.7 µs → 48.0 µs (-37.4%)
  - find_in_set_scalar/string_len_32: 76.5 µs → 47.6 µs (-37.8%)
  - find_in_set_scalar/string_len_1024: 76.2 µs → 48.0 µs (-37.0%)
  - find_in_set_scalar/string_view_len_8: 81.9 µs → 55.7 µs (-32.0%)
  - find_in_set_scalar/string_view_len_32: 85.5 µs → 56.8 µs (-33.6%)
  - find_in_set_scalar/string_view_len_1024: 85.2 µs → 57.4 µs (-32.6%)

The change should be an improvement for both scalar and array cases. The relative improvement is larger in the scalar case because the scalar case is doing less work and so NULL handling was a larger fraction of the total runtime.

What changes are included in this PR?

  • Optimize NULL handling for both scalar and array arg cases

Are these changes tested?

Yes.

Are there any user-facing changes?

No.

@github-actions github-actions bot added the functions Changes to functions implementation label Apr 8, 2026
@neilconway
Copy link
Copy Markdown
Contributor Author

Btw I tried refactoring this to use an explicit for (so we can dispense with the zero dummy assignment), but it was a bit slower than the map approach:

  - find_in_set/string_len_8: 588.6 µs → 530.9 µs (-9.8%)
  - find_in_set/string_len_32: 735.2 µs → 669.5 µs (-8.9%)
  - find_in_set/string_len_1024: 7.5 ms → 7.4 ms (-1.3%)
  - find_in_set/string_view_len_8: 615.7 µs → 581.0 µs (-5.6%)
  - find_in_set/string_view_len_32: 743.8 µs → 708.9 µs (-4.7%)
  - find_in_set/string_view_len_1024: 7.6 ms → 7.6 ms (-0.0%)
  - find_in_set_scalar/string_len_8: 76.7 µs → 56.9 µs (-25.8%)
  - find_in_set_scalar/string_len_32: 76.7 µs → 56.8 µs (-25.9%)
  - find_in_set_scalar/string_len_1024: 76.4 µs → 56.9 µs (-25.5%)
  - find_in_set_scalar/string_view_len_8: 81.5 µs → 59.0 µs (-27.6%)
  - find_in_set_scalar/string_view_len_32: 85.3 µs → 59.4 µs (-30.4%)
  - find_in_set_scalar/string_view_len_1024: 85.0 µs → 59.2 µs (-30.4%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

functions Changes to functions implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Optimize NULL handling for find_in_set

1 participant