Skip to content

Commit b7725a2

Browse files
committed
refactor engine: model Awaiter with unique ownership
No changelog entry intended, no effects on API or measurable effects on performance. * `engine::impl::Awaiter` is now passed around in `AwaiterPtr`, which is a `unique_ptr` with a custom deleter that decrements the intrusive refcounter of `TaskContext` calls virtual `PolymorphicAwaiter::DisposeWithoutNotification` on destruction. * Rationale: * `unique_ptr` better suits `AwaitableBase` API, which are the majority of awaitables. Only legacy `WaitAny` actually copies the `intrusive_ptr`. * `unique_ptr` better suits most awaiters, which don't support being notified concurrently. And those that do can easily add an explicit `AsAwaiterPtr` method, see e.g. `WaitAnyContext::Impl`. * Making an expensive locked increment an explicit operation can be seen as an additional benefit. * The existing performance of `TaskContext` awaiter is maintained, there are still no virtual calls for this kind of `Awaiter`. * Moved the atomic refcount from `Awaiter` to `TaskContext`. * Added `IntrusiveRefCounterOne` as a common place for intrusive refcounter that initializes to 1 instead of 0 (that place was `Awaiter` previously). commit_hash:3170fb506a660a90af55e129ff22f8c9a9b677bb
1 parent 215794a commit b7725a2

39 files changed

Lines changed: 314 additions & 282 deletions

.mapping.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1147,6 +1147,7 @@
11471147
"core/include/userver/engine/get_all.hpp":"taxi/uservices/userver/core/include/userver/engine/get_all.hpp",
11481148
"core/include/userver/engine/impl/actor.hpp":"taxi/uservices/userver/core/include/userver/engine/impl/actor.hpp",
11491149
"core/include/userver/engine/impl/awaiter.hpp":"taxi/uservices/userver/core/include/userver/engine/impl/awaiter.hpp",
1150+
"core/include/userver/engine/impl/awaiter_fwd.hpp":"taxi/uservices/userver/core/include/userver/engine/impl/awaiter_fwd.hpp",
11501151
"core/include/userver/engine/impl/condition_variable_any.hpp":"taxi/uservices/userver/core/include/userver/engine/impl/condition_variable_any.hpp",
11511152
"core/include/userver/engine/impl/context_accessor.hpp":"taxi/uservices/userver/core/include/userver/engine/impl/context_accessor.hpp",
11521153
"core/include/userver/engine/impl/detach.hpp":"taxi/uservices/userver/core/include/userver/engine/impl/detach.hpp",
@@ -6003,6 +6004,7 @@
60036004
"universal/include/userver/utils/impl/internal_tag.hpp":"taxi/uservices/userver/universal/include/userver/utils/impl/internal_tag.hpp",
60046005
"universal/include/userver/utils/impl/internal_tag_fwd.hpp":"taxi/uservices/userver/universal/include/userver/utils/impl/internal_tag_fwd.hpp",
60056006
"universal/include/userver/utils/impl/intrusive_link_mode.hpp":"taxi/uservices/userver/universal/include/userver/utils/impl/intrusive_link_mode.hpp",
6007+
"universal/include/userver/utils/impl/intrusive_ref_counter_one.hpp":"taxi/uservices/userver/universal/include/userver/utils/impl/intrusive_ref_counter_one.hpp",
60066008
"universal/include/userver/utils/impl/projecting_view.hpp":"taxi/uservices/userver/universal/include/userver/utils/impl/projecting_view.hpp",
60076009
"universal/include/userver/utils/impl/source_location.hpp":"taxi/uservices/userver/universal/include/userver/utils/impl/source_location.hpp",
60086010
"universal/include/userver/utils/impl/static_registration.hpp":"taxi/uservices/userver/universal/include/userver/utils/impl/static_registration.hpp",

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

Lines changed: 40 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,40 +1,33 @@
11
#pragma once
22

3-
#include <atomic>
43
#include <cstddef>
54
#include <cstdint>
65

