Skip to content

Add source location to task and tasktrace object #2707

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions include/seastar/core/coroutine.hh
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

#ifndef SEASTAR_MODULE
#include <coroutine>
#include <source_location>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it took me a while to check if it's safe to use <source_location> instead of util/std-compat.hh . the answer is yes. as we support the latest two major versions of the GCC and Clang compilers. in general, we use libstdc++, which added the support to source_location in gcc-mirror/gcc@57d76ee, which is included by gcc 12 and up. and the latest stable release of GCC is GCC 14.2.

#endif

namespace seastar {
Expand Down Expand Up @@ -144,7 +145,8 @@ public:
}

template<typename U>
void await_suspend(std::coroutine_handle<U> hndl) noexcept {
void await_suspend(std::coroutine_handle<U> hndl, std::source_location sl = std::source_location::current()) noexcept {
hndl.promise().update_resume_point(sl);
if (!CheckPreempt || !_future.available()) {
_future.set_coroutine(hndl.promise());
} else {
Expand All @@ -169,7 +171,8 @@ public:
}

template<typename U>
void await_suspend(std::coroutine_handle<U> hndl) noexcept {
void await_suspend(std::coroutine_handle<U> hndl, std::source_location sl = std::source_location::current()) noexcept {
hndl.promise().update_resume_point(sl);
if (!CheckPreempt || !_future.available()) {
_future.set_coroutine(hndl.promise());
} else {
Expand Down
64 changes: 33 additions & 31 deletions include/seastar/core/future.hh
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include <stdexcept>
#include <type_traits>
#include <utility>
#include <source_location>
#endif

#include <seastar/core/task.hh>
Expand Down Expand Up @@ -1231,13 +1232,14 @@ private:
future_base::schedule(tws, &tws->_state);
}
template <typename Pr, typename Func, typename Wrapper>
void schedule(Pr&& pr, Func&& func, Wrapper&& wrapper) noexcept {
void schedule(Pr&& pr, Func&& func, Wrapper&& wrapper, std::source_location sl) noexcept {
// If this new throws a std::bad_alloc there is nothing that
// can be done about it. The corresponding future is not ready
// and we cannot break the chain. Since this function is
// noexcept, it will call std::terminate if new throws.
memory::scoped_critical_alloc_section _;
auto tws = new continuation<Pr, Func, Wrapper, T>(std::move(pr), std::move(func), std::move(wrapper));
tws->update_resume_point(sl);
// In a debug build we schedule ready futures, but not in
// other build modes.
#ifdef SEASTAR_DEBUG
Expand Down Expand Up @@ -1355,9 +1357,9 @@ public:
requires std::invocable<Func, T>
|| (std::same_as<void, T> && std::invocable<Func>)
Result
then(Func&& func) noexcept {
then(Func&& func, std::source_location sl = std::source_location::current()) noexcept {
#ifndef SEASTAR_TYPE_ERASE_MORE
return then_impl(std::move(func));
return then_impl(std::move(func), sl);
#else
using func_type = typename internal::future_result<Func, T>::func_type;
noncopyable_function<func_type> ncf;
Expand All @@ -1367,7 +1369,7 @@ public:
return futurize_invoke(func, std::forward<decltype(args)>(args)...);
});
}
return then_impl(std::move(ncf));
return then_impl(std::move(ncf), sl);
#endif
}

Expand All @@ -1393,18 +1395,18 @@ public:
template <typename Func, typename Result = futurize_t<internal::result_of_apply_t<Func, T>>>
requires ::seastar::CanApplyTuple<Func, T>
Result
then_unpack(Func&& func) noexcept {
then_unpack(Func&& func, std::source_location sl = std::source_location::current()) noexcept {
return then([func = std::forward<Func>(func)] (T&& tuple) mutable {
// sizeof...(tuple) is required to be 1
return std::apply(func, std::move(tuple));
});
}, sl);
}

private:

// Keep this simple so that Named Return Value Optimization is used.
template <typename Func, typename Result>
Result then_impl_nrvo(Func&& func) noexcept {
Result then_impl_nrvo(Func&& func, std::source_location sl) noexcept {
using futurator = futurize<internal::future_result_t<Func, T>>;
typename futurator::type fut(future_for_get_promise_marker{});
using pr_type = decltype(fut.get_promise());
Expand All @@ -1419,13 +1421,13 @@ private:
return internal::future_invoke(func, std::move(state).get_value());
});
}
});
}, sl);
return fut;
}

