Skip to content

Commit 888c08a

Browse files
apronchenkovcopybara-github
authored andcommitted
Refactor CancellationContext
This change makes the countdown period non-configurable (motivation: it needs to match the clock implementation, which is already non-configurable) and allows grouping multiple SoftCheck() calls into a single SoftCheck(n) call. Benchmarks: name cpu/op BM_CancellationContext_SoftCheck<1> 0.97ns ± 1% BM_CancellationContext_SoftCheck<2> 1.45ns ± 1% BM_CancellationContext_SoftCheck<4> 2.86ns ± 1% BM_CancellationContext_SoftCheck<8> 5.61ns ± 1% BM_CancellationContext_SoftCheck<-1> 11.4ns ± 1% PiperOrigin-RevId: 721316028 Change-Id: I78bf9bcc7f9830dc6f32e4698ec597d86e7024bf
1 parent b485841 commit 888c08a

File tree

3 files changed

+27
-29
lines changed

3 files changed

+27
-29
lines changed

koladata/functor/BUILD

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,6 @@ cc_test(
254254
"@com_google_absl//absl/status:status_matchers",
255255
"@com_google_absl//absl/status:statusor",
256256
"@com_google_absl//absl/strings:string_view",
257-
"@com_google_absl//absl/time",
258257
"@com_google_arolla//arolla/dense_array",
259258
"@com_google_arolla//arolla/expr",
260259
"@com_google_arolla//arolla/jagged_shape/testing",
@@ -297,7 +296,6 @@ cc_test(
297296
"@com_google_absl//absl/status:status_matchers",
298297
"@com_google_absl//absl/status:statusor",
299298
"@com_google_absl//absl/strings:string_view",
300-
"@com_google_absl//absl/time",
301299
"@com_google_arolla//arolla/expr",
302300
"@com_google_arolla//arolla/expr/operators/all",
303301
"@com_google_arolla//arolla/qexpr/operators/all",

koladata/functor/call_test.cc

Lines changed: 18 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
#include "absl/status/status_matchers.h"
2727
#include "absl/status/statusor.h"
2828
#include "absl/strings/string_view.h"
29-
#include "absl/time/time.h"
3029
#include "koladata/data_slice.h"
3130
#include "koladata/expr/expr_eval.h"
3231
#include "koladata/functor/functor.h"
@@ -244,37 +243,33 @@ TEST(CallTest, Cancellation) {
244243
}));
245244
ASSERT_OK_AND_ASSIGN(auto koda_signature,
246245
CppSignatureToKodaSignature(signature));
247-
// returns_expr = I.x + I.x + I.x
248-
ASSERT_OK_AND_ASSIGN(
249-
auto returns_expr,
250-
WrapExpr(arolla::expr::CallOp(
251-
"math.add", {arolla::expr::CallOp(
252-
"math.add", {CreateInput("a"), CreateInput("a")}),
253-
CreateInput("a")})));
254-
ASSERT_OK_AND_ASSIGN(auto fn,
255-
CreateFunctor(returns_expr, koda_signature, {}));
256-
246+
const auto gen_returns_expr = [](int op_count) {
247+
auto result = CreateInput("a");
248+
for (int i = 0; i < op_count; ++i) {
249+
result = arolla::expr::CallOp("math.add", {result, CreateInput("a")});
250+
}
251+
return WrapExpr(result);
252+
};
257253
{
258-
int op_count = 2; // Stop after the second operator.
254+
const int op_count = 512; // Long enough computation.
255+
ASSERT_OK_AND_ASSIGN(auto returns_expr, gen_returns_expr(op_count));
256+
ASSERT_OK_AND_ASSIGN(auto fn,
257+
CreateFunctor(returns_expr, koda_signature, {}));
259258
auto cancel_ctx = arolla::CancellationContext::Make(
260-
/*no cooldown*/ absl::Nanoseconds(-1),
261-
/*no countdown*/ -1, [&op_count] {
262-
return --op_count > 0 ? absl::OkStatus() : absl::CancelledError("");
263-
});
259+
/*no cooldown period*/ {}, [] { return absl::CancelledError(""); });
264260
expr::EvalOptions eval_options{.cancellation_context = cancel_ctx.get()};
265261
EXPECT_THAT(CallFunctorWithCompilationCache(
266262
fn, /*args=*/{arolla::TypedRef::FromValue(1)},
267263
/*kwnames=*/{}, eval_options),
268264
StatusIs(absl::StatusCode::kCancelled));
269265
}
270-
{
271-
int op_count = 3; // Should stop after the third operator;
272-
// however, there are only two operators.
266+
{ // The computation is insufficiently long to detect cancellation.
267+
const int op_count = 2;
268+
ASSERT_OK_AND_ASSIGN(auto returns_expr, gen_returns_expr(op_count));
269+
ASSERT_OK_AND_ASSIGN(auto fn,
270+
CreateFunctor(returns_expr, koda_signature, {}));
273271
auto cancel_ctx = arolla::CancellationContext::Make(
274-
/*no cooldown*/ absl::Nanoseconds(-1),
275-
/*no countdown*/ -1, [&op_count] {
276-
return --op_count > 0 ? absl::OkStatus() : absl::CancelledError("");
277-
});
272+
/*no cooldown period*/ {}, [] { return absl::CancelledError(""); });
278273
expr::EvalOptions eval_options{.cancellation_context = cancel_ctx.get()};
279274
ASSERT_OK_AND_ASSIGN(auto result,
280275
CallFunctorWithCompilationCache(

koladata/functor/map_test.cc

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
#include "absl/status/status_matchers.h"
2929
#include "absl/status/statusor.h"
3030
#include "absl/strings/string_view.h"
31-
#include "absl/time/time.h"
3231
#include "koladata/data_bag.h"
3332
#include "koladata/data_slice.h"
3433
#include "koladata/expr/expr_eval.h"
@@ -171,11 +170,17 @@ TEST(MapTest, Cancellation) {
171170
ASSERT_OK_AND_ASSIGN(auto fn,
172171
CreateFunctor(returns_expr, koda_signature, {}));
173172

173+
std::vector<int> items(512); // Prepare a long enough test slice.
174+
std::iota(items.begin(), items.end(), 0);
175+
ASSERT_OK_AND_ASSIGN(
176+
auto test_slice,
177+
DataSlice::Create(
178+
internal::DataSliceImpl::Create(arolla::CreateFullDenseArray(items)),
179+
DataSlice::JaggedShape::FlatFromSize(items.size()),
180+
internal::DataItem(schema::GetDType<int>())));
174181
auto cancel_ctx = arolla::CancellationContext::Make(
175-
/*no cooldown*/ absl::Nanoseconds(-1),
176-
/*no countdown*/ -1, [] { return absl::CancelledError(""); });
182+
/*no cooldown*/ {}, [] { return absl::CancelledError(""); });
177183
expr::EvalOptions eval_options{.cancellation_context = cancel_ctx.get()};
178-
auto test_slice = test::DataSlice<int>({1, 2, 3});
179184
EXPECT_THAT(MapFunctorWithCompilationCache(
180185
fn, /*args=*/{test_slice},
181186
/*kwnames=*/{}, /*include_missing=*/false, eval_options),

0 commit comments

Comments
 (0)