Skip to content

Commit 14f959e

Browse files
committed
Fix thread local realm client allocator usage
This was actually sharing one allocator for all clients because the cached TLS handle is stored in a single instance of the allocator, rather than the intended usage where each client has a handle via its dynamic buffer. Oops! Making the allocator itself thread local fixes it but leads to every created thread allocating, even if they aren't going to serve clients. The easy solution is to add a mode to require an explicit thread_enter() call to tell the allocator that you intend to use it on the current thread. Solved!
1 parent 969e690 commit 14f959e

3 files changed

Lines changed: 14 additions & 7 deletions

File tree

src/libs/core/allocators/include/allocators/TLSBlockAllocator.h

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,18 @@ namespace ember::allocators {
2323

2424
struct SafeEntrant {};
2525
struct NoRefCounting {};
26+
struct ImplicitInit {};
2627

2728
struct UnsafeEntrant : SafeEntrant {};
28-
struct RefCounting : NoRefCounting {};
29+
struct RefCounting : NoRefCounting {};
30+
struct ExplicitInit : ImplicitInit {};
2931

3032
template<typename _ty,
3133
std::size_t _elements,
3234
std::derived_from<NoRefCounting> RefCountPolicy = NoRefCounting,
3335
std::derived_from<SafeEntrant> EntrantPolicy = SafeEntrant,
34-
std::derived_from<NoPageLock> PageLockPolicy = NoPageLock
36+
std::derived_from<NoPageLock> PageLockPolicy = NoPageLock,
37+
std::derived_from<ImplicitInit> InitPolicy = ImplicitInit
3538
>
3639
class TLSBlockAllocator final {
3740
using AllocatorType = BlockAllocator<_ty, PageLockPolicy>;
@@ -75,7 +78,9 @@ class TLSBlockAllocator final {
7578
#endif
7679

7780
TLSBlockAllocator(std::string_view tag = {}) : tag_(tag) {
78-
thread_enter();
81+
if constexpr(std::is_same_v<InitPolicy, ImplicitInit>) {
82+
thread_enter();
83+
}
7984
}
8085

8186
/*

src/realm/ClientAllocator.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@ using ClientAllocator = allocators::TLSBlockAllocator<
2525
PREALLOCATED_CLIENTS_PER_THREAD,
2626
allocators::NoRefCounting,
2727
allocators::UnsafeEntrant,
28-
PageLockPolicy
28+
PageLockPolicy,
29+
allocators::ExplicitInit
2930
>;
3031

3132
} // realm, ember

src/realm/ClientBuilder.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,17 @@ namespace ember::realm {
2020

2121
class ClientBuilder {
2222
constexpr static std::string_view allocator_tag { "realm_client_create" };
23+
static inline thread_local ClientAllocator allocator_ { allocator_tag };
2324

24-
mutable ClientAllocator allocator_;
2525
ClientHandlerBuilder ch_builder_;
2626
ClientConnectionBuilder cc_builder_;
2727
EventDispatcher& dispatcher_;
2828
thread::ServicePool& pool_;
2929
log::Logger& logger_;
3030

3131
unique_client_ptr make_unique_client(tcp_socket socket, std::size_t index) const {
32+
allocator_.thread_enter();
33+
3234
return unique_client_ptr(allocator_.allocate(
3335
std::move(socket), index, dispatcher_, logger_, ch_builder_, cc_builder_
3436
), ClientDeleter(allocator_, pool_.get(index)));
@@ -37,8 +39,7 @@ class ClientBuilder {
3739
public:
3840
ClientBuilder(ClientHandlerBuilder ch_builder, ClientConnectionBuilder cc_builder,
3941
EventDispatcher& dispatcher, thread::ServicePool& pool, log::Logger& logger)
40-
: allocator_(allocator_tag)
41-
, ch_builder_(ch_builder)
42+
: ch_builder_(ch_builder)
4243
, cc_builder_(cc_builder)
4344
, dispatcher_(dispatcher)
4445
, pool_(pool)

0 commit comments

Comments
 (0)