template <typename Func, typename Result = futurize_t<internal::future_result_t<Func, T>>>
Result
then_impl(Func&& func) noexcept {
then_impl(Func&& func, std::source_location sl) noexcept {
#ifndef SEASTAR_DEBUG
using futurator = futurize<internal::future_result_t<Func, T>>;
if (failed()) {
Expand All @@ -1434,7 +1436,7 @@ private:
return futurator::invoke(std::forward<Func>(func), get_available_state_ref().take_value());
}
#endif
return then_impl_nrvo<Func, Result>(std::forward<Func>(func));
return then_impl_nrvo<Func, Result>(std::forward<Func>(func), sl);
}

public:
Expand All @@ -1455,23 +1457,23 @@ public:
/// to the eventual value of this future.
template <std::invocable<future> Func, typename FuncResult = std::invoke_result_t<Func, future>>
futurize_t<FuncResult>
then_wrapped(Func&& func) & noexcept {
return then_wrapped_maybe_erase<false, FuncResult>(std::forward<Func>(func));
then_wrapped(Func&& func, std::source_location sl = std::source_location::current()) & noexcept {
return then_wrapped_maybe_erase<false, FuncResult>(std::forward<Func>(func), sl);
}

template <std::invocable<future&&> Func, typename FuncResult = std::invoke_result_t<Func, future&&>>
futurize_t<FuncResult>
then_wrapped(Func&& func) && noexcept {
return then_wrapped_maybe_erase<true, FuncResult>(std::forward<Func>(func));
then_wrapped(Func&& func, std::source_location sl = std::source_location::current()) && noexcept {
return then_wrapped_maybe_erase<true, FuncResult>(std::forward<Func>(func), sl);
}

private:

template <bool AsSelf, typename FuncResult, typename Func>
futurize_t<FuncResult>
then_wrapped_maybe_erase(Func&& func) noexcept {
then_wrapped_maybe_erase(Func&& func, std::source_location sl) noexcept {
#ifndef SEASTAR_TYPE_ERASE_MORE
return then_wrapped_common<AsSelf, FuncResult>(std::forward<Func>(func));
return then_wrapped_common<AsSelf, FuncResult>(std::forward<Func>(func), sl);
#else
using futurator = futurize<FuncResult>;
using WrapFuncResult = typename futurator::type;
Expand All @@ -1482,29 +1484,29 @@ private:
return futurator::invoke(func, std::move(f));
});
}
return then_wrapped_common<AsSelf, WrapFuncResult>(std::move(ncf));
return then_wrapped_common<AsSelf, WrapFuncResult>(std::move(ncf), sl);
#endif
}

// Keep this simple so that Named Return Value Optimization is used.
template <typename FuncResult, typename Func>
futurize_t<FuncResult>
then_wrapped_nrvo(Func&& func) noexcept {
then_wrapped_nrvo(Func&& func, std::source_location sl) noexcept {
using futurator = futurize<FuncResult>;
typename futurator::type fut(future_for_get_promise_marker{});
using pr_type = decltype(fut.get_promise());
schedule(fut.get_promise(), std::move(func), [](pr_type&& pr, Func& func, future_state&& state) {
futurator::satisfy_with_result_of(std::move(pr), [&func, &state] {
return func(future(std::move(state)));
});
});
}, sl);
return fut;
}


template <bool AsSelf, typename FuncResult, typename Func>
futurize_t<FuncResult>
then_wrapped_common(Func&& func) noexcept {
then_wrapped_common(Func&& func, std::source_location sl) noexcept {
#ifndef SEASTAR_DEBUG
using futurator = futurize<FuncResult>;
if (available()) {
Expand All @@ -1518,7 +1520,7 @@ private:
}
}
#endif
return then_wrapped_nrvo<FuncResult, Func>(std::forward<Func>(func));
return then_wrapped_nrvo<FuncResult, Func>(std::forward<Func>(func), sl);
}

