Skip to content
This repository has been archived by the owner on Jun 23, 2022. It is now read-only.

Commit

Permalink
fix timer bug in perf counters
Browse files Browse the repository at this point in the history
  • Loading branch information
@0xfdi
0xfdi committed Dec 17, 2015
1 parent 671cd7a commit 5422c63
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 33 deletions.
9 changes: 1 addition & 8 deletions src/core/tests/perf_counter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,20 +92,15 @@ static void test_perf_counter()
std::vector<int> gen_numbers{1, 5, 1043};
int sleep_interval = config()->get_value<int>("components.simple_perf_counter", "counter_computation_interval_seconds", 3, "period");

perf_counter_impl* counter = new perf_counter_impl("", "", dsn_perf_counter_type_t::COUNTER_TYPE_NUMBER,"");
perf_counter_ptr counter = new perf_counter_impl("", "", dsn_perf_counter_type_t::COUNTER_TYPE_NUMBER, "");
perf_counter_inc_dec(counter);
perf_counter_add(counter, vec);
ddebug("%lf", counter->get_value());

//don't delete the counter as it is shared by timer callback
//delete counter;

counter = new perf_counter_impl("", "", dsn_perf_counter_type_t::COUNTER_TYPE_RATE,"");
perf_counter_inc_dec(counter);
perf_counter_add(counter, vec);
ddebug("%lf", counter->get_value());
//don't delete the counter as it is shared by timer callback
//delete counter;

counter = new perf_counter_impl("", "", dsn_perf_counter_type_t::COUNTER_TYPE_NUMBER_PERCENTILES,"");
std::this_thread::sleep_for(std::chrono::seconds(sleep_interval));
Expand All @@ -116,8 +111,6 @@ static void test_perf_counter()
for (int i=0; i!=COUNTER_PERCENTILE_COUNT; ++i)
ddebug("%lf", counter->get_percentile((dsn_perf_counter_percentile_type_t)i));
}
//don't delete the counter as it is shared by timer callback
//delete counter;
}

TEST(tools_common, simple_perf_counter)
Expand Down
28 changes: 19 additions & 9 deletions src/core/tools/common/simple_perf_counter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,14 +120,15 @@ namespace dsn {

_timer.reset(new boost::asio::deadline_timer(shared_io_service::instance().ios));
_timer->expires_from_now(boost::posix_time::seconds(rand() % _counter_computation_interval_seconds + 1));
_timer->async_wait(std::bind(&perf_counter_number_percentile::on_timer, this, std::placeholders::_1));

this->add_ref();
_timer->async_wait(std::bind(&perf_counter_number_percentile::on_timer, this, _timer, std::placeholders::_1));

memset(_samples, 0, sizeof(_samples));
}

~perf_counter_number_percentile(void)
{
_timer->cancel();
}

virtual void increment() { dassert(false, "invalid execution flow"); }
Expand Down Expand Up @@ -281,17 +282,22 @@ namespace dsn {
return;
}

void on_timer(const boost::system::error_code& ec)
void on_timer(std::shared_ptr<boost::asio::deadline_timer> timer, const boost::system::error_code& ec)
{
//as the callback is not in tls context, so the log system calls like ddebug, dassert will cause a lock
if (!ec)
{
boost::shared_ptr<compute_context> ctx(new compute_context());
calc(ctx);
// only when others also hold the reference
if (this->get_count() > 1)
{
boost::shared_ptr<compute_context> ctx(new compute_context());
calc(ctx);

timer->expires_from_now(boost::posix_time::seconds(_counter_computation_interval_seconds));

_timer.reset(new boost::asio::deadline_timer(shared_io_service::instance().ios));
_timer->expires_from_now(boost::posix_time::seconds(_counter_computation_interval_seconds));
_timer->async_wait(std::bind(&perf_counter_number_percentile::on_timer, this, std::placeholders::_1));
this->add_ref();
timer->async_wait(std::bind(&perf_counter_number_percentile::on_timer, this, timer, std::placeholders::_1));
}
}
else if (boost::system::errc::operation_canceled == ec)
{
Expand All @@ -301,6 +307,8 @@ namespace dsn {
{
dassert(false, "on_timer error!!!");
}

this->release_ref();
}

std::shared_ptr<boost::asio::deadline_timer> _timer;
Expand All @@ -321,11 +329,13 @@ namespace dsn {
_counter_impl = new perf_counter_rate(section, name, type, dsptr);
else
_counter_impl = new perf_counter_number_percentile(section, name, type, dsptr);

_counter_impl->add_ref();
}

simple_perf_counter::~simple_perf_counter(void)
{
delete _counter_impl;
_counter_impl->release_ref();
}
}
}
25 changes: 17 additions & 8 deletions src/core/tools/common/simple_perf_counter_v2_atomic.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,8 @@ namespace dsn {
"period (seconds) the system computes the percentiles of the counters");
_timer.reset(new boost::asio::deadline_timer(shared_io_service::instance().ios));
_timer->expires_from_now(boost::posix_time::seconds(rand() % _counter_computation_interval_seconds + 1));
_timer->async_wait(std::bind(&perf_counter_number_percentile_v2_atomic::on_timer, this, std::placeholders::_1));
this->add_ref();
_timer->async_wait(std::bind(&perf_counter_number_percentile_v2_atomic::on_timer, this, _timer, std::placeholders::_1));
}

