Skip to content

Commit cd024ee

Browse files
committed
Revert "Make Allocator.h less silly (no creepy "proto" object). (apache#6241)"
This reverts commit b24f62f. Revert "Call constructors and destructors for H1/2 Session/Transaction via ClassAllocator (apache#7584)" This reverts commit a0dd3c2. Revert "Changes HttpSM to be Proxy Allocated (apache#8082)" This reverts commit 6806795.
1 parent cb7eda6 commit cd024ee

19 files changed

+160
-98
lines changed

include/tscore/Allocator.h

Lines changed: 67 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@
4141

4242
#include <new>
4343
#include <cstdlib>
44-
#include <utility>
4544
#include "tscore/ink_queue.h"
4645
#include "tscore/ink_defs.h"
4746
#include "tscore/ink_resource.h"
@@ -109,39 +108,29 @@ class Allocator
109108
ink_freelist_madvise_init(&this->fl, name, element_size, chunk_size, alignment, advice);
110109
}
111110

112-
// Dummies
113-
void
114-
destroy_if_enabled(void *)
115-
{
116-
}
117-
Allocator &
118-
raw()
119-
{
120-
return *this;
121-
}
122-
123111
protected:
124112
InkFreeList *fl;
125113
};
126114

127115
/**
128-
Allocator for Class objects.
116+
Allocator for Class objects. It uses a prototype object to do
117+
fast initialization. Prototype of the template class is created
118+
when the fast allocator is created. This is instantiated with
119+
default (no argument) constructor. Constructor is not called for
120+
the allocated objects. Instead, the prototype is just memory
121+
copied onto the new objects. This is done for performance reasons.
129122
130123
*/
131-
template <class C, bool Destruct_on_free_ = false> class ClassAllocator : private Allocator
124+
template <class C> class ClassAllocator : public Allocator
132125
{
133126
public:
134-
using Value_type = C;
135-
static bool const Destruct_on_free = Destruct_on_free_;
136-
137-
/** Allocates objects of the templated type. Arguments are forwarded to the constructor for the object. */
138-
template <typename... Args>
127+
/** Allocates objects of the templated type. */
139128
C *
140-
alloc(Args &&... args)
129+
alloc()
141130
{
142131
void *ptr = ink_freelist_new(this->fl);
143132

144-
::new (ptr) C(std::forward<Args>(args)...);
133+
memcpy(ptr, (void *)&this->proto.typeObject, sizeof(C));
145134
return (C *)ptr;
146135
}
147136

@@ -153,47 +142,82 @@ template <class C, bool Destruct_on_free_ = false> class ClassAllocator : privat
153142
void
154143
free(C *ptr)
155144
{
156-
destroy_if_enabled(ptr);
157-
158145
ink_freelist_free(this->fl, ptr);
159146
}
160147

161148
/**
162-
Create a new class specific ClassAllocator.
149+
Deallocates objects of the templated type.
163150
164-
@param name some identifying name, used for mem tracking purposes.
165-
@param chunk_size number of units to be allocated if free pool is empty.
166-
@param alignment of objects must be a power of 2.
151+
@param head pointer to be freed.
152+
@param tail pointer to be freed.
153+
@param num_item of blocks to be freed.
154+
*/
155+
void
156+
free_bulk(C *head, C *tail, size_t num_item)
157+
{
158+
ink_freelist_free_bulk(this->fl, head, tail, num_item);
159+
}
160+
161+
/**
162+
Allocate objects of the templated type via the inherited interface
163+
using void pointers.
167164
*/
168-
ClassAllocator(const char *name, unsigned int chunk_size = 128, unsigned int alignment = 16)
165+
void *
166+
alloc_void()
169167
{
170-
ink_freelist_init(&this->fl, name, RND16(sizeof(C)), chunk_size, RND16(alignment));
168+
return (void *)alloc();
171169
}
172170

173-
Allocator &
174-
raw()
171+
/**
172+
Deallocate objects of the templated type via the inherited
173+
interface using void pointers.
174+
175+
@param ptr pointer to be freed.
176+
*/
177+
void
178+
free_void(void *ptr)
175179
{
176-
return *this;
180+
free((C *)ptr);
177181
}
178182

183+
/**
184+
Deallocate objects of the templated type via the inherited
185+
interface using void pointers.
186+
187+
@param head pointer to be freed.
188+
@param tail pointer to be freed.
189+
@param num_item of blocks.
190+
*/
179191
void
180-
destroy_if_enabled(C *ptr)
192+
free_void_bulk(void *head, void *tail, size_t num_item)
181193
{
182-
if (Destruct_on_free) {
183-
ptr->~C();
184-
}
194+
free_bulk((C *)head, (C *)tail, num_item);
195+
}
196+
197+
/**
198+
Create a new class specific ClassAllocator.
199+
200+
@param name some identifying name, used for mem tracking purposes.
201+
@param chunk_size number of units to be allocated if free pool is empty.
202+
@param alignment of objects must be a power of 2.
203+
*/
204+
ClassAllocator(const char *name, unsigned int chunk_size = 128, unsigned int alignment = 16)
205+
{
206+
::new ((void *)&proto.typeObject) C();
207+
ink_freelist_init(&this->fl, name, RND16(sizeof(C)), chunk_size, RND16(alignment));
185208
}
186209

187-
// Ensure that C is big enough to hold a void pointer (when it's stored in the free list as raw memory).
188-
//
189-
static_assert(sizeof(C) >= sizeof(void *), "Can not allocate instances of this class using ClassAllocator");
210+
struct {
211+
uint8_t typeObject[sizeof(C)];
212+
int64_t space_holder = 0;
213+
} proto;
190214
};
191215

