From 688f92686a14250faf95f6d4185855a4ad85099a Mon Sep 17 00:00:00 2001 From: Yuri Gorshenin Date: Fri, 22 May 2015 18:17:08 +0300 Subject: [PATCH] Review fixes. --- android/jni/com/mapswithme/maps/Framework.cpp | 4 +- android/jni/com/mapswithme/maps/Framework.hpp | 6 +-- base/base.pro | 4 +- base/base_tests/base_tests.pro | 2 +- ...d_task_test.cpp => deferred_task_test.cpp} | 29 ++++++------ .../{scheduled_task.cpp => deferred_task.cpp} | 24 +++++----- .../{scheduled_task.hpp => deferred_task.hpp} | 44 ++++++++++--------- qt/draw_widget.cpp | 18 ++++---- qt/draw_widget.hpp | 6 +-- 9 files changed, 72 insertions(+), 65 deletions(-) rename base/base_tests/{scheduled_task_test.cpp => deferred_task_test.cpp} (69%) rename base/{scheduled_task.cpp => deferred_task.cpp} (63%) rename base/{scheduled_task.hpp => deferred_task.hpp} (66%) diff --git a/android/jni/com/mapswithme/maps/Framework.cpp b/android/jni/com/mapswithme/maps/Framework.cpp index fef4885bc4..8da8af481d 100644 --- a/android/jni/com/mapswithme/maps/Framework.cpp +++ b/android/jni/com/mapswithme/maps/Framework.cpp @@ -302,11 +302,11 @@ namespace android void Framework::StartTouchTask(double x, double y, unsigned ms) { KillTouchTask(); - m_scheduledTask.reset(new ScheduledTask( + m_deferredTask.reset(new DeferredTask( bind(&android::Framework::OnProcessTouchTask, this, x, y, ms), milliseconds(ms))); } - void Framework::KillTouchTask() { m_scheduledTask.reset(); } + void Framework::KillTouchTask() { m_deferredTask.reset(); } /// @param[in] mask Active pointers bits : 0x0 - no, 0x1 - (x1, y1), 0x2 - (x2, y2), 0x3 - (x1, y1)(x2, y2). void Framework::Touch(int action, int mask, double x1, double y1, double x2, double y2) diff --git a/android/jni/com/mapswithme/maps/Framework.hpp b/android/jni/com/mapswithme/maps/Framework.hpp index d9c24e971c..505e56bc37 100644 --- a/android/jni/com/mapswithme/maps/Framework.hpp +++ b/android/jni/com/mapswithme/maps/Framework.hpp @@ -8,9 +8,9 @@ #include "../../../../../geometry/avg_vector.hpp" -#include "../../../../../base/timer.hpp" -#include "../../../../../base/scheduled_task.hpp" +#include "../../../../../base/deferred_task.hpp" #include "../../../../../base/strings_bundle.hpp" +#include "../../../../../base/timer.hpp" #include "../../../../../indexer/map_style.hpp" @@ -61,7 +61,7 @@ namespace android math::LowPassVector m_sensors[2]; double m_lastCompass; - unique_ptr m_scheduledTask; + unique_ptr m_deferredTask; bool m_wasLongClick; int m_densityDpi; diff --git a/base/base.pro b/base/base.pro index 288cc5962c..42e71abc2b 100644 --- a/base/base.pro +++ b/base/base.pro @@ -11,6 +11,7 @@ SOURCES += \ base.cpp \ commands_queue.cpp \ condition.cpp \ + deferred_task.cpp \ exception.cpp \ fence_manager.cpp \ internal/message.cpp \ @@ -21,7 +22,6 @@ SOURCES += \ pseudo_random.cpp \ resource_pool.cpp \ runner.cpp \ - scheduled_task.cpp \ shared_buffer_manager.cpp \ src_point.cpp \ string_format.cpp \ @@ -45,6 +45,7 @@ HEADERS += \ commands_queue.hpp \ condition.hpp \ const_helper.hpp \ + deferred_task.hpp \ exception.hpp \ fence_manager.hpp \ internal/message.hpp \ @@ -63,7 +64,6 @@ HEADERS += \ resource_pool.hpp \ rolling_hash.hpp \ runner.hpp \ - scheduled_task.hpp \ scope_guard.hpp \ set_operations.hpp \ shared_buffer_manager.hpp \ diff --git a/base/base_tests/base_tests.pro b/base/base_tests/base_tests.pro index 6b2bcb923a..cd08df0505 100644 --- a/base/base_tests/base_tests.pro +++ b/base/base_tests/base_tests.pro @@ -22,6 +22,7 @@ SOURCES += \ condition_test.cpp \ const_helper.cpp \ containers_test.cpp \ + deferred_task_test.cpp \ fence_manager_test.cpp \ logging_test.cpp \ math_test.cpp \ @@ -31,7 +32,6 @@ SOURCES += \ observer_list_test.cpp \ regexp_test.cpp \ rolling_hash_test.cpp \ - scheduled_task_test.cpp \ scope_guard_test.cpp \ stl_add_test.cpp \ string_format_test.cpp \ diff --git a/base/base_tests/scheduled_task_test.cpp b/base/base_tests/deferred_task_test.cpp similarity index 69% rename from base/base_tests/scheduled_task_test.cpp rename to base/base_tests/deferred_task_test.cpp index 352f0c8225..1ddb06d333 100644 --- a/base/base_tests/scheduled_task_test.cpp +++ b/base/base_tests/deferred_task_test.cpp @@ -1,13 +1,16 @@ #include "testing/testing.hpp" -#include "base/scheduled_task.hpp" +#include "base/deferred_task.hpp" #include "std/atomic.hpp" #include "std/bind.hpp" namespace { -milliseconds const kTimeInaccuracy(1); +steady_clock::duration kSteadyClockResolution(1); + +milliseconds const kTimeInaccuracy = + std::max(milliseconds(1), duration_cast(kSteadyClockResolution)); void AddInt(atomic & value, int a) { value += a; } @@ -19,18 +22,18 @@ void MulInt(atomic & value, int m) } } // namespace -// ScheduledTask start (stop) is a memory barrier because it starts +// DeferredTask start (stop) is a memory barrier because it starts // (stops) a thread. That's why it's ok to create time points before -// ScheduledTask creation and after ScheduledTask completion. Also, +// DeferredTask creation and after DeferredTask completion. Also, // we're assuming that steady_clocks are consistent between CPU cores. -UNIT_TEST(ScheduledTask_SimpleAdd) +UNIT_TEST(DeferredTask_SimpleAdd) { steady_clock::time_point const start = steady_clock::now(); milliseconds const delay(1000); atomic value(0); - ScheduledTask task1(bind(&AddInt, ref(value), 1), delay); - ScheduledTask task2(bind(&AddInt, ref(value), 2), delay); + DeferredTask task1(bind(&AddInt, ref(value), 1), delay); + DeferredTask task2(bind(&AddInt, ref(value), 2), delay); task1.WaitForCompletion(); task2.WaitForCompletion(); TEST_EQUAL(value, 3, ()); @@ -40,14 +43,14 @@ UNIT_TEST(ScheduledTask_SimpleAdd) TEST(elapsed >= delay - kTimeInaccuracy, (elapsed.count(), delay.count())); } -UNIT_TEST(ScheduledTask_SimpleMul) +UNIT_TEST(DeferredTask_SimpleMul) { steady_clock::time_point const start = steady_clock::now(); milliseconds const delay(1500); atomic value(1); - ScheduledTask task1(bind(&MulInt, ref(value), 2), delay); - ScheduledTask task2(bind(&MulInt, ref(value), 3), delay); + DeferredTask task1(bind(&MulInt, ref(value), 2), delay); + DeferredTask task2(bind(&MulInt, ref(value), 3), delay); task1.WaitForCompletion(); task2.WaitForCompletion(); TEST_EQUAL(value, 6, ()); @@ -57,15 +60,15 @@ UNIT_TEST(ScheduledTask_SimpleMul) TEST(elapsed >= delay - kTimeInaccuracy, (elapsed.count(), delay.count())); } -UNIT_TEST(ScheduledTask_CancelNoBlocking) +UNIT_TEST(DeferredTask_CancelNoBlocking) { steady_clock::time_point const start = steady_clock::now(); milliseconds const delay(1500); atomic value(0); - ScheduledTask task(bind(&AddInt, ref(value), 1), delay); + DeferredTask task(bind(&AddInt, ref(value), 1), delay); - task.CancelNoBlocking(); + task.Cancel(); task.WaitForCompletion(); if (task.WasStarted()) diff --git a/base/scheduled_task.cpp b/base/deferred_task.cpp similarity index 63% rename from base/scheduled_task.cpp rename to base/deferred_task.cpp index 6ad2605fe3..fdc396dfbd 100644 --- a/base/scheduled_task.cpp +++ b/base/deferred_task.cpp @@ -1,4 +1,4 @@ -#include "base/scheduled_task.hpp" +#include "base/deferred_task.hpp" #include "base/timer.hpp" #include "base/logging.hpp" @@ -6,12 +6,12 @@ #include "std/algorithm.hpp" #include "std/mutex.hpp" -ScheduledTask::Routine::Routine(fn_t const & fn, milliseconds delay, atomic & started) - : m_fn(fn), m_delay(delay), m_started(started) +DeferredTask::Routine::Routine(TTask const & task, milliseconds delay, atomic & started) + : m_task(task), m_delay(delay), m_started(started) { } -void ScheduledTask::Routine::Do() +void DeferredTask::Routine::Do() { mutex mu; unique_lock lock(mu); @@ -31,34 +31,34 @@ void ScheduledTask::Routine::Do() if (!IsCancelled()) { m_started = true; - m_fn(); + m_task(); } } -void ScheduledTask::Routine::Cancel() +void DeferredTask::Routine::Cancel() { threads::IRoutine::Cancel(); m_cv.notify_one(); } -ScheduledTask::ScheduledTask(fn_t const & fn, milliseconds ms) : m_started(false) +DeferredTask::DeferredTask(TTask const & task, milliseconds ms) : m_started(false) { - m_thread.Create(make_unique(fn, ms, m_started)); + m_thread.Create(make_unique(task, ms, m_started)); } -ScheduledTask::~ScheduledTask() +DeferredTask::~DeferredTask() { CHECK(m_threadChecker.CalledOnOriginalThread(), ()); m_thread.Cancel(); } -bool ScheduledTask::WasStarted() const +bool DeferredTask::WasStarted() const { CHECK(m_threadChecker.CalledOnOriginalThread(), ()); return m_started; } -void ScheduledTask::CancelNoBlocking() +void DeferredTask::Cancel() { CHECK(m_threadChecker.CalledOnOriginalThread(), ()); threads::IRoutine * routine = m_thread.GetRoutine(); @@ -66,7 +66,7 @@ void ScheduledTask::CancelNoBlocking() routine->Cancel(); } -void ScheduledTask::WaitForCompletion() +void DeferredTask::WaitForCompletion() { CHECK(m_threadChecker.CalledOnOriginalThread(), ()); m_thread.Join(); diff --git a/base/scheduled_task.hpp b/base/deferred_task.hpp similarity index 66% rename from base/scheduled_task.hpp rename to base/deferred_task.hpp index 91a070db2a..ecbc3b4c45 100644 --- a/base/scheduled_task.hpp +++ b/base/deferred_task.hpp @@ -1,6 +1,7 @@ #pragma once #include "base/condition.hpp" +#include "base/macros.hpp" #include "base/thread.hpp" #include "base/thread_checker.hpp" @@ -9,21 +10,37 @@ #include "std/function.hpp" #include "std/unique_ptr.hpp" -/// Class, which performs any function when the specified amount of -/// time is elapsed. This class is not thread safe. -class ScheduledTask +// This class is used to call a function after waiting for a specified +// amount of time. The function is called in a separate thread. This +// class is not thread safe. +class DeferredTask { - typedef function fn_t; +public: + typedef function TTask; + DeferredTask(TTask const & task, milliseconds ms); + + ~DeferredTask(); + + /// Returns true if task was started after delay. + bool WasStarted() const; + + /// Cancels task without waiting for worker thread termination. + void Cancel(); + + /// Waits for task's completion and worker thread termination. + void WaitForCompletion(); + +private: class Routine : public threads::IRoutine { - fn_t const m_fn; + TTask const m_task; milliseconds const m_delay; condition_variable m_cv; atomic & m_started; public: - Routine(fn_t const & fn, milliseconds delay, atomic & started); + Routine(TTask const & task, milliseconds delay, atomic & started); // IRoutine overrides: void Do() override; @@ -36,20 +53,7 @@ class ScheduledTask /// is used by routine that will be executed on m_thread. atomic m_started; threads::Thread m_thread; - ThreadChecker m_threadChecker; -public: - ScheduledTask(fn_t const & fn, milliseconds ms); - - ~ScheduledTask(); - - /// Returns true if task was started after delay. - bool WasStarted() const; - - /// Cancels task without waiting for worker thread termination. - void CancelNoBlocking(); - - /// Waits for task's completion and worker thread termination. - void WaitForCompletion(); + DISALLOW_COPY_AND_MOVE(DeferredTask); }; diff --git a/qt/draw_widget.cpp b/qt/draw_widget.cpp index 1562025429..6a7f94772d 100644 --- a/qt/draw_widget.cpp +++ b/qt/draw_widget.cpp @@ -1,4 +1,5 @@ #include "qt/draw_widget.hpp" + #include "qt/slider_ctrl.hpp" #include "map/country_status_display.hpp" @@ -13,24 +14,23 @@ #include "graphics/opengl/opengl.hpp" #include "graphics/depth_constants.hpp" -#include "platform/settings.hpp" #include "platform/platform.hpp" +#include "platform/settings.hpp" #include "std/chrono.hpp" -#include - -#include #include +#include +#include #if QT_VERSION < QT_VERSION_CHECK(5, 0, 0) - #include #include #include + #include #else - #include #include #include + #include #endif namespace qt @@ -345,11 +345,11 @@ namespace qt void DrawWidget::StartPressTask(m2::PointD const & pt, unsigned ms) { KillPressTask(); - m_scheduledTask.reset( - new ScheduledTask(bind(&DrawWidget::OnPressTaskEvent, this, pt, ms), milliseconds(ms))); + m_deferredTask.reset( + new DeferredTask(bind(&DrawWidget::OnPressTaskEvent, this, pt, ms), milliseconds(ms))); } - void DrawWidget::KillPressTask() { m_scheduledTask.reset(); } + void DrawWidget::KillPressTask() { m_deferredTask.reset(); } void DrawWidget::OnPressTaskEvent(m2::PointD const & pt, unsigned ms) { diff --git a/qt/draw_widget.hpp b/qt/draw_widget.hpp index 7794d74932..5287025aac 100644 --- a/qt/draw_widget.hpp +++ b/qt/draw_widget.hpp @@ -6,10 +6,10 @@ #include "render/window_handle.hpp" -#include "base/scheduled_task.hpp" - #include "platform/video_timer.hpp" +#include "base/deferred_task.hpp" + #include "std/unique_ptr.hpp" #include @@ -145,7 +145,7 @@ namespace qt QScaleSlider * m_pScale; - unique_ptr m_scheduledTask; + unique_ptr m_deferredTask; m2::PointD m_taskPoint; bool m_wasLongClick, m_isCleanSingleClick;