76
#include <boost/intrusive/list_hook.hpp>
8-
#include <boost/smart_ptr/intrusive_ptr.hpp>
9-
#include <boost/smart_ptr/intrusive_ref_counter.hpp>
107

8+
#include <userver/engine/impl/awaiter_fwd.hpp>
119
#include <userver/engine/impl/epoch.hpp>
1210
#include <userver/utils/assert.hpp>
1311

1412
USERVER_NAMESPACE_BEGIN
1513

1614
namespace engine::impl {
1715

18-
class PolymorphicAwaiter;
16+
class TaskContext;
1917

2018
class Awaiter {
2119
public:
22-
enum InitialRefCounter : std::size_t { kZero = 0, kOne = 1 }; // NOLINT(performance-enum-size)
23-
2420
Awaiter(const Awaiter&) = delete;
2521
Awaiter(Awaiter&&) = delete;
2622
Awaiter& operator=(const Awaiter&) = delete;
2723
Awaiter& operator=(Awaiter&&) = delete;
2824

29-
std::size_t UseCount() const noexcept;
30-
31-
friend void intrusive_ptr_add_ref(Awaiter* awaiter) noexcept; // NOLINT(readability-identifier-naming)
32-
friend void intrusive_ptr_release(Awaiter* awaiter) noexcept; // NOLINT(readability-identifier-naming)
25+
TaskContext& CastToTaskContext() noexcept;
3326

3427
protected:
35-
enum StaticType : std::uint8_t { kPolymorphic, kTaskContext };
28+
enum class StaticType : std::uint8_t { kPolymorphic, kTaskContext };
3629

37-
Awaiter(StaticType type, InitialRefCounter initial_ref_counter);
30+
explicit Awaiter(StaticType type) noexcept;
3831
~Awaiter() = default;
3932

4033
private:
@@ -47,46 +40,60 @@ class Awaiter {
4740
std::uintptr_t context{0};
4841
};
4942

43+
friend struct AwaiterDeleter;
5044
friend class WaitList;
51-
friend void Notify(boost::intrusive_ptr<Awaiter>&& awaiter, std::uintptr_t context) noexcept;
5245

53-
static void NotifyTaskContext(boost::intrusive_ptr<Awaiter> self, std::uintptr_t context) noexcept;
46+
friend void NotifyAndDispose(AwaiterPtr&& awaiter, std::uintptr_t context) noexcept;
5447

55-
PolymorphicAwaiter& CastToPolymorphic() noexcept;
56-
57-
// refcounter for resources and memory deallocation
58-
std::atomic<std::size_t> intrusive_refcount_{0};
48+
static void NotifyAndDisposeTaskContext(Awaiter* self, std::uintptr_t context) noexcept;
49+
static void DisposeWithoutNotificationTaskContext(Awaiter* self) noexcept;
5950

6051
StaticType type_;
61-
6252
WaitListData wait_list_data_;
6353
};
6454

55+
struct AwaiterDeleter {
56+
void operator()(Awaiter* awaiter) noexcept;
57+
};
58+
59+
using AwaiterPtr = std::unique_ptr<Awaiter, AwaiterDeleter>;
60+
6561
class PolymorphicAwaiter : public Awaiter {
6662
public:
67-
virtual void DoNotify(boost::intrusive_ptr<PolymorphicAwaiter> self, std::uintptr_t context) noexcept = 0;
63+
virtual void NotifyAndDispose(std::uintptr_t context) noexcept = 0;
64+
65+
virtual void DisposeWithoutNotification() noexcept = 0;
6866

6967
protected:
7068
PolymorphicAwaiter();
71-
explicit PolymorphicAwaiter(InitialRefCounter initial_ref_counter);
72-
7369
~PolymorphicAwaiter() = default;
70+
};
7471

75-
// Called from intrusive_ptr_release. Should delete the instance
76-
virtual void Destroy() noexcept = 0;
72+
inline void NotifyAndDispose(AwaiterPtr&& awaiter, std::uintptr_t context) noexcept {
73+
UASSERT(awaiter);
7774

78-
private:
79-
friend void intrusive_ptr_release(Awaiter* awaiter) noexcept; // NOLINT(readability-identifier-naming)
80-
};
75+
switch (awaiter->type_) {
76+
case Awaiter::StaticType::kTaskContext:
77+
Awaiter::NotifyAndDisposeTaskContext(awaiter.release(), context);
78+
break;
79+
case Awaiter::StaticType::kPolymorphic:
80+
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast)
81+
static_cast<PolymorphicAwaiter&>(*awaiter.release()).NotifyAndDispose(context);
82+
break;
83+
}
84+
}
8185

