Optimization - Get rid of the vector that held the unique_ptr.

Use the replies list directly. I can't see any significant
performance improvement yet, but there is something else I want to
try starting from this point.
This commit is contained in:
King_DuckZ 2016-12-06 18:39:41 +00:00
parent 093d1dd198
commit a6fab83296
8 changed files with 192 additions and 33 deletions

3
.gitignore vendored
View File

@ -1 +1,4 @@
tags
.ycm_extra_conf.py
__pycache__/
compile_commands.json

View File

@ -22,6 +22,7 @@ add_library(${PROJECT_NAME} SHARED
src/async_connection.cpp
src/incredis.cpp
src/incredis_batch.cpp
src/reply_list.cpp
)
target_include_directories(${PROJECT_NAME} SYSTEM

View File

@ -20,8 +20,9 @@
#include "reply.hpp"
#include "arg_to_bin_safe.hpp"
#include <vector>
#include "sized_range.hpp"
#include <memory>
#include <forward_list>
namespace redis {
class Command;
@ -31,12 +32,15 @@ namespace redis {
class Batch {
friend class Command;
public:
using ConstReplies = SizedRange<std::forward_list<Reply>::const_iterator>;
using Replies = SizedRange<std::forward_list<Reply>::iterator>;
Batch ( Batch&& parOther );
Batch ( const Batch& ) = delete;
~Batch ( void ) noexcept;
const std::vector<Reply>& replies ( void );
std::vector<Reply>& replies_nonconst ( void );
ConstReplies replies ( void ) const;
Replies replies_nonconst ( void );
bool replies_requested ( void ) const;
void throw_if_failed ( void );
@ -54,8 +58,6 @@ namespace redis {
explicit Batch ( AsyncConnection* parConn, ThreadContext& parThreadContext );
void run_pvt ( int parArgc, const char** parArgv, std::size_t* parLengths );
std::vector<std::unique_ptr<Reply>> m_futures;
std::vector<Reply> m_replies;
std::unique_ptr<LocalData> m_local_data;
AsyncConnection* m_async_conn;
};

View File

@ -27,6 +27,8 @@
namespace redis {
class IncRedisBatch {
public:
using ConstReplies = Batch::ConstReplies;
enum ZADD_Mode {
ZADD_XX_UpdateOnly,
ZADD_NX_AlwaysAdd,
@ -46,7 +48,7 @@ namespace redis {
void reset ( void );
void throw_if_failed ( void );
const std::vector<Reply>& replies ( void ) { return m_batch.replies(); }
ConstReplies replies ( void ) { return m_batch.replies(); }
Batch& batch ( void ) { return m_batch; }
const Batch& batch ( void ) const { return m_batch; }

View File

@ -0,0 +1,56 @@
/* Copyright 2016, Michele Santullo
* This file is part of "incredis".
*
* "incredis" is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* "incredis" is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with "incredis". If not, see <http://www.gnu.org/licenses/>.
*/
#ifndef idC3909E193A3C4DBEB9B646A4F5ED3518
#define idC3909E193A3C4DBEB9B646A4F5ED3518
#include <boost/range/iterator_range_core.hpp>
#include <cstddef>
#include <cassert>
#include <ciso646>
namespace redis {
template <typename I>
class SizedRange : public boost::iterator_range<I> {
public:
SizedRange ( I parBegin, I parEnd, std::size_t parSize ) :
boost::iterator_range<I>(parBegin, parEnd),
m_size(parSize)
{
}
template <typename R>
SizedRange ( R& parRange, std::size_t parSize ) :
boost::iterator_range<I>(parRange),
m_size(parSize)
{
}
~SizedRange ( void ) noexcept = default;
std::size_t size ( void ) const { return m_size; }
bool empty ( void ) const { return static_cast<bool>(0 == m_size); }
typename I::reference front ( void ) { assert(not empty()); return *this->begin(); }
const typename I::reference front ( void ) const { assert(not empty()); return *this->begin(); }
private:
std::size_t m_size;
};
} //namespace redis
#endif

View File

@ -18,6 +18,7 @@
#include "batch.hpp"
#include "async_connection.hpp"
#include "thread_context.hpp"
#include "reply_list.hpp"
#include <hiredis/hiredis.h>
#include <hiredis/async.h>
#include <cassert>
@ -41,13 +42,13 @@ namespace redis {
HiredisCallbackData ( std::atomic_size_t& parPendingFutures, std::atomic_size_t& parLocalPendingFutures, std::condition_variable& parSendCmdCond, std::condition_variable& parLocalCmdsCond ) :
pending_futures(parPendingFutures),
local_pending_futures(parLocalPendingFutures),
reply_ptr(nullptr),
reply_ptr(),
send_command_condition(parSendCmdCond),
local_commands_condition(parLocalCmdsCond)
{
}
Reply* reply_ptr;
ReplyList::ReplyPtr reply_ptr;
std::atomic_size_t& pending_futures;
std::atomic_size_t& local_pending_futures;
std::condition_variable& send_command_condition;
@ -92,7 +93,6 @@ namespace redis {
if (parReply) {
auto reply = make_redis_reply_type(static_cast<redisReply*>(parReply));
assert(data->reply_ptr);
*data->reply_ptr = std::move(reply);
}
else {
@ -108,7 +108,8 @@ namespace redis {
delete data;
}
int array_throw_if_failed (int parErrCount, int parMaxReportedErrors, const std::vector<Reply>& parReplies, std::ostream& parStream) {
template <typename R>
int array_throw_if_failed (int parErrCount, int parMaxReportedErrors, const R& parReplies, std::ostream& parStream) {
int err_count = 0;
for (const auto& rep : parReplies) {
if (rep.which() == RedisVariantType_Error) {
@ -135,6 +136,7 @@ namespace redis {
{
}
ReplyList replies;
std::condition_variable free_cmd_slot;
std::condition_variable no_more_pending_futures;
std::mutex futures_mutex;
@ -146,8 +148,6 @@ namespace redis {
Batch::Batch (Batch&&) = default;
Batch::Batch (AsyncConnection* parConn, ThreadContext& parThreadContext) :
m_futures(),
m_replies(),
m_local_data(new LocalData(parThreadContext)),
m_async_conn(parConn)
{
@ -161,7 +161,6 @@ namespace redis {
}
void Batch::run_pvt (int parArgc, const char** parArgv, std::size_t* parLengths) {
assert(not replies_requested());
assert(parArgc >= 1);
assert(parArgv);
assert(parLengths); //This /could/ be null, but I don't see why it should
@ -185,9 +184,7 @@ namespace redis {
std::cout << " emplace_back(future)... ";
#endif
std::unique_ptr<Reply> new_reply(new Reply);
data->reply_ptr = new_reply.get();
m_futures.emplace_back(std::move(new_reply));
data->reply_ptr = m_local_data->replies.add();
{
std::lock_guard<std::mutex> lock(m_async_conn->event_mutex());
const int command_added = redisAsyncCommandArgv(m_async_conn->connection(), &hiredis_run_callback, data, parArgc, parArgv, parLengths);
@ -202,28 +199,22 @@ namespace redis {
}
bool Batch::replies_requested() const {
return not m_replies.empty();
return static_cast<bool>(0 == m_local_data->local_pending_futures);
}
const std::vector<Reply>& Batch::replies() {
return replies_nonconst();
}
std::vector<Reply>& Batch::replies_nonconst() {
auto Batch::replies() const -> ConstReplies {
if (not replies_requested()) {
if (m_local_data->local_pending_futures > 0) {
std::unique_lock<std::mutex> u_lock(m_local_data->pending_futures_mutex);
m_local_data->no_more_pending_futures.wait(u_lock, [this]() { return m_local_data->local_pending_futures == 0; });
}
m_replies.reserve(m_futures.size());
for (auto& itm : m_futures) {
m_replies.emplace_back(std::move(*itm));
}
auto empty_vec = std::move(m_futures);
}
return m_replies;
return ConstReplies(m_local_data->replies.begin(), m_local_data->replies.end(), m_local_data->replies.size());
}
auto Batch::replies_nonconst() -> Replies {
replies();
return Replies(m_local_data->replies.begin(), m_local_data->replies.end(), m_local_data->replies.size());
}
void Batch::throw_if_failed() {
@ -233,7 +224,7 @@ namespace redis {
oss << "Error in reply: ";
const int err_count = array_throw_if_failed(0, max_reported_errors, replies(), oss);
if (err_count) {
oss << " (showing " << err_count << '/' << max_reported_errors << " errors on " << replies().size() << " total replies)";
oss << " (showing " << err_count << '/' << max_reported_errors << " errors on " << m_local_data->replies.size() << " total replies)";
throw std::runtime_error(oss.str());
}
}
@ -248,8 +239,7 @@ namespace redis {
assert(m_local_data);
assert(0 == m_local_data->local_pending_futures);
m_futures.clear();
m_replies.clear();
m_local_data->replies.clear();
}
RedisError::RedisError (const char* parMessage, std::size_t parLength) :

58
src/reply_list.cpp Normal file
View File

@ -0,0 +1,58 @@
/* Copyright 2016, Michele Santullo
* This file is part of "incredis".
*
* "incredis" is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* "incredis" is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with "incredis". If not, see <http://www.gnu.org/licenses/>.
*/
#include "reply_list.hpp"
#include "reply.hpp"
namespace redis {
ReplyList::ReplyList() :
m_replies(),
m_next_iterator(m_replies.before_begin()),
m_size(0)
{
}
ReplyList::~ReplyList() noexcept = default;
auto ReplyList::add() -> ReplyPtr {
m_next_iterator = m_replies.emplace_after(m_next_iterator);
++m_size;
return m_next_iterator;
}
std::size_t ReplyList::size() const {
return m_size;
}
bool ReplyList::empty() const {
return static_cast<bool>(0 == m_size);
}
std::forward_list<Reply>::iterator ReplyList::begin() {
return m_replies.begin();
}
std::forward_list<Reply>::iterator ReplyList::end() {
return m_replies.end();
}
void ReplyList::clear() {
m_replies.clear();
m_next_iterator = m_replies.begin();
m_size = 0;
}
} //namespace redis

47
src/reply_list.hpp Normal file
View File

@ -0,0 +1,47 @@
/* Copyright 2016, Michele Santullo
* This file is part of "incredis".
*
* "incredis" is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* "incredis" is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with "incredis". If not, see <http://www.gnu.org/licenses/>.
*/
#ifndef idF28625434960439DBB5B4F2A1E24CD60
#define idF28625434960439DBB5B4F2A1E24CD60
#include <forward_list>
namespace redis {
class Reply;
class ReplyList {
public:
using ReplyPtr = std::forward_list<Reply>::iterator;
ReplyList ( void );
~ReplyList ( void ) noexcept;
ReplyPtr add ( void );
std::size_t size ( void ) const;
bool empty ( void ) const;
std::forward_list<Reply>::iterator begin ( void );
std::forward_list<Reply>::iterator end ( void );
void clear ( void );
private:
std::forward_list<Reply> m_replies;
std::forward_list<Reply>::iterator m_next_iterator;
std::size_t m_size;
};
} //namespace redis
#endif