ishaaaan17 commented on PR #53160:
URL: https://github.com/apache/arrow/pull/53160#issuecomment-6223259699
hi @dmitry-chirkov-dremio,
thanks for the detailed review and guidance! i have addressed all the points
in two commits:
1. **thread-safe lazy initialization**
- fixed the race in `MatchSubstringState` using double-checked locking in
`GetOrInitMatcher`, with `std::atomic` acquire/release and a `std::mutex`:
- fast path (normal case across batches): only one atomic acquire load,
no mutex contention.
- slow path (first batch race): takes the mutex and does a relaxed
double check before building and storing the matcher.
- changed `MatchLike` to call `ExecWithState(ctx, batch, out,
like_state.get())` directly, so there is no `ctx->SetState()` change at runtime
anymore.
- fixed the formatting in `enum class LikeKind` so the clang-format CI
check passes.
2. **concurrent first-use tests**
- added `MatchSubstringConcurrentFirstUse` and
`MatchLikeConcurrentFirstUse` to `TestBaseBinaryKernels` in
`scalar_string_test.cc`.
- the test binds an expression that is not initialized yet, starts 16
threads that wait on an atomic barrier, and then all of them run
`ExecuteScalarExpression(bound_expr, batch)` on the first batch together, to
check there is no data race or use-after-free during initialization.
3. **multi-batch google benchmark**
- added `MatcherReusedBenchmark` in `scalar_string_benchmark.cc`, in a
separate commit on top as you asked.
- it uses `GetFunctionExecutor` with 1, 8 and 64 batches, keeping the
total input fixed at 1M elements. it covers plain substring
(`match_substring`), simple regex, expensive regex (`match_substring_regex`),
the LIKE fast path, and the LIKE regex fallback (`match_like`).
--
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]
