Skip to content

Commit

Permalink
cross-tree: convert deprecated later() to yield()
Browse files Browse the repository at this point in the history
A recent patch deprecated the later() function, splitting its work to
yield() and check_for_io_immediately(). The result was numerous
deprecation warnings when building Seastar and its tests, so this patch
sets out replace every call to later() by yield() - because as far as I
can tell, non of the callers of later() needed check_for_io_immediately().

While doing this, I noticed that some of the code using later() (and now
yield()) were missing an explicit include of <seastar/util/later.hh>.
This is something we'll probably regret later, so I took the opportunity
to also add the missing include.

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Message-Id: <20220110160202.573719-1-nyh@scylladb.com>
  • Loading branch information
nyh authored and avikivity committed Jan 10, 2022
1 parent b3984f8 commit ae8d1c2
Show file tree
Hide file tree
Showing 10 changed files with 58 additions and 56 deletions.
8 changes: 4 additions & 4 deletions src/core/file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -775,8 +775,8 @@ append_challenged_posix_file_impl::commit_size(uint64_t size) noexcept {
future<size_t>
append_challenged_posix_file_impl::read_dma(uint64_t pos, void* buffer, size_t len, const io_priority_class& pc, io_intent* intent) noexcept {
if (pos >= _logical_size) {
// later() avoids tail recursion
return later().then([] {
// yield() avoids tail recursion
return yield().then([] {
return size_t(0);
});
}
Expand All @@ -795,8 +795,8 @@ append_challenged_posix_file_impl::read_dma(uint64_t pos, void* buffer, size_t l
future<size_t>
append_challenged_posix_file_impl::read_dma(uint64_t pos, std::vector<iovec> iov, const io_priority_class& pc, io_intent* intent) noexcept {
if (pos >= _logical_size) {
// later() avoids tail recursion
return later().then([] {
// yield() avoids tail recursion
return yield().then([] {
return size_t(0);
});
}
Expand Down
2 changes: 1 addition & 1 deletion tests/perf/future_util_perf.cc
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ PERF_TEST_F(parallel_for_each, immediate)
future<> suspend(int v, int& vs)
{
vs += v;
return later();
return yield();
}

PERF_TEST_F(parallel_for_each, suspend)
Expand Down
4 changes: 2 additions & 2 deletions tests/perf/perf_tests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ void performance_test::do_run(const config& conf)
// dry run, estimate the number of iterations
if (conf.single_run_duration.count()) {
// switch out of seastar thread
later().then([&] {
yield().then([&] {
tmr.arm(conf.single_run_duration);
return do_single_run().finally([&] {
tmr.cancel();
Expand All @@ -235,7 +235,7 @@ void performance_test::do_run(const config& conf)
uint64_t total_iterations = 0;
for (auto i = 0u; i < conf.number_of_runs; i++) {
// switch out of seastar thread
later().then([&] {
yield().then([&] {
_single_run_iterations = 0;
return do_single_run().then([&] (clock_type::duration dt) {
double ns = std::chrono::duration_cast<std::chrono::nanoseconds>(dt).count();
Expand Down
13 changes: 7 additions & 6 deletions tests/unit/coroutines_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <seastar/core/future-util.hh>
#include <seastar/testing/test_case.hh>
#include <seastar/core/sleep.hh>
#include <seastar/util/later.hh>

using namespace seastar;
using namespace std::chrono_literals;
Expand All @@ -41,13 +42,13 @@ SEASTAR_TEST_CASE(test_coroutines_not_compiled_in) {
namespace {

future<int> old_fashioned_continuations() {
return later().then([] {
return yield().then([] {
return 42;
});
}

future<int> simple_coroutine() {
co_await later();
co_await yield();
co_return 53;
}

Expand All @@ -60,7 +61,7 @@ future<std::tuple<int, double>> tuple_coroutine() {
}

future<int> failing_coroutine() {
co_await later();
co_await yield();
throw 42;
}

Expand All @@ -69,7 +70,7 @@ future<int> failing_coroutine() {
}

future<int> failing_coroutine2() noexcept {
co_await later();
co_await yield();
co_return throw_exception(17);
}

Expand Down Expand Up @@ -157,7 +158,7 @@ SEASTAR_TEST_CASE(test_scheduling_group) {
SEASTAR_TEST_CASE(test_preemption) {
bool x = false;
unsigned preempted = 0;
auto f = later().then([&x] {
auto f = yield().then([&x] {
x = true;
});

Expand All @@ -169,7 +170,7 @@ SEASTAR_TEST_CASE(test_preemption) {
co_await make_ready_future<>();
}
auto save_x = x;
// wait for later() to complete
// wait for yield() to complete
co_await std::move(f);
BOOST_REQUIRE(save_x);
co_return;
Expand Down
10 changes: 5 additions & 5 deletions tests/unit/expiring_fifo_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ SEASTAR_TEST_CASE(test_expiry_operations) {
BOOST_REQUIRE_EQUAL(fifo.front(), 1);

manual_clock::advance(1s);
later().get();
yield().get();

BOOST_REQUIRE(fifo.empty());
BOOST_REQUIRE_EQUAL(fifo.size(), 0u);
Expand All @@ -108,7 +108,7 @@ SEASTAR_TEST_CASE(test_expiry_operations) {
fifo.push_back(3);

manual_clock::advance(1s);
later().get();
yield().get();

BOOST_REQUIRE(!fifo.empty());
BOOST_REQUIRE_EQUAL(fifo.size(), 2u);
Expand All @@ -130,7 +130,7 @@ SEASTAR_TEST_CASE(test_expiry_operations) {
fifo.push_back(4, manual_clock::now() + 2s);

manual_clock::advance(1s);
later().get();
yield().get();

BOOST_REQUIRE(!fifo.empty());
BOOST_REQUIRE_EQUAL(fifo.size(), 2u);
Expand All @@ -154,7 +154,7 @@ SEASTAR_TEST_CASE(test_expiry_operations) {
fifo.push_back(4, manual_clock::now() + 1s);

manual_clock::advance(1s);
later().get();
yield().get();

BOOST_REQUIRE(!fifo.empty());
BOOST_REQUIRE_EQUAL(fifo.size(), 1u);
Expand All @@ -177,7 +177,7 @@ SEASTAR_TEST_CASE(test_expiry_operations) {
fifo.push_back(5);

manual_clock::advance(1s);
later().get();
yield().get();

BOOST_REQUIRE_EQUAL(fifo.size(), 2u);
BOOST_REQUIRE_EQUAL(fifo.front(), 1);
Expand Down
24 changes: 12 additions & 12 deletions tests/unit/fair_queue_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ SEASTAR_THREAD_TEST_CASE(test_fair_queue_equal_2classes) {
env.do_op(b, 1);
}

later().get();
yield().get();
// allow half the requests in
env.tick(100);
env.verify("equal_2classes", {1, 1});
Expand All @@ -213,7 +213,7 @@ SEASTAR_THREAD_TEST_CASE(test_fair_queue_equal_4classes) {
env.do_op(c, 1);
env.do_op(d, 1);
}
later().get();
yield().get();
// allow half the requests in
env.tick(200);
env.verify("equal_4classes", {1, 1, 1, 1});
Expand All @@ -230,7 +230,7 @@ SEASTAR_THREAD_TEST_CASE(test_fair_queue_different_shares) {
env.do_op(a, 1);
env.do_op(b, 1);
}
later().get();
yield().get();
// allow half the requests in
env.tick(100);
return env.verify("different_shares", {1, 2});
Expand All @@ -250,7 +250,7 @@ SEASTAR_THREAD_TEST_CASE(test_fair_queue_equal_hi_capacity_2classes) {
env.do_op(a, 1);
env.do_op(b, 1);
}
later().get();
yield().get();

// queue has capacity 10, 10 x 10 = 100, allow half the requests in
env.tick(10);
Expand All @@ -272,7 +272,7 @@ SEASTAR_THREAD_TEST_CASE(test_fair_queue_different_shares_hi_capacity) {
env.do_op(a, 1);
env.do_op(b, 1);
}
later().get();
yield().get();
// queue has capacity 10, 10 x 10 = 100, allow half the requests in
env.tick(10);
env.verify("different_shares_hi_capacity", {1, 2});
Expand All @@ -289,7 +289,7 @@ SEASTAR_THREAD_TEST_CASE(test_fair_queue_different_weights) {
env.do_op(a, 2);
env.do_op(b, 1);
}
later().get();
yield().get();
// allow half the requests in
env.tick(100);
env.verify("different_weights", {1, 2});
Expand All @@ -305,7 +305,7 @@ SEASTAR_THREAD_TEST_CASE(test_fair_queue_dominant_queue) {
for (int i = 0; i < 100; ++i) {
env.do_op(b, 1);
}
later().get();
yield().get();

// consume all requests
env.tick(100);
Expand Down Expand Up @@ -335,7 +335,7 @@ SEASTAR_THREAD_TEST_CASE(test_fair_queue_forgiving_queue) {
for (int i = 0; i < 100; ++i) {
env.do_op(a, 1);
}
later().get();
yield().get();

// consume all requests
env.tick(100);
Expand All @@ -345,7 +345,7 @@ SEASTAR_THREAD_TEST_CASE(test_fair_queue_forgiving_queue) {
env.do_op(a, 1);
env.do_op(b, 1);
}
later().get();
yield().get();

// allow half the requests in
env.tick(100);
Expand All @@ -366,13 +366,13 @@ SEASTAR_THREAD_TEST_CASE(test_fair_queue_update_shares) {
env.do_op(b, 1);
}

later().get();
yield().get();
// allow 25% of the requests in
env.tick(250);
env.update_shares(a, 10);
env.update_shares(b, 20);

later().get();
yield().get();
// allow 25% of the requests in
env.tick(250);
env.verify("update_shares", {1, 1}, 2);
Expand Down Expand Up @@ -441,7 +441,7 @@ SEASTAR_THREAD_TEST_CASE(test_fair_queue_random_run) {
env.do_op(b, 1);
}

later().get();
yield().get();
// In total allow half the requests in
env.tick(reqs);

Expand Down
Loading

0 comments on commit ae8d1c2

Please sign in to comment.