Skip to content

Commit 74a037d

Browse files
committed
feat engine: move local variables inheritance to TaskContext
Changelog entry: * Allowed to mark any @ref engine::TaskInheritedVariable as @ref engine::TaskInheritedVariablePriority::kBackground, which means that it spills even into @ref utils::AsyncBackground tasks. Previously, only baggage inherited variable had this quality. * Added @ref engine::TaskLocalVariable::GetOrEmplace, which allows to perform essentially non-default initialization of variables, and more. Internal engine refactorings (probably not suitable for changelog): * Moved handling the inheritance of `engine::thread_local::Storage` from `SpanWrapCall` to `TaskContext` constructor. * It allowed to properly order variable inheritance w.r.t. plugin hooks. * It opens the door to add optional support for inherited variables for `utils::TaskBuilder::NoTracing`. commit_hash:dbba4ee958e1d24b261025caa3b815fd38889927
1 parent c371810 commit 74a037d

19 files changed

Lines changed: 344 additions & 219 deletions

.mapping.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1192,6 +1192,7 @@
11921192
"core/include/userver/engine/task/cancel.hpp":"taxi/uservices/userver/core/include/userver/engine/task/cancel.hpp",
11931193
"core/include/userver/engine/task/current_task.hpp":"taxi/uservices/userver/core/include/userver/engine/task/current_task.hpp",
11941194
"core/include/userver/engine/task/inherited_variable.hpp":"taxi/uservices/userver/core/include/userver/engine/task/inherited_variable.hpp",
1195+
"core/include/userver/engine/task/inherited_variable_options.hpp":"taxi/uservices/userver/core/include/userver/engine/task/inherited_variable_options.hpp",
11951196
"core/include/userver/engine/task/local_variable.hpp":"taxi/uservices/userver/core/include/userver/engine/task/local_variable.hpp",
11961197
"core/include/userver/engine/task/shared_task.hpp":"taxi/uservices/userver/core/include/userver/engine/task/shared_task.hpp",
11971198
"core/include/userver/engine/task/shared_task_with_result.hpp":"taxi/uservices/userver/core/include/userver/engine/task/shared_task_with_result.hpp",

core/include/userver/baggage/baggage.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ bool HasInvalidSymbols(const T& obj) {
149149
}) != obj.end();
150150
}
151151

152-
inline engine::TaskInheritedVariable<Baggage> kInheritedBaggage;
152+
inline engine::TaskInheritedVariable<Baggage> kInheritedBaggage{engine::TaskInheritedVariablePriority::kBackground};
153153

154154
} // namespace baggage
155155