192-
template <class C, bool Destruct_on_free = false> class TrackerClassAllocator : public ClassAllocator<C, Destruct_on_free>
216+
template <class C> class TrackerClassAllocator : public ClassAllocator<C>
193217
{
194218
public:
195219
TrackerClassAllocator(const char *name, unsigned int chunk_size = 128, unsigned int alignment = 16)
196-
: ClassAllocator<C, Destruct_on_free>(name, chunk_size, alignment), allocations(0), trackerLock(PTHREAD_MUTEX_INITIALIZER)
220+
: ClassAllocator<C>(name, chunk_size, alignment), allocations(0), trackerLock(PTHREAD_MUTEX_INITIALIZER)
197221
{
198222
}
199223

@@ -202,7 +226,7 @@ template <class C, bool Destruct_on_free = false> class TrackerClassAllocator :
202226
{
203227
void *callstack[3];
204228
int frames = backtrace(callstack, 3);
205-
C *ptr = ClassAllocator<C, Destruct_on_free>::alloc();
229+
C *ptr = ClassAllocator<C>::alloc();
206230

207231
const void *symbol = nullptr;
208232
if (frames == 3 && callstack[2] != nullptr) {
@@ -228,7 +252,7 @@ template <class C, bool Destruct_on_free = false> class TrackerClassAllocator :
228252
reverse_lookup.erase(it);
229253
}
230254
ink_mutex_release(&trackerLock);
231-
ClassAllocator<C, Destruct_on_free>::free(ptr);
255+
ClassAllocator<C>::free(ptr);
232256
}
233257

234258
private:

iocore/cache/P_CacheInternal.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -432,7 +432,7 @@ struct CacheVC : public CacheVConnection {
432432
Ptr<IOBufferBlock> blocks; // data available to write
433433
Ptr<IOBufferBlock> writer_buf;
434434

435-
OpenDirEntry *od = nullptr;
435+
OpenDirEntry *od;
436436
AIOCallbackInternal io;
437437
int alternate_index = CACHE_ALT_INDEX_DEFAULT; // preferred position in vector
438438
LINK(CacheVC, opendir_link);

iocore/eventsystem/I_ProxyAllocator.h

Lines changed: 63 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,6 @@
3030
*****************************************************************************/
3131
#pragma once
3232

33-
#include <new>
34-
#include <utility>
35-
3633
#include "tscore/ink_platform.h"
3734

3835
class EThread;
@@ -48,51 +45,84 @@ struct ProxyAllocator {
4845
ProxyAllocator() {}
4946
};
5047

51-
template <class CAlloc, typename... Args>
52-
typename CAlloc::Value_type *
53-
thread_alloc(CAlloc &a, ProxyAllocator &l, Args &&... args)
48+
template <class C>
49+
inline C *
50+
thread_alloc(ClassAllocator<C> &a, ProxyAllocator &l)
5451
{
5552
if (!cmd_disable_pfreelist && l.freelist) {
56-
void *v = l.freelist;
57-
l.freelist = *reinterpret_cast<void **>(l.freelist);
53+
C *v = (C *)l.freelist;
54+
l.freelist = *(C **)l.freelist;
5855
--(l.allocated);
59-
::new (v) typename CAlloc::Value_type(std::forward<Args>(args)...);
60-
return static_cast<typename CAlloc::Value_type *>(v);
56+
*(void **)v = *(void **)&a.proto.typeObject;
57+
return v;
6158
}
62-
return a.alloc(std::forward<Args>(args)...);
59+
return a.alloc();
6360
}
6461

65-
class Allocator;
62+
template <class C>
63+
inline C *
64+
thread_alloc_init(ClassAllocator<C> &a, ProxyAllocator &l)
65+
{
66+
if (!cmd_disable_pfreelist && l.freelist) {
67+
C *v = (C *)l.freelist;
68+
l.freelist = *(C **)l.freelist;
69+
--(l.allocated);
70+
memcpy((void *)v, (void *)&a.proto.typeObject, sizeof(C));
71+
return v;
72+
}
73+
return a.alloc();
74+
}
6675

67-
void *thread_alloc(Allocator &a, ProxyAllocator &l);
68-
void thread_freeup(Allocator &a, ProxyAllocator &l);
76+
template <class C>
77+
inline void
78+
thread_free(ClassAllocator<C> &a, C *p)
79+
{
80+
a.free(p);
81+
}
6982

70-
#if 1
83+
static inline void
84+
thread_free(Allocator &a, void *p)
85+
{
86+
a.free_void(p);
87+
}
7188

72-
// Potentially empty variable arguments -- non-standard GCC way
73-
//
74-
#define THREAD_ALLOC(_a, _t, ...) thread_alloc(::_a, _t->_a, ##__VA_ARGS__)
75-
#define THREAD_ALLOC_INIT(_a, _t, ...) thread_alloc(::_a, _t->_a, ##__VA_ARGS__)
89+
template <class C>
90+
inline void
91+
thread_freeup(ClassAllocator<C> &a, ProxyAllocator &l)
92+
{
93+
C *head = (C *)l.freelist;
94+
C *tail = (C *)l.freelist;
95+
size_t count = 0;
96+
while (l.freelist && l.allocated > thread_freelist_low_watermark) {
97+
tail = (C *)l.freelist;
98+
l.freelist = *(C **)l.freelist;
99+
--(l.allocated);
100+
++count;
101+
}
76102

77-
#else
103+
if (unlikely(count == 1)) {
104+
a.free(tail);
105+
} else if (count > 0) {
106+
a.free_bulk(head, tail, count);
107+
}
78108

79-
// Potentially empty variable arguments -- Standard C++20 way
80-
//
81-
#define THREAD_ALLOC(_a, _t, ...) thread_alloc(::_a, _t->_a __VA_OPT__(, ) __VA_ARGS__)
82-
#define THREAD_ALLOC_INIT(_a, _t, ...) thread_alloc(::_a, _t->_a __VA_OPT__(, ) __VA_ARGS__)
109+
ink_assert(l.allocated >= thread_freelist_low_watermark);
110+
}
83111

84-
#endif
112+
void *thread_alloc(Allocator &a, ProxyAllocator &l);
113+
void thread_freeup(Allocator &a, ProxyAllocator &l);
85114

115+
#define THREAD_ALLOC(_a, _t) thread_alloc(::_a, _t->_a)
116+
#define THREAD_ALLOC_INIT(_a, _t) thread_alloc_init(::_a, _t->_a)
86117
#define THREAD_FREE(_p, _a, _t) \
87-
do { \
88-
::_a.destroy_if_enabled(_p); \
89-
if (!cmd_disable_pfreelist) { \
118+
if (!cmd_disable_pfreelist) { \
119+
do { \
90120
*(char **)_p = (char *)_t->_a.freelist; \
91121
_t->_a.freelist = _p; \
92122
_t->_a.allocated++; \
93123
if (_t->_a.allocated > thread_freelist_high_watermark) \
94-
thread_freeup(::_a.raw(), _t->_a); \
95-
} else { \
96-
::_a.raw().free_void(_p); \
97-
} \
98-
} while (0)
124+
thread_freeup(::_a, _t->_a); \
125+
} while (0); \
126+
} else { \
127+
thread_free(::_a, _p); \
128+
}

iocore/eventsystem/I_Thread.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,6 @@ class Thread
122122
ProxyAllocator http1ClientSessionAllocator;
123123
ProxyAllocator http2ClientSessionAllocator;
124124
ProxyAllocator http2StreamAllocator;
125-
ProxyAllocator httpSMAllocator;
126125
ProxyAllocator quicClientSessionAllocator;
127126
ProxyAllocator quicBidiStreamAllocator;
128127
ProxyAllocator quicSendStreamAllocator;

proxy/ProxyTransaction.cc

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,6 @@
2626

2727
#define HttpTxnDebug(fmt, ...) SsnDebug(this, "http_txn", fmt, __VA_ARGS__)
2828

29-
extern ClassAllocator<HttpSM> httpSMAllocator;
30-
3129
ProxyTransaction::ProxyTransaction(ProxySession *session) : VConnection(nullptr), _proxy_ssn(session) {}
3230

3331
ProxyTransaction::~ProxyTransaction()
@@ -45,7 +43,7 @@ ProxyTransaction::new_transaction(bool from_early_data)
4543
// connection re-use
4644

4745
ink_release_assert(_proxy_ssn != nullptr);
48-
_sm = THREAD_ALLOC(httpSMAllocator, this_thread());
46+
_sm = HttpSM::allocate();
4947
_sm->init(from_early_data);
5048
HttpTxnDebug("[%" PRId64 "] Starting transaction %d using sm [%" PRId64 "]", _proxy_ssn->connection_id(),
5149
_proxy_ssn->get_transact_count(), _sm->sm_id);

proxy/http/Http1ClientSession.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ ink_mutex debug_cs_list_mutex;
5555

5656
#endif /* USE_HTTP_DEBUG_LISTS */
5757

58-
ClassAllocator<Http1ClientSession, true> http1ClientSessionAllocator("http1ClientSessionAllocator");
58+
ClassAllocator<Http1ClientSession> http1ClientSessionAllocator("http1ClientSessionAllocator");
5959

6060
Http1ClientSession::Http1ClientSession() : super(), trans(this) {}
6161

@@ -118,6 +118,7 @@ Http1ClientSession::free()
118118
_vc = nullptr;
119119
}
120120

121+
this->~Http1ClientSession();
121122
THREAD_FREE(this, http1ClientSessionAllocator, this_thread());
122123
}
123124