~perf_counter_number_percentile_v2_atomic(void)
Expand Down Expand Up @@ -327,22 +328,28 @@ namespace dsn {
return;
}

void on_timer(const boost::system::error_code& ec)
void on_timer(std::shared_ptr<boost::asio::deadline_timer> timer, const boost::system::error_code& ec)
{
//as the callback is not in tls context, so the log system calls like ddebug, dassert will cause a lock
if (!ec)
{
boost::shared_ptr<compute_context> ctx(new compute_context());
calc(ctx);
// only when others also hold the reference
if (this->get_count() > 1)
{
boost::shared_ptr<compute_context> ctx(new compute_context());
calc(ctx);

_timer.reset(new boost::asio::deadline_timer(shared_io_service::instance().ios));
_timer->expires_from_now(boost::posix_time::seconds(_counter_computation_interval_seconds));
_timer->async_wait(std::bind(&perf_counter_number_percentile_v2_atomic::on_timer, this, std::placeholders::_1));
timer->expires_from_now(boost::posix_time::seconds(_counter_computation_interval_seconds));
this->add_ref();
timer->async_wait(std::bind(&perf_counter_number_percentile_v2_atomic::on_timer, this, timer, std::placeholders::_1));
}
}
else if (boost::system::errc::operation_canceled != ec)
{
dassert(false, "on_timer error!!!");
}

this->release_ref();
}

std::shared_ptr<boost::asio::deadline_timer> _timer;
Expand All @@ -363,11 +370,13 @@ namespace dsn {
_counter_impl = new perf_counter_rate_v2_atomic(section, name, type, dsptr);
else
_counter_impl = new perf_counter_number_percentile_v2_atomic(section, name, type, dsptr);

_counter_impl->add_ref();
}

simple_perf_counter_v2_atomic::~simple_perf_counter_v2_atomic(void)
{
delete _counter_impl;
_counter_impl->release_ref();
}

#pragma pack(pop)
Expand Down
24 changes: 16 additions & 8 deletions src/core/tools/common/simple_perf_counter_v2_fast.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,8 @@ namespace dsn {
"period (seconds) the system computes the percentiles of the counters");
_timer.reset(new boost::asio::deadline_timer(shared_io_service::instance().ios));
_timer->expires_from_now(boost::posix_time::seconds(rand() % _counter_computation_interval_seconds + 1));
_timer->async_wait(std::bind(&perf_counter_number_percentile_v2_fast::on_timer, this, std::placeholders::_1));
this->add_ref();
_timer->async_wait(std::bind(&perf_counter_number_percentile_v2_fast::on_timer, this, _timer, std::placeholders::_1));
}

~perf_counter_number_percentile_v2_fast(void)
Expand Down Expand Up @@ -326,22 +327,27 @@ namespace dsn {
return;
}

void on_timer(const boost::system::error_code& ec)
void on_timer(std::shared_ptr<boost::asio::deadline_timer> timer, const boost::system::error_code& ec)
{
//as the callback is not in tls context, so the log system calls like ddebug, dassert will cause a lock
if (!ec)
{
boost::shared_ptr<compute_context> ctx(new compute_context());
calc(ctx);
// only when others also hold the reference
if (this->get_count() > 1)
{
boost::shared_ptr<compute_context> ctx(new compute_context());
calc(ctx);

_timer.reset(new boost::asio::deadline_timer(shared_io_service::instance().ios));
_timer->expires_from_now(boost::posix_time::seconds(_counter_computation_interval_seconds));
_timer->async_wait(std::bind(&perf_counter_number_percentile_v2_fast::on_timer, this, std::placeholders::_1));
timer->expires_from_now(boost::posix_time::seconds(_counter_computation_interval_seconds));
this->add_ref();
timer->async_wait(std::bind(&perf_counter_number_percentile_v2_fast::on_timer, this, timer, std::placeholders::_1));
}
}
else if (boost::system::errc::operation_canceled != ec)
{
dassert(false, "on_timer error!!!");
}
this->release_ref();
}

std::shared_ptr<boost::asio::deadline_timer> _timer;
Expand All @@ -362,11 +368,13 @@ namespace dsn {
_counter_impl = new perf_counter_rate_v2_fast(section, name, type, dsptr);
else
_counter_impl = new perf_counter_number_percentile_v2_fast(section, name, type, dsptr);

_counter_impl->add_ref();
}

simple_perf_counter_v2_fast::~simple_perf_counter_v2_fast(void)
{
delete _counter_impl;
_counter_impl->release_ref();
}

#pragma pack(pop)
Expand Down

0 comments on commit 5422c63

Please sign in to comment.