ishaaaan17 commented on code in PR #53160:
URL: https://github.com/apache/arrow/pull/53160#discussion_r4354483226
##########
cpp/src/arrow/compute/kernels/scalar_string_test.cc:
##########
@@ -1703,6 +1706,41 @@ TYPED_TEST(TestBaseBinaryKernels, MatchSubstring) {
&options_empty);
}
+TYPED_TEST(TestBaseBinaryKernels, MatchSubstringConcurrentFirstUse) {
+ auto in_schema = schema({field("x", this->type())});
+ MatchSubstringOptions options{"ab"};
+ auto expr = call("match_substring", {field_ref("x")}, options);
+ ASSERT_OK_AND_ASSIGN(auto bound_expr, expr.Bind(*in_schema));
+
+ Datum input =
+ ArrayFromJSON(this->type(), R"(["abc", "acb", "cab", null, "bac",
"AB"])");
+ ExecBatch batch({input}, 6);
+
+ constexpr int kNumThreads = 16;
+ std::vector<std::thread> threads;
+ std::vector<Result<Datum>> results(kNumThreads);
+ std::atomic<bool> ready{false};
+
+ for (int i = 0; i < kNumThreads; ++i) {
Review Comment:
thanks for pointing this out @lriggs! you are right about the thread
scheduling delay, and about making sure all the threads really race on the
first initialization together.
i have changed the test synchronization to use an atomic barrier: a
`waiting` counter and a `go` flag with acquire/release semantics, and
`std::this_thread::yield()` inside the spin-waits so no core is starved. i made
the same change in `MatchLikeConcurrentFirstUse`.
pushed the updated revision.
--
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]