core/include/userver/engine/async.hpp

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,10 @@ namespace engine {
3838
template <typename Function, typename... Args>
3939
[[nodiscard]] auto AsyncNoTracing(TaskProcessor& task_processor, Function&& f, Args&&... args) {
4040
return impl::MakeTaskWithResult<TaskWithResult>(
41-
impl::TaskConfig{.task_processor = &task_processor},
41+
impl::TaskConfig{
42+
.task_processor = &task_processor,
43+
.inherited_variables_priority = TaskInheritedVariablePriority::kNone,
44+
},
4245
std::forward<Function>(f),
4346
std::forward<Args>(args)...
4447
);
@@ -47,8 +50,11 @@ template <typename Function, typename... Args>
4750
/// @overload
4851
template <typename Function, typename... Args>
4952
[[nodiscard]] auto AsyncNoTracing(Function&& f, Args&&... args) {
50-
return impl::MakeTaskWithResult<
51-
TaskWithResult>(impl::TaskConfig{}, std::forward<Function>(f), std::forward<Args>(args)...);
53+
return impl::MakeTaskWithResult<TaskWithResult>(
54+
impl::TaskConfig{.inherited_variables_priority = TaskInheritedVariablePriority::kNone},
55+
std::forward<Function>(f),
56+
std::forward<Args>(args)...
57+
);
5258
}
5359

5460
/// @overload
@@ -59,6 +65,7 @@ template <typename Function, typename... Args>
5965
impl::TaskConfig{
6066
.task_processor = &task_processor,
6167
.importance = Task::Importance::kCritical,
68+
.inherited_variables_priority = TaskInheritedVariablePriority::kNone,
6269
},
6370
std::forward<Function>(f),
6471
std::forward<Args>(args)...
@@ -70,7 +77,10 @@ template <typename Function, typename... Args>
7077
template <typename Function, typename... Args>
7178
[[nodiscard]] auto CriticalAsyncNoTracing(Function&& f, Args&&... args) {
7279
return impl::MakeTaskWithResult<TaskWithResult>(
73-
impl::TaskConfig{.importance = Task::Importance::kCritical},
80+
impl::TaskConfig{
81+
.importance = Task::Importance::kCritical,
82+
.inherited_variables_priority = TaskInheritedVariablePriority::kNone,
83+
},
7484
std::forward<Function>(f),
7585
std::forward<Args>(args)...
7686
);

core/include/userver/engine/impl/task_context_factory.hpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@
66
#include <utility>
77

88
#include <userver/engine/impl/task_context_holder.hpp>
9+
#include <userver/engine/impl/task_local_storage.hpp>
10+
#include <userver/engine/task/current_task.hpp>
11+
#include <userver/engine/task/inherited_variable_options.hpp>
912
#include <userver/engine/task/task.hpp>
1013
#include <userver/utils/fast_scope_guard.hpp>
1114
#include <userver/utils/impl/wrapped_call.hpp>
@@ -25,6 +28,9 @@ struct TaskConfig final {
2528
Task::Importance importance{Task::Importance::kNormal};
2629
Task::WaitMode wait_mode{Task::WaitMode::kSingleAwaiter};
2730
engine::Deadline deadline{};
31+
// The lower bound of priority of variables that the new task inherits from the creating task.
32+
// The parent storage itself is captured synchronously on the creating thread.
33+
TaskInheritedVariablePriority inherited_variables_priority{TaskInheritedVariablePriority::kNormal};
2834
};
2935

3036
[[nodiscard]] TaskContext& PlacementNewTaskContext(

core/include/userver/engine/impl/task_local_storage.hpp

Lines changed: 17 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include <type_traits>
77
#include <typeinfo>
88

9+
#include <userver/engine/task/inherited_variable_options.hpp>
910
#include <userver/utils/fast_pimpl.hpp>
1011

1112
USERVER_NAMESPACE_BEGIN
@@ -14,7 +15,7 @@ namespace engine::impl::task_local {
1415

1516
using Key = std::size_t;
1617

17-
enum class VariableKind { kNormal, kInherited };
18+
enum class VariableKind : std::uint8_t { kNormal, kInherited };
1819

1920
class DataBase {
2021
public:
@@ -89,40 +90,29 @@ class Storage final {
8990
Storage(Storage&&) = delete;
9091
Storage& operator=(Storage&&) = delete;
9192

92-
// Requires that the variables have already been destroyed using
93-
// DestroyVariables, or that there are none.
93+
// Requires that the normal (local) variables have already been destroyed
94+
// using DestroyVariables. Any remaining inherited variables (which can be
95+
// left over if the task never finished normally) are destroyed here.
9496
~Storage();
9597

9698
// Destroys the variables in reverse-initialization order.
97-
// Must be called before the destructor. Must be called in the coroutine
98-
// owning `this`: the destructors may want to sleep, and the storage must
99-
// still be usable (e.g. via GetCurrentStorage) while they run.
99+
// Must be called for normal (local) variables before the destructor, in the
100+
// coroutine owning `this`: their destructors may want to sleep, and the
101+
// storage must still be usable (e.g. via GetCurrentStorage) while they run.
100102
void DestroyVariables() noexcept;
101103

102-
// Copies pointers to inherited variables from 'other'
103-
// 'this' must not contain any variables
104-
void InheritFrom(Storage& other);
104+
// Copies pointers to inherited variables from 'other'.
105+
// 'this' must not contain any variables.
106+
// A variable is copied when its priority is not less than 'priority'.
107+
void InheritFrom(Storage& other, TaskInheritedVariablePriority priority);
105108

106-
// Copies pointers to specific inherited variables from 'other'
107-
// 'this' must not contain the variable being copied
108-
// does nothing if there is nothing to copy
109-
void InheritNodeIfExists(Storage& other, Key key);
110-
111-
// Moves other's variables into 'this', leaving 'other' with no variables.
112-
// 'this' must not contain inherited variables. It may contain normal
113-
// variables (e.g. task-locals initialized by engine plugin hooks before
114-
// the task payload starts); those are kept. In that case 'other' must not
115-
// contain normal variables.
116-
void InitializeFrom(Storage&& other) noexcept;
117-
118-
// Variable accessors must be called with the same T, Kind, key.
119-
// Otherwise it is UB.
120-
template <typename T, VariableKind Kind>
121-
T& GetOrEmplace(Key key) {
109+
// Multiple calls with the same key should have the same T and Kind, otherwise it is UB.
110+
template <typename T, VariableKind Kind, typename... Args>
111+
T& GetOrEmplace(Key key, Args&&... args) {
122112
DataBase* const old_data = GetGeneric(key);
123113
if (!old_data) {
124114
const bool has_existing_variable = false;
125-
return DoEmplace<T, Kind>(key, has_existing_variable);
115+
return DoEmplace<T, Kind>(key, has_existing_variable, std::forward<Args>(args)...);
126116
}
127117
return static_cast<DataImpl<T, Kind>&>(*old_data).Get();
128118
}
@@ -188,7 +178,7 @@ class Storage final {
188178

189179
class Variable final {
190180
public:
191-
Variable();
181+
explicit Variable(TaskInheritedVariablePriority priority = TaskInheritedVariablePriority::kNormal);
192182

193183
Variable(const Variable&) = delete;
194184
Variable(Variable&&) = delete;
@@ -203,10 +193,6 @@ class Variable final {
203193

204194
Storage& GetCurrentStorage() noexcept;
205195

206-
struct InternalTag final {
207-
explicit InternalTag() = default;
208-
};
209-
210196
} // namespace engine::impl::task_local
211197

212198
USERVER_NAMESPACE_END

core/include/userver/engine/task/inherited_variable.hpp

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#include <utility>
88

99
#include <userver/engine/impl/task_local_storage.hpp>
10+
#include <userver/engine/task/inherited_variable_options.hpp>
1011

1112
USERVER_NAMESPACE_BEGIN
1213

@@ -19,13 +20,26 @@ namespace engine {
1920
/// These are like engine::TaskLocalVariable, but the variable instances are
2021
/// inherited by child tasks created via utils::Async.
2122
///
23+
/// By default a variable is inherited only by regular child tasks, not @ref utils::AsyncBackground.
24+
/// This behavior can be changed by passing a different priority to the constructor.
25+
///
2226
/// The order of destruction of task-inherited variables is unspecified.
2327
template <typename T>
2428
class TaskInheritedVariable final {
2529
static_assert(!std::is_reference_v<T>);
2630
static_assert(!std::is_const_v<T>);
2731

2832
public:
33+
/// @brief Create a task-inherited variable with @ref TaskInheritedVariablePriority::kNormal.
34+
TaskInheritedVariable()
35+
: TaskInheritedVariable(TaskInheritedVariablePriority::kNormal)
36+
{}
37+
38+
/// @brief Create a task-inherited variable with the specified inheritance priority.
39+
explicit TaskInheritedVariable(TaskInheritedVariablePriority priority)
40+
: impl_(priority)
41+
{}
42+
2943
/// @brief Get the variable instance for the current task.
3044
/// @returns the variable or `nullptr` if variable was not set.
3145
const T* GetOptional() const noexcept { return Storage().GetOptional<T, kVariableKind>(impl_.GetKey()); }
@@ -51,14 +65,6 @@ class TaskInheritedVariable final {
5165
/// @note The variable might not actually be destroyed immediately.
5266
void Erase() { Storage().Erase<T, kVariableKind>(impl_.GetKey()); }
5367

54-
/// @cond
55-
// For internal use only
56-
// inherits data to another storage
57-
void InheritTo(impl::task_local::Storage& other, impl::task_local::InternalTag) {
58-
other.InheritNodeIfExists(Storage(), impl_.GetKey());
59-
}
60-
/// @endcond
61-
6268
private:
6369
static constexpr auto kVariableKind = impl::task_local::VariableKind::kInherited;
6470

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
#pragma once
2+
3+
/// @file userver/engine/task/inherited_variable_options.hpp
4+
/// @brief @copybrief engine::TaskInheritedVariablePriority
5+
6+
#include <cstdint>
7+
8+
USERVER_NAMESPACE_BEGIN
9+
10+
namespace engine {
11+
12+
/// @brief Controls whether a @ref engine::TaskInheritedVariable instance is inherited from the creating task.
13+
enum class TaskInheritedVariablePriority : std::uint8_t {
14+
/// Default priority for @ref engine::TaskInheritedVariable. Propagates to normal tasks.
15+
kNormal = 0,
16+
/// The minimum priority for @ref engine::TaskInheritedVariable instances to propagate to background tasks
17+
/// (e.g. @ref utils::AsyncBackground).
18+
kBackground = 1,
19+
/// The minimum priority for @ref engine::TaskInheritedVariable instances to propagate to no-tracing tasks
20+
/// (e.g. @ref engine::AsyncNoTracing).
21+
/// As of now, this priority cannot be assigned to a variable.
22+
kNoTracing = 2,
23+
/// Do not inherit any variables.
24+
kNone = 2,
25+
};
26+
27+
} // namespace engine
28+
29+
USERVER_NAMESPACE_END

core/include/userver/engine/task/local_variable.hpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,15 @@ class TaskLocalVariable final {
3434
/// @overload
3535
T* operator->();
3636

37+
/// @brief Get the variable instance for the current task, or emplace a new
38+
/// one with the given arguments.
39+
/// @note Must be called from a coroutine, otherwise it is UB.
40+
template <typename... Args>
41+
T& GetOrEmplace(Args&&... args) {
42+
return impl::task_local::GetCurrentStorage()
43+
.GetOrEmplace<T, kVariableKind>(impl_.GetKey(), std::forward<Args>(args)...);
44+
}
45+
3746
/// @brief Get the variable instance for the current task.
3847
/// @returns the variable or `nullptr` if the variable was not initialized,
3948
/// was already destroyed, or is being destroyed right now. That is,

core/include/userver/utils/async.hpp

Lines changed: 27 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,9 @@ template <typename Function, typename... Args>
7474
template <typename Function, typename... Args>
7575
[[nodiscard]] auto Async(engine::TaskProcessor& task_processor, std::string name, Function&& f, Args&&... args) {
7676
return engine::impl::MakeTaskWithResult<engine::TaskWithResult>(
77-
engine::impl::TaskConfig{.task_processor = &task_processor},
77+
engine::impl::TaskConfig{
78+
.task_processor = &task_processor,
79+
},
7880
utils::impl::SpanLazyPrvalue(std::move(name)),
7981
std::forward<Function>(f),
8082
std::forward<Args>(args)...
@@ -98,11 +100,7 @@ template <typename Function, typename... Args>
98100
[[nodiscard]] auto AsyncHideSpan(Function&& f, Args&&... args) {
99101
return engine::impl::MakeTaskWithResult<engine::TaskWithResult>(
100102
engine::impl::TaskConfig{},
101-
utils::impl::SpanLazyPrvalue(
102-
std::string{},
103-
utils::impl::SpanWrapCall::InheritVariables::kYes,
104-
utils::impl::SpanWrapCall::HideSpan::kYes
105-
),
103+
utils::impl::SpanLazyPrvalue(std::string{}, utils::impl::SpanWrapCall::HideSpan::kYes),
106104
std::forward<Function>(f),
107105
std::forward<Args>(args)...
108106
);
@@ -125,12 +123,10 @@ template <typename Function, typename... Args>
125123
template <typename Function, typename... Args>
126124
[[nodiscard]] auto AsyncHideSpan(engine::TaskProcessor& task_processor, Function&& f, Args&&... args) {
127125
return engine::impl::MakeTaskWithResult<engine::TaskWithResult>(
128-
engine::impl::TaskConfig{.task_processor = &task_processor},
129-
utils::impl::SpanLazyPrvalue(
130-
std::string{},
131-
utils::impl::SpanWrapCall::InheritVariables::kYes,
132-
utils::impl::SpanWrapCall::HideSpan::kYes
133-
),
126+
engine::impl::TaskConfig{
127+
.task_processor = &task_processor,
128+
},
129+
utils::impl::SpanLazyPrvalue(std::string{}, utils::impl::SpanWrapCall::HideSpan::kYes),
134130
std::forward<Function>(f),
135131
std::forward<Args>(args)...
136132
);
@@ -179,7 +175,9 @@ template <typename Function, typename... Args>
179175
template <typename Function, typename... Args>
180176
[[nodiscard]] auto SharedAsync(engine::TaskProcessor& task_processor, std::string name, Function&& f, Args&&... args) {
181177
return engine::impl::MakeTaskWithResult<engine::SharedTaskWithResult>(
182-
engine::impl::TaskConfig{.task_processor = &task_processor},
178+
engine::impl::TaskConfig{
179+
.task_processor = &task_processor,
180+
},
183181
utils::impl::SpanLazyPrvalue(std::move(name)),
184182
std::forward<Function>(f),
185183
std::forward<Args>(args)...
@@ -199,7 +197,9 @@ template <typename Function, typename... Args>
199197
template <typename Function, typename... Args>
200198
[[nodiscard]] auto CriticalAsync(std::string name, Function&& f, Args&&... args) {
201199
return engine::impl::MakeTaskWithResult<engine::TaskWithResult>(
202-
engine::impl::TaskConfig{.importance = engine::Task::Importance::kCritical},
200+
engine::impl::TaskConfig{
201+
.importance = engine::Task::Importance::kCritical,
202+
},
203203
utils::impl::SpanLazyPrvalue(std::move(name)),
204204
std::forward<Function>(f),
205205
std::forward<Args>(args)...
@@ -219,7 +219,9 @@ template <typename Function, typename... Args>
219219
template <typename Function, typename... Args>
220220
[[nodiscard]] auto SharedCriticalAsync(std::string name, Function&& f, Args&&... args) {
221221
return engine::impl::MakeTaskWithResult<engine::SharedTaskWithResult>(
222-
engine::impl::TaskConfig{.importance = engine::Task::Importance::kCritical},
222+
engine::impl::TaskConfig{
223+
.importance = engine::Task::Importance::kCritical,
224+
},
223225
utils::impl::SpanLazyPrvalue(std::move(name)),
224226
std::forward<Function>(f),
225227
std::forward<Args>(args)...
@@ -260,7 +262,9 @@ template <typename Function, typename... Args>
260262
template <typename Function, typename... Args>
261263
[[nodiscard]] auto Async(std::string name, engine::Deadline deadline, Function&& f, Args&&... args) {
262264
return engine::impl::MakeTaskWithResult<engine::TaskWithResult>(
263-
engine::impl::TaskConfig{.deadline = deadline},
265+
engine::impl::TaskConfig{
266+
.deadline = deadline,
267+
},
264268
utils::impl::SpanLazyPrvalue(std::move(name)),
265269
std::forward<Function>(f),
266270
std::forward<Args>(args)...
@@ -305,12 +309,11 @@ template <typename Function, typename... Args>
305309
Args&&... args
306310
) {
307311
return engine::impl::MakeTaskWithResult<engine::TaskWithResult>(
308-
engine::impl::TaskConfig{.task_processor = &task_processor},
309-
utils::impl::SpanLazyPrvalue(
310-
std::move(name),
311-
utils::impl::SpanWrapCall::InheritVariables::kNo,
312-
utils::impl::SpanWrapCall::HideSpan::kNo
313-
),
312+
engine::impl::TaskConfig{
313+
.task_processor = &task_processor,
314+
.inherited_variables_priority = engine::TaskInheritedVariablePriority::kBackground,
315+
},
316+
utils::impl::SpanLazyPrvalue(std::move(name)),
314317
std::forward<Function>(f),
315318
std::forward<Args>(args)...
316319
);
@@ -340,12 +343,9 @@ template <typename Function, typename... Args>
340343
engine::impl::TaskConfig{
341344
.task_processor = &task_processor,
342345
.importance = engine::Task::Importance::kCritical,
346+
.inherited_variables_priority = engine::TaskInheritedVariablePriority::kBackground,
343347
},
344-
utils::impl::SpanLazyPrvalue(
345-
std::move(name),
346-
utils::impl::SpanWrapCall::InheritVariables::kNo,
347-
utils::impl::SpanWrapCall::HideSpan::kNo
348-
),
348+
utils::impl::SpanLazyPrvalue(std::move(name)),
349349
std::forward<Function>(f),
350350
std::forward<Args>(args)...
351351
);

0 commit comments

Comments
 (0)