This is an automated email from the ASF dual-hosted git repository. jamesge pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/incubator-brpc.git
The following commit(s) were added to refs/heads/master by this push: new fb93753 SimpleDataPool::Return resets data before returning back to pool fb93753 is described below commit fb93753bc54666fb815ed2fe099b0a98a01ea61e Author: jamesge <jge...@gmail.com> AuthorDate: Mon Dec 7 12:40:16 2020 +0800 SimpleDataPool::Return resets data before returning back to pool --- src/brpc/data_factory.h | 15 ++- .../{simple_data_pool.h => simple_data_pool.cpp} | 56 ++--------- src/brpc/simple_data_pool.h | 111 --------------------- 3 files changed, 21 insertions(+), 161 deletions(-) diff --git a/src/brpc/data_factory.h b/src/brpc/data_factory.h index 4fd8bf5..a487262 100644 --- a/src/brpc/data_factory.h +++ b/src/brpc/data_factory.h @@ -24,19 +24,24 @@ namespace brpc { +// ---- thread safety ---- +// Method implementations of this interface should be thread-safe class DataFactory { public: virtual ~DataFactory() {} - // Implement this method to create a piece of data. - // Notice that this method is const. + // Implement this method to create a piece of data // Returns the data, NULL on error. virtual void* CreateData() const = 0; - // Implement this method to destroy a piece of data that was created - // by Create(). - // Notice that this method is const. + // Implement this method to destroy data created by Create(). virtual void DestroyData(void*) const = 0; + + // Overwrite this method to reset the data before reuse. Nothing done by default. + // Returns + // true: the data can be kept for future reuse + // false: the data was already destroyed and should NOT be reused. + virtual bool ResetData(void*) const { return true; } }; } // namespace brpc diff --git a/src/brpc/simple_data_pool.h b/src/brpc/simple_data_pool.cpp similarity index 69% copy from src/brpc/simple_data_pool.h copy to src/brpc/simple_data_pool.cpp index 4ab3f1f..9374da4 100644 --- a/src/brpc/simple_data_pool.h +++ b/src/brpc/simple_data_pool.cpp @@ -16,46 +16,11 @@ // under the License. -#ifndef BRPC_SIMPLE_DATA_POOL_H -#define BRPC_SIMPLE_DATA_POOL_H - -#include "butil/scoped_lock.h" -#include "brpc/data_factory.h" - +#include "brpc/simple_data_pool.h" namespace brpc { -// As the name says, this is a simple unbounded dynamic-size pool for -// reusing void* data. We're assuming that data consumes considerable -// memory and should be reused as much as possible, thus unlike the -// multi-threaded allocator caching objects thread-locally, we just -// put everything in a global list to maximize sharing. It's currently -// used by Server to reuse session-local data. -class SimpleDataPool { -public: - struct Stat { - unsigned nfree; - unsigned ncreated; - }; - - explicit SimpleDataPool(const DataFactory* factory); - ~SimpleDataPool(); - void Reset(const DataFactory* factory); - void Reserve(unsigned n); - void* Borrow(); - void Return(void*); - Stat stat() const; - -private: - butil::Mutex _mutex; - unsigned _capacity; - unsigned _size; - butil::atomic<unsigned> _ncreated; - void** _pool; - const DataFactory* _factory; -}; - -inline SimpleDataPool::SimpleDataPool(const DataFactory* factory) +SimpleDataPool::SimpleDataPool(const DataFactory* factory) : _capacity(0) , _size(0) , _ncreated(0) @@ -63,11 +28,11 @@ inline SimpleDataPool::SimpleDataPool(const DataFactory* factory) , _factory(factory) { } -inline SimpleDataPool::~SimpleDataPool() { +SimpleDataPool::~SimpleDataPool() { Reset(NULL); } -inline void SimpleDataPool::Reset(const DataFactory* factory) { +void SimpleDataPool::Reset(const DataFactory* factory) { unsigned saved_size = 0; void** saved_pool = NULL; const DataFactory* saved_factory = NULL; @@ -92,7 +57,7 @@ inline void SimpleDataPool::Reset(const DataFactory* factory) { } } -inline void SimpleDataPool::Reserve(unsigned n) { +void SimpleDataPool::Reserve(unsigned n) { if (_capacity >= n) { return; } @@ -124,7 +89,7 @@ inline void SimpleDataPool::Reserve(unsigned n) { } } -inline void* SimpleDataPool::Borrow() { +void* SimpleDataPool::Borrow() { if (_size) { BAIDU_SCOPED_LOCK(_mutex); if (_size) { @@ -138,10 +103,13 @@ inline void* SimpleDataPool::Borrow() { return data; } -inline void SimpleDataPool::Return(void* data) { +void SimpleDataPool::Return(void* data) { if (data == NULL) { return; } + if (!_factory->ResetData(data)) { + return; + } std::unique_lock<butil::Mutex> mu(_mutex); if (_capacity == _size) { const unsigned new_cap = (_capacity <= 1 ? 128 : (_capacity * 3 / 2)); @@ -160,12 +128,10 @@ inline void SimpleDataPool::Return(void* data) { _pool[_size++] = data; } -inline SimpleDataPool::Stat SimpleDataPool::stat() const { +SimpleDataPool::Stat SimpleDataPool::stat() const { Stat s = { _size, _ncreated.load(butil::memory_order_relaxed) }; return s; } } // namespace brpc - -#endif // BRPC_SIMPLE_DATA_POOL_H diff --git a/src/brpc/simple_data_pool.h b/src/brpc/simple_data_pool.h index 4ab3f1f..390f6e5 100644 --- a/src/brpc/simple_data_pool.h +++ b/src/brpc/simple_data_pool.h @@ -55,117 +55,6 @@ private: const DataFactory* _factory; }; -inline SimpleDataPool::SimpleDataPool(const DataFactory* factory) - : _capacity(0) - , _size(0) - , _ncreated(0) - , _pool(NULL) - , _factory(factory) { -} - -inline SimpleDataPool::~SimpleDataPool() { - Reset(NULL); -} - -inline void SimpleDataPool::Reset(const DataFactory* factory) { - unsigned saved_size = 0; - void** saved_pool = NULL; - const DataFactory* saved_factory = NULL; - { - BAIDU_SCOPED_LOCK(_mutex); - saved_size = _size; - saved_pool = _pool; - saved_factory = _factory; - _capacity = 0; - _size = 0; - _ncreated.store(0, butil::memory_order_relaxed); - _pool = NULL; - _factory = factory; - } - if (saved_pool) { - if (saved_factory) { - for (unsigned i = 0; i < saved_size; ++i) { - saved_factory->DestroyData(saved_pool[i]); - } - } - free(saved_pool); - } -} - -inline void SimpleDataPool::Reserve(unsigned n) { - if (_capacity >= n) { - return; - } - BAIDU_SCOPED_LOCK(_mutex); - if (_capacity >= n) { - return; - } - // Resize. - const unsigned new_cap = std::max(_capacity * 3 / 2, n); - void** new_pool = (void**)malloc(new_cap * sizeof(void*)); - if (NULL == new_pool) { - return; - } - if (_pool) { - memcpy(new_pool, _pool, _capacity * sizeof(void*)); - free(_pool); - } - unsigned i = _capacity; - _capacity = new_cap; - _pool = new_pool; - - for (; i < n; ++i) { - void* data = _factory->CreateData(); - if (data == NULL) { - break; - } - _ncreated.fetch_add(1, butil::memory_order_relaxed); - _pool[_size++] = data; - } -} - -inline void* SimpleDataPool::Borrow() { - if (_size) { - BAIDU_SCOPED_LOCK(_mutex); - if (_size) { - return _pool[--_size]; - } - } - void* data = _factory->CreateData(); - if (data) { - _ncreated.fetch_add(1, butil::memory_order_relaxed); - } - return data; -} - -inline void SimpleDataPool::Return(void* data) { - if (data == NULL) { - return; - } - std::unique_lock<butil::Mutex> mu(_mutex); - if (_capacity == _size) { - const unsigned new_cap = (_capacity <= 1 ? 128 : (_capacity * 3 / 2)); - void** new_pool = (void**)malloc(new_cap * sizeof(void*)); - if (NULL == new_pool) { - mu.unlock(); - return _factory->DestroyData(data); - } - if (_pool) { - memcpy(new_pool, _pool, _capacity * sizeof(void*)); - free(_pool); - } - _capacity = new_cap; - _pool = new_pool; - } - _pool[_size++] = data; -} - -inline SimpleDataPool::Stat SimpleDataPool::stat() const { - Stat s = { _size, _ncreated.load(butil::memory_order_relaxed) }; - return s; -} - } // namespace brpc - #endif // BRPC_SIMPLE_DATA_POOL_H --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org For additional commands, e-mail: dev-h...@brpc.apache.org