proxy/http/Http1ClientSession.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,4 +129,4 @@ class Http1ClientSession : public ProxySession
129129
Http1Transaction trans;
130130
};
131131

132-
extern ClassAllocator<Http1ClientSession, true> http1ClientSessionAllocator;
132+
extern ClassAllocator<Http1ClientSession> http1ClientSessionAllocator;

proxy/http/Http1ServerSession.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636
#include "HttpSessionManager.h"
3737
#include "HttpSM.h"
3838

39-
ClassAllocator<Http1ServerSession, true> httpServerSessionAllocator("httpServerSessionAllocator");
39+
ClassAllocator<Http1ServerSession> httpServerSessionAllocator("httpServerSessionAllocator");
4040

4141
void
4242
Http1ServerSession::destroy()
@@ -50,6 +50,7 @@ Http1ServerSession::destroy()
5050
}
5151

5252
mutex.clear();
53+
this->~Http1ServerSession();
5354
if (httpSessionManager.get_pool_type() == TS_SERVER_SESSION_SHARING_POOL_THREAD) {
5455
THREAD_FREE(this, httpServerSessionAllocator, this_thread());
5556
} else {

proxy/http/Http1ServerSession.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ class Http1ServerSession : public PoolableSession
107107
IOBufferReader *buf_reader = nullptr;
108108
};
109109

110-
extern ClassAllocator<Http1ServerSession, true> httpServerSessionAllocator;
110+
extern ClassAllocator<Http1ServerSession> httpServerSessionAllocator;
111111

112112
////////////////////////////////////////////
113113
// INLINE

0 commit comments

Comments
 (0)