void forward_to(internal::promise_base_with_type<T>&& pr) noexcept {
Expand Down Expand Up @@ -1570,8 +1572,8 @@ public:
* nested will be propagated.
*/
template <std::invocable Func>
future<T> finally(Func&& func) noexcept {
return then_wrapped(finally_body<Func, is_future<std::invoke_result_t<Func>>::value>(std::forward<Func>(func)));
future<T> finally(Func&& func, std::source_location sl = std::source_location::current()) noexcept {
return then_wrapped(finally_body<Func, is_future<std::invoke_result_t<Func>>::value>(std::forward<Func>(func)), sl);
}


Expand Down Expand Up @@ -1617,24 +1619,24 @@ public:
///
/// Terminates the entire program is this future resolves
/// to an exception. Use with caution.
future<> or_terminate() noexcept {
future<> or_terminate(std::source_location sl = std::source_location::current()) noexcept {
return then_wrapped([] (auto&& f) {
try {
f.get();
} catch (...) {
engine_exit(std::current_exception());
}
});
}, sl);
}

/// \brief Discards the value carried by this future.
///
/// Converts the future into a no-value \c future<>, by
/// ignoring any result. Exceptions are propagated unchanged.
future<> discard_result() noexcept {
future<> discard_result(std::source_location sl = std::source_location::current()) noexcept {
// We need the generic variadic lambda, below, because then() behaves differently
// when value_type is when_all_succeed_tuple
return then([] (auto&&...) {});
return then([] (auto&&...) {}, sl);
}

/// \brief Handle the exception carried by this future.
Expand All @@ -1655,15 +1657,15 @@ public:
|| (std::tuple_size_v<tuple_type> == 0 && std::is_invocable_r_v<void, Func, std::exception_ptr>)
|| (std::tuple_size_v<tuple_type> == 1 && std::is_invocable_r_v<T, Func, std::exception_ptr>)
|| (std::tuple_size_v<tuple_type> > 1 && std::is_invocable_r_v<tuple_type ,Func, std::exception_ptr>)
future<T> handle_exception(Func&& func) noexcept {
future<T> handle_exception(Func&& func, std::source_location sl = std::source_location::current()) noexcept {
return then_wrapped([func = std::forward<Func>(func)]
(auto&& fut) mutable -> future<T> {
if (!fut.failed()) {
return make_ready_future<T>(fut.get());
} else {
return futurize_invoke(func, fut.get_exception());
}
});
}, sl);
}

/// \brief Handle the exception of a certain type carried by this future.
Expand All @@ -1677,7 +1679,7 @@ public:
/// If exception, that future holds, does not match func parameter type
/// it is propagated as is.
template <typename Func>
future<T> handle_exception_type(Func&& func) noexcept {
future<T> handle_exception_type(Func&& func, std::source_location sl = std::source_location::current()) noexcept {
using trait = function_traits<Func>;
static_assert(trait::arity == 1, "func can take only one parameter");
using ex_type = typename trait::template arg<0>::type;
Expand All @@ -1688,7 +1690,7 @@ public:
} catch(ex_type& ex) {
return futurize_invoke(func, ex);
}
});
}, sl);
}

/// \brief Ignore any result hold by this future
Expand Down
5 changes: 5 additions & 0 deletions include/seastar/core/task.hh
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

#ifndef SEASTAR_MODULE
#include <utility>
#include <source_location>
#endif