82-
inline void Notify(boost::intrusive_ptr<Awaiter>&& awaiter, std::uintptr_t context) noexcept {
86+
inline void AwaiterDeleter::operator()(Awaiter* awaiter) noexcept {
8387
UASSERT(awaiter);
8488

85-
if (awaiter->type_ == Awaiter::StaticType::kTaskContext) {
86-
Awaiter::NotifyTaskContext(std::move(awaiter), context);
87-
} else {
88-
auto polymorphic_awaiter = boost::static_pointer_cast<PolymorphicAwaiter>(std::move(awaiter));
89-
polymorphic_awaiter->DoNotify(std::move(polymorphic_awaiter), context);
89+
switch (awaiter->type_) {
90+
case Awaiter::StaticType::kTaskContext:
91+
Awaiter::DisposeWithoutNotificationTaskContext(awaiter);
92+
break;
93+
case Awaiter::StaticType::kPolymorphic:
94+
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast)
95+
static_cast<PolymorphicAwaiter&>(*awaiter).DisposeWithoutNotification();
96+
break;
9097
}
9198
}
9299

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
#pragma once
2+
3+
#include <memory>
4+
5+
USERVER_NAMESPACE_BEGIN
6+
7+
namespace engine::impl {
8+
9+
class Awaiter;
10+
struct AwaiterDeleter;
11+
12+
using AwaiterPtr = std::unique_ptr<Awaiter, AwaiterDeleter>;
13+
14+
} // namespace engine::impl
15+
16+
USERVER_NAMESPACE_END

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

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,14 @@
22

33
#include <cstdint>
44
#include <exception>
5+
#include <memory>
56

6-
#include <boost/smart_ptr/intrusive_ptr.hpp>
7+
#include <userver/engine/impl/awaiter_fwd.hpp>
78

89
USERVER_NAMESPACE_BEGIN
910

1011
namespace engine::impl {
1112

12-
class Awaiter;
13-
class TaskContext;
14-
15-
void intrusive_ptr_add_ref(Awaiter* awaiter) noexcept; // NOLINT(readability-identifier-naming)
16-
void intrusive_ptr_release(Awaiter* awaiter) noexcept; // NOLINT(readability-identifier-naming)
17-
1813
class WeakAwaitable {
1914
public:
2015
// Auxiliary method to check if the awaitable is ready.
@@ -31,7 +26,7 @@ class WeakAwaitable {
3126
// * do not notify `awaiter`.
3227
//
3328
// You may not sleep in `TryAppendAwaiter`.
34-
virtual void TryAppendAwaiter(boost::intrusive_ptr<Awaiter>& awaiter, std::uintptr_t context) = 0;
29+
virtual void TryAppendAwaiter(AwaiterPtr& awaiter, std::uintptr_t context) = 0;
3530

3631
// Atomically:
3732
//
@@ -48,7 +43,7 @@ class WeakAwaitable {
4843
//
4944
// Depending on a wait list implementation `context` match also could be required for the awaiter removal.
5045
// You may not sleep in `RemoveAwaiter`.
51-
virtual boost::intrusive_ptr<Awaiter> RemoveAwaiter(Awaiter& awaiter, std::uintptr_t context) noexcept = 0;
46+
virtual AwaiterPtr RemoveAwaiter(Awaiter& awaiter, std::uintptr_t context) noexcept = 0;
5247

5348
// Returns the stored exception if present.
5449
// Precondition: IsReady.

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,8 @@ class FutureStateBase : private AwaitableBase {
3838
void WaitForResult();
3939

4040
private:
41-
void TryAppendAwaiter(boost::intrusive_ptr<Awaiter>& awaiter, std::uintptr_t context) final;
42-
boost::intrusive_ptr<Awaiter> RemoveAwaiter(Awaiter& awaiter, std::uintptr_t context) noexcept final;
41+
void TryAppendAwaiter(AwaiterPtr& awaiter, std::uintptr_t context) final;
42+
AwaiterPtr RemoveAwaiter(Awaiter& awaiter, std::uintptr_t context) noexcept final;
4343

4444
FastPimplWaitListLight finish_awaiters_;
4545
std::atomic<bool> is_result_store_locked_;

core/include/userver/engine/multi_consumer_event.hpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,8 @@ class MultiConsumerEvent final : private impl::AwaitableBase {
6666
}
6767

6868
private:
69-
void TryAppendAwaiter(boost::intrusive_ptr<impl::Awaiter>& awaiter, std::uintptr_t context) override;
70-
boost::intrusive_ptr<impl::Awaiter> RemoveAwaiter(impl::Awaiter& awaiter, std::uintptr_t context) noexcept override;
69+
void TryAppendAwaiter(impl::AwaiterPtr& awaiter, std::uintptr_t context) override;
70+
impl::AwaiterPtr RemoveAwaiter(impl::Awaiter& awaiter, std::uintptr_t context) noexcept override;
7171

7272
std::atomic<bool> is_ready_{false};
7373
impl::FastPimplWaitList awaiters_;

core/include/userver/engine/single_use_event.hpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,8 @@ class SingleUseEvent final : private impl::AwaitableBase {
8686
}
8787

8888
private:
89-
void TryAppendAwaiter(boost::intrusive_ptr<impl::Awaiter>& awaiter, std::uintptr_t context) override;
90-
boost::intrusive_ptr<impl::Awaiter> RemoveAwaiter(impl::Awaiter& awaiter, std::uintptr_t context) noexcept override;
89+
void TryAppendAwaiter(impl::AwaiterPtr& awaiter, std::uintptr_t context) override;
90+
impl::AwaiterPtr RemoveAwaiter(impl::Awaiter& awaiter, std::uintptr_t context) noexcept override;
9191

9292
impl::FastPimplWaitListLight awaiters_;
9393
};

core/src/engine/awaitable.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,9 @@ class ReadyAwaitable final : private impl::AwaitableBase {
1717
private:
1818
bool IsReady() const noexcept override { return true; }
1919

20-
void TryAppendAwaiter(boost::intrusive_ptr<impl::Awaiter>&, std::uintptr_t) override {}
20+
void TryAppendAwaiter(impl::AwaiterPtr&, std::uintptr_t) override {}
2121

22-
boost::intrusive_ptr<impl::Awaiter> RemoveAwaiter(impl::Awaiter&, std::uintptr_t) noexcept override {
22+
impl::AwaiterPtr RemoveAwaiter(impl::Awaiter&, std::uintptr_t) noexcept override {
2323
// The awaiter is always notified early by TryAppendAwaiter, nothing to remove.
2424
return {};
2525
}

core/src/engine/impl/awaiter.cpp

Lines changed: 14 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -7,64 +7,29 @@ USERVER_NAMESPACE_BEGIN
77

88
namespace engine::impl {
99

10-
Awaiter::Awaiter(StaticType type, InitialRefCounter initial_ref_counter)
11-
: intrusive_refcount_(initial_ref_counter),
12-
type_(type)
13-
{}
14-
15-
std::size_t Awaiter::UseCount() const noexcept {
16-
// memory order could potentially be less restrictive, but it gets very
17-
// complicated to reason about
18-
return intrusive_refcount_.load(std::memory_order_seq_cst);
19-
}
20-
21-
void Awaiter::NotifyTaskContext(boost::intrusive_ptr<Awaiter> self, std::uintptr_t context) noexcept {
22-
UASSERT(self);
23-
TaskContext::Wakeup(
24-
boost::static_pointer_cast<TaskContext>(std::move(self)),
25-
TaskContext::WakeupSource::kNotify,
26-
static_cast<Epoch>(context)
27-
);
28-
}
29-
30-
PolymorphicAwaiter& Awaiter::CastToPolymorphic() noexcept {
31-
UASSERT_MSG(type_ == StaticType::kPolymorphic, "Unexpected awaiter type");
10+
TaskContext& Awaiter::CastToTaskContext() noexcept {
11+
UASSERT(type_ == StaticType::kTaskContext);
3212
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast)
33-
return static_cast<PolymorphicAwaiter&>(*this);
13+
return static_cast<TaskContext&>(*this);
3414
}
3515

36-
void intrusive_ptr_add_ref(Awaiter* awaiter) noexcept {
37-
UASSERT(awaiter);
16+
Awaiter::Awaiter(StaticType type) noexcept
17+
: type_(type)
18+
{}
3819

39-
// memory order could potentially be less restrictive, but it gets very
40-
// complicated to reason about
41-
awaiter->intrusive_refcount_.fetch_add(1, std::memory_order_seq_cst);
20+
void Awaiter::NotifyAndDisposeTaskContext(Awaiter* self, std::uintptr_t context) noexcept {
21+
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast)
22+
boost::intrusive_ptr<TaskContext> task_context(&static_cast<TaskContext&>(*self), /*add_ref=*/false);
23+
TaskContext::Wakeup(std::move(task_context), TaskContext::WakeupSource::kNotify, static_cast<Epoch>(context));
4224
}
4325

44-
void intrusive_ptr_release(Awaiter* awaiter) noexcept {
45-
UASSERT(awaiter);
46-
47-
// memory order could potentially be less restrictive, but it gets very
48-
// complicated to reason about
49-
if (awaiter->intrusive_refcount_.fetch_sub(1, std::memory_order_seq_cst) != 1) {
50-
return;
51-
}
52-
53-
if (awaiter->type_ == Awaiter::StaticType::kTaskContext) {
54-
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast)
55-
static_cast<TaskContext*>(awaiter)->Destroy();
56-
return;
57-
}
58-
59-
awaiter->CastToPolymorphic().Destroy();
26+
void Awaiter::DisposeWithoutNotificationTaskContext(Awaiter* self) noexcept {
27+
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast)
28+
intrusive_ptr_release(&static_cast<TaskContext&>(*self));
6029
}
6130

6231
PolymorphicAwaiter::PolymorphicAwaiter()
63-
: PolymorphicAwaiter(InitialRefCounter::kZero)
64-
{}
65-
66-
PolymorphicAwaiter::PolymorphicAwaiter(InitialRefCounter initial_ref_counter)
67-
: Awaiter(StaticType::kPolymorphic, initial_ref_counter)
32+
: Awaiter(StaticType::kPolymorphic)
6833
{}
6934

7035
} // namespace engine::impl

core/src/engine/impl/condition_variable_any.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ class CvAwaitable final : public WeakAwaitable {
2323

2424
bool IsReady() const noexcept override { return false; }
2525

26-
void TryAppendAwaiter(boost::intrusive_ptr<Awaiter>& awaiter, std::uintptr_t context) override {
26+
void TryAppendAwaiter(engine::impl::AwaiterPtr& awaiter, std::uintptr_t context) override {
2727
UASSERT(mutex_lock_);
2828
{
2929
WaitList::Lock awaiters_lock{awaiters_};
@@ -36,7 +36,7 @@ class CvAwaitable final : public WeakAwaitable {
3636
// by user under mutex_lock_.
3737
}
3838

39-
boost::intrusive_ptr<Awaiter> RemoveAwaiter(Awaiter& awaiter, std::uintptr_t context) noexcept override {
39+
engine::impl::AwaiterPtr RemoveAwaiter(engine::impl::Awaiter& awaiter, std::uintptr_t context) noexcept override {
4040
WaitList::Lock awaiters_lock{awaiters_};
4141
return awaiters_.Remove(awaiters_lock, awaiter, context);
4242
}

0 commit comments

Comments
 (0)