From 5092c95baab45d27248e1b54bf17cd7b1fc42e06 Mon Sep 17 00:00:00 2001 From: Matt Witherspoon <32485495+spoonincode@users.noreply.github.com> Date: Mon, 15 Jul 2019 22:37:11 -0400 Subject: [PATCH 1/6] use checktime timer exclusively for checktime Remove the "gear down" from timer to poll and instead strictly use a timer --- libraries/chain/checktime_timer_calibrate.cpp | 19 ++--- libraries/chain/checktime_timer_dummy.cpp | 85 +++++++++++++++++-- libraries/chain/checktime_timer_macos.cpp | 14 ++- libraries/chain/checktime_timer_posix.cpp | 12 +-- .../eosio/chain/checktime_timer_calibrate.hpp | 21 +---- libraries/chain/transaction_context.cpp | 52 ++++++------ 6 files changed, 120 insertions(+), 83 deletions(-) diff --git a/libraries/chain/checktime_timer_calibrate.cpp b/libraries/chain/checktime_timer_calibrate.cpp index cdff4ee98fe..fccd8479764 100755 --- a/libraries/chain/checktime_timer_calibrate.cpp +++ b/libraries/chain/checktime_timer_calibrate.cpp @@ -18,10 +18,7 @@ namespace eosio { namespace chain { namespace bacc = boost::accumulators; -checktime_timer_calibrate::checktime_timer_calibrate() = default; -checktime_timer_calibrate::~checktime_timer_calibrate() = default; - -void checktime_timer_calibrate::do_calibrate(checktime_timer& timer) { +void compute_and_print_timer_accuracy(checktime_timer& timer) { static std::mutex m; static bool once_is_enough; @@ -33,7 +30,7 @@ void checktime_timer_calibrate::do_calibrate(checktime_timer& timer) { bacc::accumulator_set, float> samples; //keep longest first in list. You're effectively going to take test_intervals[0]*sizeof(test_intervals[0]) - //time to do the the "calibration" + //time to do the the test int test_intervals[] = {50000, 10000, 5000, 1000, 500, 100, 50, 10}; for(int& interval : test_intervals) { @@ -56,19 +53,15 @@ void checktime_timer_calibrate::do_calibrate(checktime_timer& timer) { samples(timer_slop, bacc::weight = interval/(float)test_intervals[0]); } } - _timer_overhead = bacc::mean(samples) + sqrt(bacc::variance(samples))*2; //target 95% of expirations before deadline - _use_timer = _timer_overhead < 1000; #define TIMER_STATS_FORMAT "min:${min}us max:${max}us mean:${mean}us stddev:${stddev}us" #define TIMER_STATS \ ("min", bacc::min(samples))("max", bacc::max(samples)) \ - ("mean", (int)bacc::mean(samples))("stddev", (int)sqrt(bacc::variance(samples))) \ - ("t", _timer_overhead) + ("mean", (int)bacc::mean(samples))("stddev", (int)sqrt(bacc::variance(samples))) - if(_use_timer) - ilog("Using ${t}us deadline timer for checktime: " TIMER_STATS_FORMAT, TIMER_STATS); - else - wlog("Using polled checktime; deadline timer too inaccurate: " TIMER_STATS_FORMAT, TIMER_STATS); + ilog("Checktime timer accuracy: " TIMER_STATS_FORMAT, TIMER_STATS); + if(bacc::mean(samples) + sqrt(bacc::variance(samples))*2 > 250) + wlog("Checktime timer accuracy on this platform and hardware combination is poor; subjective billing accuracy may suffer"); once_is_enough = true; } diff --git a/libraries/chain/checktime_timer_dummy.cpp b/libraries/chain/checktime_timer_dummy.cpp index 0c82d258cbf..2ad1d34c111 100644 --- a/libraries/chain/checktime_timer_dummy.cpp +++ b/libraries/chain/checktime_timer_dummy.cpp @@ -1,16 +1,91 @@ #include +#include + #include +#include //set_os_thread_name() + +#include + +#include +#include namespace eosio { namespace chain { -struct checktime_timer::impl {}; +//a thread is shared for all instances +static std::mutex timer_ref_mutex; +static unsigned refcount; +static std::thread checktime_thread; +static std::unique_ptr checktime_ios; + +struct checktime_timer::impl { + std::unique_ptr timer; +}; checktime_timer::checktime_timer() { - expired = 1; + static_assert(sizeof(impl) <= fwd_size); + + std::lock_guard guard(timer_ref_mutex); + + if(refcount++ == 0) { + std::promise p; + checktime_thread = std::thread([&p]() { + fc::set_os_thread_name("checktime"); + checktime_ios = std::make_unique(); + boost::asio::io_service::work work(*checktime_ios); + p.set_value(); + + checktime_ios->run(); + }); + p.get_future().get(); + } + + my->timer = std::make_unique(*checktime_ios); + + //compute_and_print_timer_accuracy(*this); +} + +checktime_timer::~checktime_timer() { + stop(); + if(std::lock_guard guard(timer_ref_mutex); --refcount == 0) { + checktime_ios->stop(); + checktime_thread.join(); + checktime_ios.reset(); + } +} + +void checktime_timer::start(fc::time_point tp) { + if(tp == fc::time_point::maximum()) { + expired = 0; + return; + } + fc::microseconds x = tp.time_since_epoch() - fc::time_point::now().time_since_epoch(); + if(x.count() <= 0) + expired = 1; + else { +#if 0 + std::promise p; + checktime_ios->post([&p,this]() { + expired = 0; + p.set_value(); + }); + p.get_future().get(); +#endif + expired = 0; + my->timer->expires_after(std::chrono::microseconds((int)x.count())); + my->timer->async_wait([this](const boost::system::error_code& ec) { + if(ec) + return; + expired = 1; + }); + } } -checktime_timer::~checktime_timer() {} -void checktime_timer::start(fc::time_point tp) {} -void checktime_timer::stop() {} +void checktime_timer::stop() { + if(expired) + return; + + my->timer->cancel(); + expired = 1; +} }} \ No newline at end of file diff --git a/libraries/chain/checktime_timer_macos.cpp b/libraries/chain/checktime_timer_macos.cpp index 5355bead86a..a667448dfc9 100755 --- a/libraries/chain/checktime_timer_macos.cpp +++ b/libraries/chain/checktime_timer_macos.cpp @@ -4,6 +4,7 @@ #include #include #include +#include //set_os_thread_name() #include #include @@ -19,8 +20,6 @@ static unsigned refcount; static int kqueue_fd; static std::thread kevent_thread; -static checktime_timer_calibrate the_calibrate; - struct checktime_timer::impl { uint64_t timerid; @@ -43,6 +42,7 @@ checktime_timer::checktime_timer() { FC_ASSERT(kevent64(kqueue_fd, &quit_event, 1, NULL, 0, KEVENT_FLAG_IMMEDIATE, NULL) == 0, "failed to create quit event"); kevent_thread = std::thread([]() { + fc::set_os_thread_name("checktime"); while(true) { struct kevent64_s anEvent; int c = kevent64(kqueue_fd, NULL, 0, &anEvent, 1, 0, NULL); @@ -61,7 +61,7 @@ checktime_timer::checktime_timer() { my->timerid = next_timerid++; - the_calibrate.do_calibrate(*this); + compute_and_print_timer_accuracy(*this); } checktime_timer::~checktime_timer() { @@ -81,16 +81,12 @@ void checktime_timer::start(fc::time_point tp) { expired = 0; return; } - if(!the_calibrate.use_timer()) { - expired = 1; - return; - } fc::microseconds x = tp.time_since_epoch() - fc::time_point::now().time_since_epoch(); - if(x.count() <= the_calibrate.timer_overhead()) + if(x.count() <= 0) expired = 1; else { struct kevent64_s aTimerEvent; - EV_SET64(&aTimerEvent, my->timerid, EVFILT_TIMER, EV_ADD|EV_ENABLE|EV_ONESHOT, NOTE_USECONDS|NOTE_CRITICAL, (int)x.count()-the_calibrate.timer_overhead(), (uint64_t)&expired, 0, 0); + EV_SET64(&aTimerEvent, my->timerid, EVFILT_TIMER, EV_ADD|EV_ENABLE|EV_ONESHOT, NOTE_USECONDS|NOTE_CRITICAL, (int)x.count(), (uint64_t)&expired, 0, 0); expired = 0; if(kevent64(kqueue_fd, &aTimerEvent, 1, NULL, 0, KEVENT_FLAG_IMMEDIATE, NULL) != 0) diff --git a/libraries/chain/checktime_timer_posix.cpp b/libraries/chain/checktime_timer_posix.cpp index 81167ff5b55..1a7b2e62adb 100644 --- a/libraries/chain/checktime_timer_posix.cpp +++ b/libraries/chain/checktime_timer_posix.cpp @@ -12,8 +12,6 @@ namespace eosio { namespace chain { -static checktime_timer_calibrate the_calibrate; - struct checktime_timer::impl { timer_t timerid; @@ -44,7 +42,7 @@ checktime_timer::checktime_timer() { FC_ASSERT(timer_create(CLOCK_REALTIME, &se, &my->timerid) == 0, "failed to create timer"); - the_calibrate.do_calibrate(*this); + compute_and_print_timer_accuracy(*this); } checktime_timer::~checktime_timer() { @@ -56,15 +54,11 @@ void checktime_timer::start(fc::time_point tp) { expired = 0; return; } - if(!the_calibrate.use_timer()) { - expired = 1; - return; - } fc::microseconds x = tp.time_since_epoch() - fc::time_point::now().time_since_epoch(); - if(x.count() <= the_calibrate.timer_overhead()) + if(x.count() <= 0) expired = 1; else { - struct itimerspec enable = {{0, 0}, {0, ((int)x.count()-the_calibrate.timer_overhead())*1000}}; + struct itimerspec enable = {{0, 0}, {0, (int)x.count()*1000}}; expired = 0; if(timer_settime(my->timerid, 0, &enable, NULL) != 0) expired = 1; diff --git a/libraries/chain/include/eosio/chain/checktime_timer_calibrate.hpp b/libraries/chain/include/eosio/chain/checktime_timer_calibrate.hpp index 21604652805..5cc3c7237e5 100755 --- a/libraries/chain/include/eosio/chain/checktime_timer_calibrate.hpp +++ b/libraries/chain/include/eosio/chain/checktime_timer_calibrate.hpp @@ -1,27 +1,8 @@ #pragma once -#include - namespace eosio { namespace chain { struct checktime_timer; - -struct checktime_timer_calibrate { - checktime_timer_calibrate(); - //will run calibration if not already run on this instance - void do_calibrate(checktime_timer& t); - ~checktime_timer_calibrate(); - - bool use_timer() const {return _use_timer;} - int timer_overhead() const {return _timer_overhead;} - -private: - //These two values will be inspected by checktime_timer impls to determine if they - // should use the timer (accurate enough) and what slop to give the expiration. Set - // the defaults to a "perfect" timer because checktime_timer_calibrate will create - // a checktime_timer to calibrate with. - bool _use_timer = true; - int _timer_overhead = 0; -}; +void compute_and_print_timer_accuracy(checktime_timer& t); }} \ No newline at end of file diff --git a/libraries/chain/transaction_context.cpp b/libraries/chain/transaction_context.cpp index 6aed4868752..10732fba6a0 100644 --- a/libraries/chain/transaction_context.cpp +++ b/libraries/chain/transaction_context.cpp @@ -156,13 +156,13 @@ namespace eosio { namespace chain { if( initial_net_usage > 0 ) add_net_usage( initial_net_usage ); // Fail early if current net usage is already greater than the calculated limit - checktime(); // Fail early if deadline has already been exceeded - if(control.skip_trx_checks()) deadline_timer.start(fc::time_point::maximum()); else deadline_timer.start(_deadline); + checktime(); // Fail early if deadline has already been exceeded + is_initialized = true; } @@ -346,34 +346,32 @@ namespace eosio { namespace chain { void transaction_context::checktime()const { if(BOOST_LIKELY(deadline_timer.expired == false)) return; + auto now = fc::time_point::now(); - if( BOOST_UNLIKELY( now > _deadline ) ) { - // edump((now-start)(now-pseudo_start)); - if( explicit_billed_cpu_time || deadline_exception_code == deadline_exception::code_value ) { - EOS_THROW( deadline_exception, "deadline exceeded ${billing_timer}us", - ("billing_timer", now - pseudo_start)("now", now)("deadline", _deadline)("start", start) ); - } else if( deadline_exception_code == block_cpu_usage_exceeded::code_value ) { - EOS_THROW( block_cpu_usage_exceeded, - "not enough time left in block to complete executing transaction ${billing_timer}us", - ("now", now)("deadline", _deadline)("start", start)("billing_timer", now - pseudo_start) ); - } else if( deadline_exception_code == tx_cpu_usage_exceeded::code_value ) { - if (cpu_limit_due_to_greylist) { - EOS_THROW( greylist_cpu_usage_exceeded, - "greylisted transaction was executing for too long ${billing_timer}us", - ("now", now)("deadline", _deadline)("start", start)("billing_timer", now - pseudo_start) ); - } else { - EOS_THROW( tx_cpu_usage_exceeded, - "transaction was executing for too long ${billing_timer}us", - ("now", now)("deadline", _deadline)("start", start)("billing_timer", now - pseudo_start) ); - } - } else if( deadline_exception_code == leeway_deadline_exception::code_value ) { - EOS_THROW( leeway_deadline_exception, - "the transaction was unable to complete by deadline, " - "but it is possible it could have succeeded if it were allowed to run to completion ${billing_timer}", - ("now", now)("deadline", _deadline)("start", start)("billing_timer", now - pseudo_start) ); + if( explicit_billed_cpu_time || deadline_exception_code == deadline_exception::code_value ) { + EOS_THROW( deadline_exception, "deadline exceeded ${billing_timer}us", + ("billing_timer", now - pseudo_start)("now", now)("deadline", _deadline)("start", start) ); + } else if( deadline_exception_code == block_cpu_usage_exceeded::code_value ) { + EOS_THROW( block_cpu_usage_exceeded, + "not enough time left in block to complete executing transaction ${billing_timer}us", + ("now", now)("deadline", _deadline)("start", start)("billing_timer", now - pseudo_start) ); + } else if( deadline_exception_code == tx_cpu_usage_exceeded::code_value ) { + if (cpu_limit_due_to_greylist) { + EOS_THROW( greylist_cpu_usage_exceeded, + "greylisted transaction was executing for too long ${billing_timer}us", + ("now", now)("deadline", _deadline)("start", start)("billing_timer", now - pseudo_start) ); + } else { + EOS_THROW( tx_cpu_usage_exceeded, + "transaction was executing for too long ${billing_timer}us", + ("now", now)("deadline", _deadline)("start", start)("billing_timer", now - pseudo_start) ); } - EOS_ASSERT( false, transaction_exception, "unexpected deadline exception code ${code}", ("code", deadline_exception_code) ); + } else if( deadline_exception_code == leeway_deadline_exception::code_value ) { + EOS_THROW( leeway_deadline_exception, + "the transaction was unable to complete by deadline, " + "but it is possible it could have succeeded if it were allowed to run to completion ${billing_timer}", + ("now", now)("deadline", _deadline)("start", start)("billing_timer", now - pseudo_start) ); } + EOS_ASSERT( false, transaction_exception, "unexpected deadline exception code ${code}", ("code", deadline_exception_code) ); } void transaction_context::pause_billing_timer() { From 0cc3261a0ffaecbe4c2d941e1c68bc4228b03d16 Mon Sep 17 00:00:00 2001 From: Matt Witherspoon <32485495+spoonincode@users.noreply.github.com> Date: Mon, 15 Jul 2019 22:42:17 -0400 Subject: [PATCH 2/6] rename some of the checktime files --- libraries/chain/CMakeLists.txt | 4 ++-- ...ktime_timer_calibrate.cpp => checktime_timer_accuracy.cpp} | 4 ++-- ...time_timer_dummy.cpp => checktime_timer_asio_fallback.cpp} | 4 ++-- libraries/chain/checktime_timer_macos.cpp | 4 ++-- libraries/chain/checktime_timer_posix.cpp | 4 ++-- ...ktime_timer_calibrate.hpp => checktime_timer_accuracy.hpp} | 0 6 files changed, 10 insertions(+), 10 deletions(-) rename libraries/chain/{checktime_timer_calibrate.cpp => checktime_timer_accuracy.cpp} (98%) rename libraries/chain/{checktime_timer_dummy.cpp => checktime_timer_asio_fallback.cpp} (97%) rename libraries/chain/include/eosio/chain/{checktime_timer_calibrate.hpp => checktime_timer_accuracy.hpp} (100%) diff --git a/libraries/chain/CMakeLists.txt b/libraries/chain/CMakeLists.txt index b47b1e4aaf2..f52e3145201 100644 --- a/libraries/chain/CMakeLists.txt +++ b/libraries/chain/CMakeLists.txt @@ -12,7 +12,7 @@ else() if(POSIX_TIMER_TEST_RUN_RESULT EQUAL 0) set(CHECKTIME_TIMER_IMPL checktime_timer_posix.cpp) else() - set(CHECKTIME_TIMER_IMPL checktime_timer_dummy.cpp) + set(CHECKTIME_TIMER_IMPL checktime_timer_asio_fallback.cpp) endif() endif() @@ -63,7 +63,7 @@ add_library( eosio_chain genesis_intrinsics.cpp whitelisted_intrinsics.cpp thread_utils.cpp - checktime_timer_calibrate.cpp + checktime_timer_accuracy.cpp ${CHECKTIME_TIMER_IMPL} ${HEADERS} ) diff --git a/libraries/chain/checktime_timer_calibrate.cpp b/libraries/chain/checktime_timer_accuracy.cpp similarity index 98% rename from libraries/chain/checktime_timer_calibrate.cpp rename to libraries/chain/checktime_timer_accuracy.cpp index fccd8479764..b24349d2315 100755 --- a/libraries/chain/checktime_timer_calibrate.cpp +++ b/libraries/chain/checktime_timer_accuracy.cpp @@ -1,4 +1,4 @@ -#include +#include #include #include @@ -66,4 +66,4 @@ void compute_and_print_timer_accuracy(checktime_timer& timer) { once_is_enough = true; } -}} \ No newline at end of file +}} diff --git a/libraries/chain/checktime_timer_dummy.cpp b/libraries/chain/checktime_timer_asio_fallback.cpp similarity index 97% rename from libraries/chain/checktime_timer_dummy.cpp rename to libraries/chain/checktime_timer_asio_fallback.cpp index 2ad1d34c111..9d1cb7d454b 100644 --- a/libraries/chain/checktime_timer_dummy.cpp +++ b/libraries/chain/checktime_timer_asio_fallback.cpp @@ -1,5 +1,5 @@ #include -#include +#include #include #include //set_os_thread_name() @@ -88,4 +88,4 @@ void checktime_timer::stop() { expired = 1; } -}} \ No newline at end of file +}} diff --git a/libraries/chain/checktime_timer_macos.cpp b/libraries/chain/checktime_timer_macos.cpp index a667448dfc9..5a1b6dd9910 100755 --- a/libraries/chain/checktime_timer_macos.cpp +++ b/libraries/chain/checktime_timer_macos.cpp @@ -1,5 +1,5 @@ #include -#include +#include #include #include @@ -104,4 +104,4 @@ void checktime_timer::stop() { expired = 1; } -}} \ No newline at end of file +}} diff --git a/libraries/chain/checktime_timer_posix.cpp b/libraries/chain/checktime_timer_posix.cpp index 1a7b2e62adb..8b1ccd3a1d6 100644 --- a/libraries/chain/checktime_timer_posix.cpp +++ b/libraries/chain/checktime_timer_posix.cpp @@ -1,5 +1,5 @@ #include -#include +#include #include #include @@ -73,4 +73,4 @@ void checktime_timer::stop() { expired = 1; } -}} \ No newline at end of file +}} diff --git a/libraries/chain/include/eosio/chain/checktime_timer_calibrate.hpp b/libraries/chain/include/eosio/chain/checktime_timer_accuracy.hpp similarity index 100% rename from libraries/chain/include/eosio/chain/checktime_timer_calibrate.hpp rename to libraries/chain/include/eosio/chain/checktime_timer_accuracy.hpp From f30742e5589a7039fa3e9f094454b191fb966b93 Mon Sep 17 00:00:00 2001 From: Matt Witherspoon <32485495+spoonincode@users.noreply.github.com> Date: Tue, 16 Jul 2019 09:34:05 -0400 Subject: [PATCH 3/6] tweak checktime time accuracy warning --- libraries/chain/checktime_timer_accuracy.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/chain/checktime_timer_accuracy.cpp b/libraries/chain/checktime_timer_accuracy.cpp index b24349d2315..e432dea2d1f 100755 --- a/libraries/chain/checktime_timer_accuracy.cpp +++ b/libraries/chain/checktime_timer_accuracy.cpp @@ -61,7 +61,7 @@ void compute_and_print_timer_accuracy(checktime_timer& timer) { ilog("Checktime timer accuracy: " TIMER_STATS_FORMAT, TIMER_STATS); if(bacc::mean(samples) + sqrt(bacc::variance(samples))*2 > 250) - wlog("Checktime timer accuracy on this platform and hardware combination is poor; subjective billing accuracy may suffer"); + wlog("Checktime timer accuracy on this platform and hardware combination is poor; accuracy of subjective transaction deadline enforcement will suffer"); once_is_enough = true; } From db75d9ae2cf7ab190454916df159ab4ee501b5b6 Mon Sep 17 00:00:00 2001 From: Matt Witherspoon <32485495+spoonincode@users.noreply.github.com> Date: Wed, 14 Aug 2019 16:06:37 -0400 Subject: [PATCH 4/6] rename checktime_timer to platform_timer --- libraries/chain/CMakeLists.txt | 12 ++++++------ libraries/chain/controller.cpp | 4 ++-- .../eosio/chain/checktime_timer_accuracy.hpp | 8 -------- .../{checktime_timer.hpp => platform_timer.hpp} | 14 +++++++------- .../eosio/chain/platform_timer_accuracy.hpp | 8 ++++++++ .../include/eosio/chain/transaction_context.hpp | 8 ++++---- ..._accuracy.cpp => platform_timer_accuracy.cpp} | 6 +++--- ...back.cpp => platform_timer_asio_fallback.cpp} | 14 +++++++------- ..._timer_macos.cpp => platform_timer_macos.cpp} | 16 ++++++++-------- ..._timer_posix.cpp => platform_timer_posix.cpp} | 14 +++++++------- ..._posix_test.c => platform_timer_posix_test.c} | 0 libraries/chain/transaction_context.cpp | 2 +- 12 files changed, 53 insertions(+), 53 deletions(-) delete mode 100755 libraries/chain/include/eosio/chain/checktime_timer_accuracy.hpp rename libraries/chain/include/eosio/chain/{checktime_timer.hpp => platform_timer.hpp} (59%) create mode 100755 libraries/chain/include/eosio/chain/platform_timer_accuracy.hpp rename libraries/chain/{checktime_timer_accuracy.cpp => platform_timer_accuracy.cpp} (94%) rename libraries/chain/{checktime_timer_asio_fallback.cpp => platform_timer_asio_fallback.cpp} (87%) rename libraries/chain/{checktime_timer_macos.cpp => platform_timer_macos.cpp} (89%) rename libraries/chain/{checktime_timer_posix.cpp => platform_timer_posix.cpp} (85%) rename libraries/chain/{checktime_timer_posix_test.c => platform_timer_posix_test.c} (100%) diff --git a/libraries/chain/CMakeLists.txt b/libraries/chain/CMakeLists.txt index f52e3145201..039bb8d3124 100644 --- a/libraries/chain/CMakeLists.txt +++ b/libraries/chain/CMakeLists.txt @@ -6,13 +6,13 @@ file(GLOB HEADERS "include/eosio/chain/*.hpp" "${CMAKE_CURRENT_BINARY_DIR}/include/eosio/chain/core_symbol.hpp" ) if(APPLE AND UNIX) - set(CHECKTIME_TIMER_IMPL checktime_timer_macos.cpp) + set(PLATFORM_TIMER_IMPL platform_timer_macos.cpp) else() - try_run(POSIX_TIMER_TEST_RUN_RESULT POSIX_TIMER_TEST_COMPILE_RESULT ${CMAKE_CURRENT_BINARY_DIR} ${CMAKE_CURRENT_SOURCE_DIR}/checktime_timer_posix_test.c) + try_run(POSIX_TIMER_TEST_RUN_RESULT POSIX_TIMER_TEST_COMPILE_RESULT ${CMAKE_CURRENT_BINARY_DIR} ${CMAKE_CURRENT_SOURCE_DIR}/platform_timer_posix_test.c) if(POSIX_TIMER_TEST_RUN_RESULT EQUAL 0) - set(CHECKTIME_TIMER_IMPL checktime_timer_posix.cpp) + set(PLATFORM_TIMER_IMPL platform_timer_posix.cpp) else() - set(CHECKTIME_TIMER_IMPL checktime_timer_asio_fallback.cpp) + set(PLATFORM_TIMER_IMPL platform_timer_asio_fallback.cpp) endif() endif() @@ -63,8 +63,8 @@ add_library( eosio_chain genesis_intrinsics.cpp whitelisted_intrinsics.cpp thread_utils.cpp - checktime_timer_accuracy.cpp - ${CHECKTIME_TIMER_IMPL} + platform_timer_accuracy.cpp + ${PLATFORM_TIMER_IMPL} ${HEADERS} ) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 09885bb656a..6a236e111ba 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -24,7 +24,7 @@ #include #include #include -#include +#include #include #include @@ -230,7 +230,7 @@ struct controller_impl { bool trusted_producer_light_validation = false; uint32_t snapshot_head_block = 0; named_thread_pool thread_pool; - checktime_timer timer; + platform_timer timer; typedef pair handler_key; map< account_name, map > apply_handlers; diff --git a/libraries/chain/include/eosio/chain/checktime_timer_accuracy.hpp b/libraries/chain/include/eosio/chain/checktime_timer_accuracy.hpp deleted file mode 100755 index 5cc3c7237e5..00000000000 --- a/libraries/chain/include/eosio/chain/checktime_timer_accuracy.hpp +++ /dev/null @@ -1,8 +0,0 @@ -#pragma once - -namespace eosio { namespace chain { - -struct checktime_timer; -void compute_and_print_timer_accuracy(checktime_timer& t); - -}} \ No newline at end of file diff --git a/libraries/chain/include/eosio/chain/checktime_timer.hpp b/libraries/chain/include/eosio/chain/platform_timer.hpp similarity index 59% rename from libraries/chain/include/eosio/chain/checktime_timer.hpp rename to libraries/chain/include/eosio/chain/platform_timer.hpp index ea820b0aace..203054907e7 100644 --- a/libraries/chain/include/eosio/chain/checktime_timer.hpp +++ b/libraries/chain/include/eosio/chain/platform_timer.hpp @@ -6,9 +6,9 @@ namespace eosio { namespace chain { -struct checktime_timer { - checktime_timer(); - ~checktime_timer(); +struct platform_timer { + platform_timer(); + ~platform_timer(); void start(fc::time_point tp); void stop(); @@ -21,13 +21,13 @@ struct checktime_timer { fc::fwd my; }; -struct checktime_timer_scoped_stop { - checktime_timer_scoped_stop(checktime_timer& t) : _timer(t) {} - ~checktime_timer_scoped_stop() { +struct platform_timer_scoped_stop { + platform_timer_scoped_stop(platform_timer& t) : _timer(t) {} + ~platform_timer_scoped_stop() { _timer.stop(); } private: - checktime_timer& _timer; + platform_timer& _timer; }; }} \ No newline at end of file diff --git a/libraries/chain/include/eosio/chain/platform_timer_accuracy.hpp b/libraries/chain/include/eosio/chain/platform_timer_accuracy.hpp new file mode 100755 index 00000000000..cc83b6a554f --- /dev/null +++ b/libraries/chain/include/eosio/chain/platform_timer_accuracy.hpp @@ -0,0 +1,8 @@ +#pragma once + +namespace eosio { namespace chain { + +struct platform_timer; +void compute_and_print_timer_accuracy(platform_timer& t); + +}} \ No newline at end of file diff --git a/libraries/chain/include/eosio/chain/transaction_context.hpp b/libraries/chain/include/eosio/chain/transaction_context.hpp index 7bbe0e66b09..ecb9a8bb5e1 100644 --- a/libraries/chain/include/eosio/chain/transaction_context.hpp +++ b/libraries/chain/include/eosio/chain/transaction_context.hpp @@ -1,7 +1,7 @@ #pragma once #include #include -#include +#include #include namespace eosio { namespace chain { @@ -15,7 +15,7 @@ namespace eosio { namespace chain { transaction_context( controller& c, const signed_transaction& t, const transaction_id_type& trx_id, - checktime_timer& timer, + platform_timer& timer, fc::time_point start = fc::time_point::now() ); void init_for_implicit_trx( uint64_t initial_net_usage = 0 ); @@ -128,8 +128,8 @@ namespace eosio { namespace chain { fc::microseconds billed_time; fc::microseconds billing_timer_duration_limit; - checktime_timer& deadline_timer; - checktime_timer_scoped_stop timer_stopper; + platform_timer& deadline_timer; + platform_timer_scoped_stop timer_stopper; }; } } diff --git a/libraries/chain/checktime_timer_accuracy.cpp b/libraries/chain/platform_timer_accuracy.cpp similarity index 94% rename from libraries/chain/checktime_timer_accuracy.cpp rename to libraries/chain/platform_timer_accuracy.cpp index e432dea2d1f..94a69c89440 100755 --- a/libraries/chain/checktime_timer_accuracy.cpp +++ b/libraries/chain/platform_timer_accuracy.cpp @@ -1,5 +1,5 @@ -#include -#include +#include +#include #include #include @@ -18,7 +18,7 @@ namespace eosio { namespace chain { namespace bacc = boost::accumulators; -void compute_and_print_timer_accuracy(checktime_timer& timer) { +void compute_and_print_timer_accuracy(platform_timer& timer) { static std::mutex m; static bool once_is_enough; diff --git a/libraries/chain/checktime_timer_asio_fallback.cpp b/libraries/chain/platform_timer_asio_fallback.cpp similarity index 87% rename from libraries/chain/checktime_timer_asio_fallback.cpp rename to libraries/chain/platform_timer_asio_fallback.cpp index 9d1cb7d454b..47c9b4b4f0b 100644 --- a/libraries/chain/checktime_timer_asio_fallback.cpp +++ b/libraries/chain/platform_timer_asio_fallback.cpp @@ -1,5 +1,5 @@ -#include -#include +#include +#include #include #include //set_os_thread_name() @@ -17,11 +17,11 @@ static unsigned refcount; static std::thread checktime_thread; static std::unique_ptr checktime_ios; -struct checktime_timer::impl { +struct platform_timer::impl { std::unique_ptr timer; }; -checktime_timer::checktime_timer() { +platform_timer::platform_timer() { static_assert(sizeof(impl) <= fwd_size); std::lock_guard guard(timer_ref_mutex); @@ -44,7 +44,7 @@ checktime_timer::checktime_timer() { //compute_and_print_timer_accuracy(*this); } -checktime_timer::~checktime_timer() { +platform_timer::~platform_timer() { stop(); if(std::lock_guard guard(timer_ref_mutex); --refcount == 0) { checktime_ios->stop(); @@ -53,7 +53,7 @@ checktime_timer::~checktime_timer() { } } -void checktime_timer::start(fc::time_point tp) { +void platform_timer::start(fc::time_point tp) { if(tp == fc::time_point::maximum()) { expired = 0; return; @@ -80,7 +80,7 @@ void checktime_timer::start(fc::time_point tp) { } } -void checktime_timer::stop() { +void platform_timer::stop() { if(expired) return; diff --git a/libraries/chain/checktime_timer_macos.cpp b/libraries/chain/platform_timer_macos.cpp similarity index 89% rename from libraries/chain/checktime_timer_macos.cpp rename to libraries/chain/platform_timer_macos.cpp index 5a1b6dd9910..670de808c90 100755 --- a/libraries/chain/checktime_timer_macos.cpp +++ b/libraries/chain/platform_timer_macos.cpp @@ -1,5 +1,5 @@ -#include -#include +#include +#include #include #include @@ -13,20 +13,20 @@ namespace eosio { namespace chain { -//a kqueue & thread is shared for all checktime_timer_macos instances +//a kqueue & thread is shared for all platform_timer_macos instances static std::mutex timer_ref_mutex; static unsigned next_timerid; static unsigned refcount; static int kqueue_fd; static std::thread kevent_thread; -struct checktime_timer::impl { +struct platform_timer::impl { uint64_t timerid; constexpr static uint64_t quit_event_id = 1; }; -checktime_timer::checktime_timer() { +platform_timer::platform_timer() { static_assert(sizeof(impl) <= fwd_size); std::lock_guard guard(timer_ref_mutex); @@ -64,7 +64,7 @@ checktime_timer::checktime_timer() { compute_and_print_timer_accuracy(*this); } -checktime_timer::~checktime_timer() { +platform_timer::~platform_timer() { stop(); if(std::lock_guard guard(timer_ref_mutex); --refcount == 0) { struct kevent64_s signal_quit_event; @@ -76,7 +76,7 @@ checktime_timer::~checktime_timer() { } } -void checktime_timer::start(fc::time_point tp) { +void platform_timer::start(fc::time_point tp) { if(tp == fc::time_point::maximum()) { expired = 0; return; @@ -94,7 +94,7 @@ void checktime_timer::start(fc::time_point tp) { } } -void checktime_timer::stop() { +void platform_timer::stop() { if(expired) return; diff --git a/libraries/chain/checktime_timer_posix.cpp b/libraries/chain/platform_timer_posix.cpp similarity index 85% rename from libraries/chain/checktime_timer_posix.cpp rename to libraries/chain/platform_timer_posix.cpp index 8b1ccd3a1d6..edf4ca806eb 100644 --- a/libraries/chain/checktime_timer_posix.cpp +++ b/libraries/chain/platform_timer_posix.cpp @@ -1,5 +1,5 @@ -#include -#include +#include +#include #include #include @@ -12,7 +12,7 @@ namespace eosio { namespace chain { -struct checktime_timer::impl { +struct platform_timer::impl { timer_t timerid; static void sig_handler(int, siginfo_t* si, void*) { @@ -20,7 +20,7 @@ struct checktime_timer::impl { } }; -checktime_timer::checktime_timer() { +platform_timer::platform_timer() { static_assert(sizeof(impl) <= fwd_size); static bool initialized; @@ -45,11 +45,11 @@ checktime_timer::checktime_timer() { compute_and_print_timer_accuracy(*this); } -checktime_timer::~checktime_timer() { +platform_timer::~platform_timer() { timer_delete(my->timerid); } -void checktime_timer::start(fc::time_point tp) { +void platform_timer::start(fc::time_point tp) { if(tp == fc::time_point::maximum()) { expired = 0; return; @@ -65,7 +65,7 @@ void checktime_timer::start(fc::time_point tp) { } } -void checktime_timer::stop() { +void platform_timer::stop() { if(expired) return; struct itimerspec disable = {{0, 0}, {0, 0}}; diff --git a/libraries/chain/checktime_timer_posix_test.c b/libraries/chain/platform_timer_posix_test.c similarity index 100% rename from libraries/chain/checktime_timer_posix_test.c rename to libraries/chain/platform_timer_posix_test.c diff --git a/libraries/chain/transaction_context.cpp b/libraries/chain/transaction_context.cpp index 10732fba6a0..4bce198c915 100644 --- a/libraries/chain/transaction_context.cpp +++ b/libraries/chain/transaction_context.cpp @@ -24,7 +24,7 @@ namespace eosio { namespace chain { transaction_context::transaction_context( controller& c, const signed_transaction& t, const transaction_id_type& trx_id, - checktime_timer& tmr, + platform_timer& tmr, fc::time_point s ) :control(c) ,trx(t) From 13b745289486e9df42b76cc3760a30e88da86f05 Mon Sep 17 00:00:00 2001 From: Matt Witherspoon <32485495+spoonincode@users.noreply.github.com> Date: Wed, 14 Aug 2019 18:00:24 -0400 Subject: [PATCH 5/6] instead of using timer directly, have trx_context take a transaction_checktime_timer transaction_checktime_timer bundles up some of the RAII-lacking problems of the previous code -- no longer does transaction_context need to bundle its own scoped exit that stops the timer, and transaction_context can be assured when it is given the transaction_checktimer_timer that the "expired" value is initialized --- libraries/chain/controller.cpp | 11 ++++--- .../include/eosio/chain/platform_timer.hpp | 9 ------ .../eosio/chain/transaction_context.hpp | 23 +++++++++++-- libraries/chain/transaction_context.cpp | 32 ++++++++++++++----- 4 files changed, 51 insertions(+), 24 deletions(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 6a236e111ba..ac23c6807e7 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -230,7 +230,7 @@ struct controller_impl { bool trusted_producer_light_validation = false; uint32_t snapshot_head_block = 0; named_thread_pool thread_pool; - platform_timer timer; + platform_timer timer; typedef pair handler_key; map< account_name, map > apply_handlers; @@ -1024,7 +1024,8 @@ struct controller_impl { etrx.set_reference_block( self.head_block_id() ); } - transaction_context trx_context( self, etrx, etrx.id(), timer, start ); + transaction_checktime_timer trx_timer(timer); + transaction_context trx_context( self, etrx, etrx.id(), std::move(trx_timer), start ); trx_context.deadline = deadline; trx_context.explicit_billed_cpu_time = explicit_billed_cpu_time; trx_context.billed_cpu_time_us = billed_cpu_time_us; @@ -1147,7 +1148,8 @@ struct controller_impl { uint32_t cpu_time_to_bill_us = billed_cpu_time_us; - transaction_context trx_context( self, dtrx, gtrx.trx_id, timer ); + transaction_checktime_timer trx_timer(timer); + transaction_context trx_context( self, dtrx, gtrx.trx_id, std::move(trx_timer) ); trx_context.leeway = fc::microseconds(0); // avoid stealing cpu resource trx_context.deadline = deadline; trx_context.explicit_billed_cpu_time = explicit_billed_cpu_time; @@ -1315,7 +1317,8 @@ struct controller_impl { } const signed_transaction& trn = trx->packed_trx()->get_signed_transaction(); - transaction_context trx_context(self, trn, trx->id(), timer, start); + transaction_checktime_timer trx_timer(timer); + transaction_context trx_context(self, trn, trx->id(), std::move(trx_timer), start); if ((bool)subjective_cpu_leeway && pending->_block_status == controller::block_status::incomplete) { trx_context.leeway = *subjective_cpu_leeway; } diff --git a/libraries/chain/include/eosio/chain/platform_timer.hpp b/libraries/chain/include/eosio/chain/platform_timer.hpp index 203054907e7..90a3f8400ac 100644 --- a/libraries/chain/include/eosio/chain/platform_timer.hpp +++ b/libraries/chain/include/eosio/chain/platform_timer.hpp @@ -21,13 +21,4 @@ struct platform_timer { fc::fwd my; }; -struct platform_timer_scoped_stop { - platform_timer_scoped_stop(platform_timer& t) : _timer(t) {} - ~platform_timer_scoped_stop() { - _timer.stop(); - } -private: - platform_timer& _timer; -}; - }} \ No newline at end of file diff --git a/libraries/chain/include/eosio/chain/transaction_context.hpp b/libraries/chain/include/eosio/chain/transaction_context.hpp index ecb9a8bb5e1..31fec506ceb 100644 --- a/libraries/chain/include/eosio/chain/transaction_context.hpp +++ b/libraries/chain/include/eosio/chain/transaction_context.hpp @@ -6,6 +6,24 @@ namespace eosio { namespace chain { + struct transaction_checktime_timer { + public: + transaction_checktime_timer() = delete; + transaction_checktime_timer(const transaction_checktime_timer&) = delete; + transaction_checktime_timer(transaction_checktime_timer&&) = default; + ~transaction_checktime_timer(); + + void start(fc::time_point tp); + void stop(); + + volatile sig_atomic_t& expired; + private: + platform_timer& _timer; + + transaction_checktime_timer(platform_timer& timer); + friend controller_impl; + }; + class transaction_context { private: void init( uint64_t initial_net_usage); @@ -15,7 +33,7 @@ namespace eosio { namespace chain { transaction_context( controller& c, const signed_transaction& t, const transaction_id_type& trx_id, - platform_timer& timer, + transaction_checktime_timer&& timer, fc::time_point start = fc::time_point::now() ); void init_for_implicit_trx( uint64_t initial_net_usage = 0 ); @@ -128,8 +146,7 @@ namespace eosio { namespace chain { fc::microseconds billed_time; fc::microseconds billing_timer_duration_limit; - platform_timer& deadline_timer; - platform_timer_scoped_stop timer_stopper; + transaction_checktime_timer transaction_timer; }; } } diff --git a/libraries/chain/transaction_context.cpp b/libraries/chain/transaction_context.cpp index 4bce198c915..f40d7877a78 100644 --- a/libraries/chain/transaction_context.cpp +++ b/libraries/chain/transaction_context.cpp @@ -21,10 +21,27 @@ namespace eosio { namespace chain { + transaction_checktime_timer::transaction_checktime_timer(platform_timer& timer) + : expired(timer.expired), _timer(timer) { + expired = 0; + } + + void transaction_checktime_timer::start(fc::time_point tp) { + _timer.start(tp); + } + + void transaction_checktime_timer::stop() { + _timer.stop(); + } + + transaction_checktime_timer::~transaction_checktime_timer() { + stop(); + } + transaction_context::transaction_context( controller& c, const signed_transaction& t, const transaction_id_type& trx_id, - platform_timer& tmr, + transaction_checktime_timer&& tmr, fc::time_point s ) :control(c) ,trx(t) @@ -34,8 +51,7 @@ namespace eosio { namespace chain { ,start(s) ,net_usage(trace->net_usage) ,pseudo_start(s) - ,deadline_timer(tmr) - ,timer_stopper(tmr) + ,transaction_timer(std::move(tmr)) { if (!c.skip_db_sessions()) { undo_session = c.mutable_db().start_undo_session(true); @@ -157,9 +173,9 @@ namespace eosio { namespace chain { add_net_usage( initial_net_usage ); // Fail early if current net usage is already greater than the calculated limit if(control.skip_trx_checks()) - deadline_timer.start(fc::time_point::maximum()); + transaction_timer.start(fc::time_point::maximum()); else - deadline_timer.start(_deadline); + transaction_timer.start(_deadline); checktime(); // Fail early if deadline has already been exceeded @@ -344,7 +360,7 @@ namespace eosio { namespace chain { } void transaction_context::checktime()const { - if(BOOST_LIKELY(deadline_timer.expired == false)) + if(BOOST_LIKELY(transaction_timer.expired == false)) return; auto now = fc::time_point::now(); @@ -381,7 +397,7 @@ namespace eosio { namespace chain { billed_time = now - pseudo_start; deadline_exception_code = deadline_exception::code_value; // Other timeout exceptions cannot be thrown while billable timer is paused. pseudo_start = fc::time_point(); - deadline_timer.stop(); + transaction_timer.stop(); } void transaction_context::resume_billing_timer() { @@ -396,7 +412,7 @@ namespace eosio { namespace chain { _deadline = deadline; deadline_exception_code = deadline_exception::code_value; } - deadline_timer.start(_deadline); + transaction_timer.start(_deadline); } void transaction_context::validate_cpu_usage_to_bill( int64_t billed_us, bool check_minimum )const { From 858379c0bde422386ab3f4546f20ebf2361facfe Mon Sep 17 00:00:00 2001 From: Matt Witherspoon <32485495+spoonincode@users.noreply.github.com> Date: Wed, 14 Aug 2019 18:04:11 -0400 Subject: [PATCH 6/6] restore ordering of a transaction_context check pre-branch change --- libraries/chain/transaction_context.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/chain/transaction_context.cpp b/libraries/chain/transaction_context.cpp index f40d7877a78..ce216063962 100644 --- a/libraries/chain/transaction_context.cpp +++ b/libraries/chain/transaction_context.cpp @@ -172,13 +172,13 @@ namespace eosio { namespace chain { if( initial_net_usage > 0 ) add_net_usage( initial_net_usage ); // Fail early if current net usage is already greater than the calculated limit + checktime(); // Fail early if deadline has already been exceeded + if(control.skip_trx_checks()) transaction_timer.start(fc::time_point::maximum()); else transaction_timer.start(_deadline); - checktime(); // Fail early if deadline has already been exceeded - is_initialized = true; }