namespace seastar {
Expand All @@ -35,6 +36,8 @@ class task {
protected:
scheduling_group _sg;
private:
std::source_location resume_point = {};

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is making a very heavily used object larger.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After adding std::source_location

coroutine_traits_base<>::promise_type (which inherits from task) is 64 bytes,
continuation<Promise, Func, Wrapper, T> is around 104-120, depending on exact version.

No difference except for that single unlucky value, who is "promoted" to the next 64 byte cache line in case of continuation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's okay.

#ifdef SEASTAR_TASK_BACKTRACE
shared_backtrace _bt;
#endif
Expand All @@ -53,6 +56,8 @@ public:
virtual void run_and_dispose() noexcept = 0;
/// Returns the next task which is waiting for this task to complete execution, or nullptr.
virtual task* waiting_task() noexcept = 0;
void update_resume_point(std::source_location sl) { resume_point = sl; }
auto get_resume_point() const { return resume_point; }
scheduling_group group() const { return _sg; }
shared_backtrace get_backtrace() const;
#ifdef SEASTAR_TASK_BACKTRACE
Expand Down
4 changes: 3 additions & 1 deletion include/seastar/coroutine/all.hh
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,9 @@ private:
return (... && futures.available());
}, state._futures);
}
void await_suspend(coroutine_handle_t h) {
template <typename U>
void await_suspend(std::coroutine_handle<U> h, std::source_location sl = std::source_location::current()) {
h.promise().update_resume_point(sl);
when_ready = h;
process<0>();
}
Expand Down
4 changes: 3 additions & 1 deletion include/seastar/coroutine/as_future.hh
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#pragma once

#include <seastar/core/coroutine.hh>
#include <source_location>

namespace seastar {

Expand All @@ -42,7 +43,8 @@ public:
}

template<typename U>
void await_suspend(std::coroutine_handle<U> hndl) noexcept {
void await_suspend(std::coroutine_handle<U> hndl, std::source_location sl = std::source_location::current()) noexcept {
hndl.promise().update_resume_point(sl);
if (!CheckPreempt || !_future.available()) {
_future.set_coroutine(hndl.promise());
} else {
Expand Down
4 changes: 3 additions & 1 deletion include/seastar/coroutine/exception.hh
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include <seastar/core/future.hh>
#include <coroutine>
#include <exception>
#include <source_location>

namespace seastar {

Expand All @@ -42,7 +43,8 @@ struct exception_awaiter {
}

template<typename U>
void await_suspend(std::coroutine_handle<U> hndl) noexcept {
void await_suspend(std::coroutine_handle<U> hndl, std::source_location sl = std::source_location::current()) noexcept {
hndl.promise().update_resume_point(sl);
hndl.promise().set_exception(std::move(eptr));
hndl.destroy();
}
Expand Down
8 changes: 5 additions & 3 deletions include/seastar/coroutine/generator.hh
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ public:
return _future.available();
}

coroutine_handle<> await_suspend(coroutine_handle<promise_type> coro) noexcept;
coroutine_handle<> await_suspend(coroutine_handle<promise_type> coro, std::source_location sl = std::source_location::current()) noexcept;
void await_resume() noexcept { }
};

Expand Down Expand Up @@ -260,7 +260,8 @@ public:
}

template<typename Promise>
void await_suspend(coroutine_handle<Promise> coro) noexcept {
void await_suspend(coroutine_handle<Promise> coro, std::source_location sl = std::source_location::current()) noexcept {
coro.promise().update_resume_point(sl);
auto& current_task = coro.promise();
if (_next_value_future.available()) {
seastar::schedule(&current_task);
Expand Down Expand Up @@ -527,7 +528,8 @@ auto generator_buffered_promise<T, Container>::get_return_object() noexcept -> g

template<typename T, template <typename> class Container>
coroutine_handle<> yield_awaiter<T, Container>::await_suspend(
coroutine_handle<generator_buffered_promise<T, Container>> coro) noexcept {
coroutine_handle<generator_buffered_promise<T, Container>> coro, std::source_location sl) noexcept {
coro.promise().update_resume_point(sl);
if (_future.available()) {
auto& current_task = coro.promise();
seastar::schedule(&current_task);
Expand Down
3 changes: 2 additions & 1 deletion include/seastar/coroutine/maybe_yield.hh
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ struct maybe_yield_awaiter final {
}

template <typename T>
void await_suspend(std::coroutine_handle<T> h) {
void await_suspend(std::coroutine_handle<T> h, std::source_location sl = std::source_location::current()) noexcept {
h.promise().update_resume_point(sl);
schedule(&h.promise());
}

Expand Down
3 changes: 2 additions & 1 deletion include/seastar/coroutine/parallel_for_each.hh
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,8 @@ public:
}

template<typename T>
void await_suspend(std::coroutine_handle<T> h) {
void await_suspend(std::coroutine_handle<T> h, std::source_location sl = std::source_location::current()) noexcept {
h.promise().update_resume_point(sl);
_when_ready = h;
_waiting_task = &h.promise();
resume_or_set_callback();
Expand Down
Loading