util: redesign scheduler::EventId
refs #3691
Change-Id: I5bc49e6a15ae7364c654ca61bd2cf0f95a5c173b
diff --git a/src/util/scheduler.cpp b/src/util/scheduler.cpp
index c2f5564..536061c 100644
--- a/src/util/scheduler.cpp
+++ b/src/util/scheduler.cpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/**
- * Copyright (c) 2013-2015 Regents of the University of California.
+ * Copyright (c) 2013-2016 Regents of the University of California.
*
* This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
*
@@ -27,176 +27,137 @@
namespace util {
namespace scheduler {
-struct EventIdImpl
+class EventInfo : noncopyable
{
- EventIdImpl(const Scheduler::EventQueue::iterator& event)
- : m_event(event)
- , m_isValid(true)
+public:
+ EventInfo(time::nanoseconds after, const EventCallback& callback)
+ : expireTime(time::steady_clock::now() + after)
+ , isExpired(false)
+ , callback(callback)
{
}
- void
- invalidate()
+ time::nanoseconds
+ expiresFromNow() const
{
- m_isValid = false;
+ return std::max(expireTime - time::steady_clock::now(), time::nanoseconds::zero());
}
- bool
- isValid() const
- {
- return m_isValid;
- }
-
- operator const Scheduler::EventQueue::iterator&() const
- {
- return m_event;
- }
-
- void
- reset(const Scheduler::EventQueue::iterator& newIterator)
- {
- m_event = newIterator;
- m_isValid = true;
- }
-
-private:
- Scheduler::EventQueue::iterator m_event;
- bool m_isValid;
+public:
+ time::steady_clock::TimePoint expireTime;
+ bool isExpired;
+ EventCallback callback;
+ EventQueue::const_iterator queueIt;
};
-Scheduler::EventInfo::EventInfo(const time::nanoseconds& after,
- const Event& event)
- : m_scheduledTime(time::steady_clock::now() + after)
- , m_event(event)
+bool
+EventId::operator!() const
{
+ return m_info.expired() || m_info.lock()->isExpired;
}
-Scheduler::EventInfo::EventInfo(const time::steady_clock::TimePoint& when,
- const EventInfo& previousEvent)
- : m_scheduledTime(when)
- , m_event(previousEvent.m_event)
- , m_eventId(previousEvent.m_eventId)
+bool
+EventId::operator==(const EventId& other) const
{
+ return (!(*this) && !other) ||
+ !(m_info.owner_before(other.m_info) || other.m_info.owner_before(m_info));
}
-time::nanoseconds
-Scheduler::EventInfo::expiresFromNow() const
+std::ostream&
+operator<<(std::ostream& os, const EventId& eventId)
{
- time::steady_clock::TimePoint now = time::steady_clock::now();
- if (now > m_scheduledTime)
- return time::seconds(0); // event should be scheduled ASAP
- else
- return m_scheduledTime - now;
+ return os << eventId.m_info.lock();
}
+bool
+EventQueueCompare::operator()(const shared_ptr<EventInfo>& a, const shared_ptr<EventInfo>& b) const
+{
+ return a->expireTime < b->expireTime;
+}
Scheduler::Scheduler(boost::asio::io_service& ioService)
- : m_scheduledEvent(m_events.end())
- , m_deadlineTimer(ioService)
+ : m_deadlineTimer(ioService)
, m_isEventExecuting(false)
{
}
EventId
-Scheduler::scheduleEvent(const time::nanoseconds& after,
- const Event& event)
+Scheduler::scheduleEvent(const time::nanoseconds& after, const EventCallback& callback)
{
- EventQueue::iterator i = m_events.insert(EventInfo(after, event));
+ BOOST_ASSERT(callback != nullptr);
- // On OSX 10.9, boost, and C++03 the following doesn't work without ndn::
- // because the argument-dependent lookup prefers STL to boost
- i->m_eventId = ndn::make_shared<EventIdImpl>(i);
+ EventQueue::iterator i = m_queue.insert(make_shared<EventInfo>(after, callback));
+ (*i)->queueIt = i;
- if (!m_isEventExecuting)
- {
- if (m_scheduledEvent == m_events.end() ||
- *i < *m_scheduledEvent)
- {
- m_deadlineTimer.expires_from_now(after);
- m_deadlineTimer.async_wait(bind(&Scheduler::onEvent, this, _1));
- m_scheduledEvent = i;
- }
- }
+ if (!m_isEventExecuting && i == m_queue.begin()) {
+ // the new event is the first one to expire
+ this->scheduleNext();
+ }
- return i->m_eventId;
+ return EventId(*i);
}
void
Scheduler::cancelEvent(const EventId& eventId)
{
- if (!static_cast<bool>(eventId) || !eventId->isValid())
- return; // event already fired or cancelled
-
- if (static_cast<EventQueue::iterator>(*eventId) != m_scheduledEvent) {
- m_events.erase(*eventId);
- eventId->invalidate();
- return;
+ shared_ptr<EventInfo> info = eventId.m_info.lock();
+ if (info == nullptr || info->isExpired) {
+ return; // event already expired or cancelled
}
- m_deadlineTimer.cancel();
- m_events.erase(static_cast<EventQueue::iterator>(*eventId));
- eventId->invalidate();
+ if (info->queueIt == m_queue.begin()) {
+ m_deadlineTimer.cancel();
+ }
+ m_queue.erase(info->queueIt);
- if (!m_isEventExecuting)
- {
- if (!m_events.empty())
- {
- m_deadlineTimer.expires_from_now(m_events.begin()->expiresFromNow());
- m_deadlineTimer.async_wait(bind(&Scheduler::onEvent, this, _1));
- m_scheduledEvent = m_events.begin();
- }
- else
- {
- m_scheduledEvent = m_events.end();
- }
- }
+ if (!m_isEventExecuting) {
+ this->scheduleNext();
+ }
}
void
Scheduler::cancelAllEvents()
{
- m_events.clear();
+ m_queue.clear();
m_deadlineTimer.cancel();
}
void
-Scheduler::onEvent(const boost::system::error_code& error)
+Scheduler::scheduleNext()
{
- if (error) // e.g., cancelled
- {
- return;
- }
+ if (!m_queue.empty()) {
+ m_deadlineTimer.expires_from_now((*m_queue.begin())->expiresFromNow());
+ m_deadlineTimer.async_wait(bind(&Scheduler::executeEvent, this, _1));
+ }
+}
+
+void
+Scheduler::executeEvent(const boost::system::error_code& error)
+{
+ if (error) { // e.g., cancelled
+ return;
+ }
m_isEventExecuting = true;
// process all expired events
time::steady_clock::TimePoint now = time::steady_clock::now();
- while(!m_events.empty() && m_events.begin()->m_scheduledTime <= now)
- {
- EventQueue::iterator head = m_events.begin();
-
- Event event = head->m_event;
- head->m_eventId->invalidate();
- m_events.erase(head);
-
- event();
+ while (!m_queue.empty()) {
+ EventQueue::iterator head = m_queue.begin();
+ shared_ptr<EventInfo> info = *head;
+ if (info->expireTime > now) {
+ break;
}
- if (!m_events.empty())
- {
- m_deadlineTimer.expires_from_now(m_events.begin()->m_scheduledTime - now);
- m_deadlineTimer.async_wait(bind(&Scheduler::onEvent, this, _1));
- m_scheduledEvent = m_events.begin();
- }
- else
- {
- m_scheduledEvent = m_events.end();
- }
+ m_queue.erase(head);
+ info->isExpired = true;
+ info->callback();
+ }
m_isEventExecuting = false;
+ this->scheduleNext();
}
-
} // namespace scheduler
} // namespace util
} // namespace ndn
diff --git a/src/util/scheduler.hpp b/src/util/scheduler.hpp
index 8948860..c908ec0 100644
--- a/src/util/scheduler.hpp
+++ b/src/util/scheduler.hpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/**
- * Copyright (c) 2013-2015 Regents of the University of California.
+ * Copyright (c) 2013-2016 Regents of the University of California.
*
* This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
*
@@ -31,31 +31,119 @@
namespace util {
namespace scheduler {
-struct EventIdImpl; ///< \brief Private storage of information about the event
/**
- * \brief Opaque type (shared_ptr) representing ID of the scheduled event
+ * \brief Function to be invoked when a scheduled event expires
*/
-typedef shared_ptr<EventIdImpl> EventId;
+typedef function<void()> EventCallback;
+
+/**
+ * \brief Stores internal information about a scheduled event
+ */
+class EventInfo;
+
+/**
+ * \brief Identifies a scheduled event
+ */
+class EventId
+{
+public:
+ /**
+ * \brief Constructs an empty EventId
+ * \note EventId is implicitly convertible from nullptr.
+ */
+ EventId(std::nullptr_t = nullptr)
+ {
+ }
+
+ /**
+ * \retval true The event is valid.
+ * \retval false This EventId is empty, or the event is expired or cancelled.
+ */
+ explicit
+ operator bool() const
+ {
+ return !this->operator!();
+ }
+
+ /**
+ * \retval true This EventId is empty, or the event is expired or cancelled.
+ * \retval false The event is valid.
+ */
+ bool
+ operator!() const;
+
+ /**
+ * \return whether this and other refer to the same event, or are both empty/expired/cancelled
+ */
+ bool
+ operator==(const EventId& other) const;
+
+ bool
+ operator!=(const EventId& other) const
+ {
+ return !this->operator==(other);
+ }
+
+ /**
+ * \brief clear this EventId
+ * \note This does not cancel the event.
+ * \post !(*this)
+ */
+ void
+ reset()
+ {
+ m_info.reset();
+ }
+
+private:
+ explicit
+ EventId(const weak_ptr<EventInfo>& info)
+ : m_info(info)
+ {
+ }
+
+private:
+ weak_ptr<EventInfo> m_info;
+
+ friend class Scheduler;
+ friend std::ostream& operator<<(std::ostream& os, const EventId& eventId);
+};
+
+std::ostream&
+operator<<(std::ostream& os, const EventId& eventId);
+
+class EventQueueCompare
+{
+public:
+ bool
+ operator()(const shared_ptr<EventInfo>& a, const shared_ptr<EventInfo>& b) const;
+};
+
+typedef std::multiset<shared_ptr<EventInfo>, EventQueueCompare> EventQueue;
/**
* \brief Generic scheduler
*/
-class Scheduler
+class Scheduler : noncopyable
{
public:
- typedef function<void()> Event;
+ /**
+ * \deprecated use EventCallback
+ */
+ typedef EventCallback Event;
+ explicit
Scheduler(boost::asio::io_service& ioService);
/**
- * \brief Schedule one time event after the specified delay
+ * \brief Schedule a one-time event after the specified delay
* \returns EventId that can be used to cancel the scheduled event
*/
EventId
- scheduleEvent(const time::nanoseconds& after, const Event& event);
+ scheduleEvent(const time::nanoseconds& after, const EventCallback& callback);
/**
- * \brief Cancel scheduled event
+ * \brief Cancel a scheduled event
*/
void
cancelEvent(const EventId& eventId);
@@ -67,43 +155,18 @@
cancelAllEvents();
private:
+ /**
+ * \brief Schedule the next event on the deadline timer
+ */
void
- onEvent(const boost::system::error_code& code);
+ scheduleNext();
+
+ void
+ executeEvent(const boost::system::error_code& code);
private:
- struct EventInfo
- {
- EventInfo(const time::nanoseconds& after, const Event& event);
-
- EventInfo(const time::steady_clock::TimePoint& when, const EventInfo& previousEvent);
-
- bool
- operator <=(const EventInfo& other) const
- {
- return this->m_scheduledTime <= other.m_scheduledTime;
- }
-
- bool
- operator <(const EventInfo& other) const
- {
- return this->m_scheduledTime < other.m_scheduledTime;
- }
-
- time::nanoseconds
- expiresFromNow() const;
-
- time::steady_clock::TimePoint m_scheduledTime;
- Event m_event;
- mutable EventId m_eventId;
- };
-
- typedef std::multiset<EventInfo> EventQueue;
- friend struct EventIdImpl;
-
- EventQueue m_events;
- EventQueue::iterator m_scheduledEvent;
monotonic_deadline_timer m_deadlineTimer;
-
+ EventQueue m_queue;
bool m_isEventExecuting;
};
diff --git a/tests/unit-tests/util/scheduler.t.cpp b/tests/unit-tests/util/scheduler.t.cpp
index 8e22f1e..dfc2826 100644
--- a/tests/unit-tests/util/scheduler.t.cpp
+++ b/tests/unit-tests/util/scheduler.t.cpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/**
- * Copyright (c) 2013-2015 Regents of the University of California.
+ * Copyright (c) 2013-2016 Regents of the University of California.
*
* This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
*
@@ -24,6 +24,7 @@
#include "boost-test.hpp"
#include "../unit-test-time-fixture.hpp"
+#include <boost/lexical_cast.hpp>
namespace ndn {
namespace util {
@@ -32,14 +33,26 @@
using namespace ndn::tests;
-BOOST_FIXTURE_TEST_SUITE(UtilScheduler, UnitTestTimeFixture)
+class SchedulerFixture : public UnitTestTimeFixture
+{
+public:
+ SchedulerFixture()
+ : scheduler(io)
+ {
+ }
+
+public:
+ Scheduler scheduler;
+};
+
+BOOST_AUTO_TEST_SUITE(Util)
+BOOST_FIXTURE_TEST_SUITE(TestScheduler, SchedulerFixture)
BOOST_AUTO_TEST_CASE(Events)
{
size_t count1 = 0;
size_t count2 = 0;
- Scheduler scheduler(io);
scheduler.scheduleEvent(time::milliseconds(500), [&] {
++count1;
BOOST_CHECK_EQUAL(count2, 1);
@@ -60,23 +73,19 @@
});
scheduler.cancelEvent(i);
- advanceClocks(time::milliseconds(1), 1000);
+ advanceClocks(time::milliseconds(25), time::milliseconds(1000));
BOOST_CHECK_EQUAL(count1, 1);
BOOST_CHECK_EQUAL(count2, 1);
}
BOOST_AUTO_TEST_CASE(CancelEmptyEvent)
{
- Scheduler scheduler(io);
-
EventId i;
scheduler.cancelEvent(i);
}
BOOST_AUTO_TEST_CASE(SelfCancel)
{
- Scheduler scheduler(io);
-
EventId selfEventId;
selfEventId = scheduler.scheduleEvent(time::milliseconds(100), [&] {
scheduler.cancelEvent(selfEventId);
@@ -85,12 +94,11 @@
BOOST_REQUIRE_NO_THROW(advanceClocks(time::milliseconds(100), 10));
}
-class SelfRescheduleFixture : public UnitTestTimeFixture
+class SelfRescheduleFixture : public SchedulerFixture
{
public:
SelfRescheduleFixture()
- : scheduler(io)
- , count(0)
+ : count(0)
{
}
@@ -133,7 +141,7 @@
scheduler.scheduleEvent(time::milliseconds(100), [&] { ++count; });
}
- Scheduler scheduler;
+public:
EventId selfEventId;
size_t count;
};
@@ -143,7 +151,7 @@
selfEventId = scheduler.scheduleEvent(time::seconds(0),
bind(&SelfRescheduleFixture::reschedule, this));
- BOOST_REQUIRE_NO_THROW(advanceClocks(time::milliseconds(10), 1000));
+ BOOST_REQUIRE_NO_THROW(advanceClocks(time::milliseconds(50), time::milliseconds(1000)));
BOOST_CHECK_EQUAL(count, 5);
}
@@ -153,7 +161,7 @@
selfEventId = scheduler.scheduleEvent(time::seconds(0),
bind(&SelfRescheduleFixture::reschedule2, this));
- BOOST_REQUIRE_NO_THROW(advanceClocks(time::milliseconds(10), 1000));
+ BOOST_REQUIRE_NO_THROW(advanceClocks(time::milliseconds(50), time::milliseconds(1000)));
BOOST_CHECK_EQUAL(count, 5);
}
@@ -163,17 +171,16 @@
selfEventId = scheduler.scheduleEvent(time::seconds(0),
bind(&SelfRescheduleFixture::reschedule3, this));
- BOOST_REQUIRE_NO_THROW(advanceClocks(time::milliseconds(10), 1000));
+ BOOST_REQUIRE_NO_THROW(advanceClocks(time::milliseconds(50), time::milliseconds(1000)));
BOOST_CHECK_EQUAL(count, 6);
}
-
-struct CancelAllFixture : public ::ndn::tests::UnitTestTimeFixture
+class CancelAllFixture : public SchedulerFixture
{
+public:
CancelAllFixture()
- : scheduler(io)
- , count(0)
+ : count(0)
{
}
@@ -185,11 +192,10 @@
scheduler.scheduleEvent(time::seconds(1), [&] { event(); });
}
- Scheduler scheduler;
+public:
uint32_t count;
};
-
BOOST_FIXTURE_TEST_CASE(CancelAll, CancelAllFixture)
{
scheduler.scheduleEvent(time::milliseconds(500), [&] { scheduler.cancelAllEvents(); });
@@ -205,21 +211,123 @@
BOOST_CHECK_EQUAL(count, 0);
}
-class ScopedEventFixture : public UnitTestTimeFixture
+BOOST_AUTO_TEST_CASE(CancelAllWithScopedEventId) // Bug 3691
{
-public:
- ScopedEventFixture()
- : scheduler(io)
- {
- }
+ Scheduler sched(io);
+ ScopedEventId eid(sched);
+ eid = sched.scheduleEvent(time::milliseconds(10), []{});
+ sched.cancelAllEvents();
+ eid.cancel(); // should not crash
+}
-public:
- Scheduler scheduler;
-};
+BOOST_AUTO_TEST_SUITE(EventId)
-BOOST_FIXTURE_TEST_SUITE(ScopedEvents, ScopedEventFixture)
+using scheduler::EventId;
-BOOST_AUTO_TEST_CASE(ScopedEventIdDestruct)
+BOOST_AUTO_TEST_CASE(ConstructEmpty)
+{
+ EventId eid;
+ eid = nullptr;
+}
+
+BOOST_AUTO_TEST_CASE(Compare)
+{
+ EventId eid, eid2;
+ BOOST_CHECK_EQUAL(eid == eid2, true);
+ BOOST_CHECK_EQUAL(eid != eid2, false);
+ BOOST_CHECK_EQUAL(eid == nullptr, true);
+ BOOST_CHECK_EQUAL(eid != nullptr, false);
+
+ eid = scheduler.scheduleEvent(time::milliseconds(10), []{});
+ BOOST_CHECK_EQUAL(eid == eid2, false);
+ BOOST_CHECK_EQUAL(eid != eid2, true);
+ BOOST_CHECK_EQUAL(eid == nullptr, false);
+ BOOST_CHECK_EQUAL(eid != nullptr, true);
+
+ eid2 = eid;
+ BOOST_CHECK_EQUAL(eid == eid2, true);
+ BOOST_CHECK_EQUAL(eid != eid2, false);
+
+ eid2 = scheduler.scheduleEvent(time::milliseconds(10), []{});
+ BOOST_CHECK_EQUAL(eid == eid2, false);
+ BOOST_CHECK_EQUAL(eid != eid2, true);
+}
+
+BOOST_AUTO_TEST_CASE(Valid)
+{
+ EventId eid;
+ BOOST_CHECK_EQUAL(static_cast<bool>(eid), false);
+ BOOST_CHECK_EQUAL(!eid, true);
+
+ eid = scheduler.scheduleEvent(time::milliseconds(10), []{});
+ BOOST_CHECK_EQUAL(static_cast<bool>(eid), true);
+ BOOST_CHECK_EQUAL(!eid, false);
+
+ EventId eid2 = eid;
+ scheduler.cancelEvent(eid2);
+ BOOST_CHECK_EQUAL(static_cast<bool>(eid), false);
+ BOOST_CHECK_EQUAL(!eid, true);
+ BOOST_CHECK_EQUAL(static_cast<bool>(eid2), false);
+ BOOST_CHECK_EQUAL(!eid2, true);
+}
+
+BOOST_AUTO_TEST_CASE(DuringCallback)
+{
+ EventId eid;
+ EventId eid2 = scheduler.scheduleEvent(time::milliseconds(20), []{});
+
+ bool isCallbackInvoked = false;
+ eid = scheduler.scheduleEvent(time::milliseconds(10), [this, &eid, &eid2, &isCallbackInvoked] {
+ isCallbackInvoked = true;
+
+ // eid is "expired" during callback execution
+ BOOST_CHECK_EQUAL(static_cast<bool>(eid), false);
+ BOOST_CHECK_EQUAL(!eid, true);
+ BOOST_CHECK_EQUAL(eid == eid2, false);
+ BOOST_CHECK_EQUAL(eid != eid2, true);
+ BOOST_CHECK_EQUAL(eid == nullptr, true);
+ BOOST_CHECK_EQUAL(eid != nullptr, false);
+
+ scheduler.cancelEvent(eid2);
+ BOOST_CHECK_EQUAL(eid == eid2, true);
+ BOOST_CHECK_EQUAL(eid != eid2, false);
+ });
+ this->advanceClocks(time::milliseconds(6), 2);
+ BOOST_CHECK(isCallbackInvoked);
+}
+
+BOOST_AUTO_TEST_CASE(Reset)
+{
+ bool isCallbackInvoked = false;
+ EventId eid = scheduler.scheduleEvent(time::milliseconds(10),
+ [&isCallbackInvoked]{ isCallbackInvoked = true; });
+ eid.reset();
+ BOOST_CHECK_EQUAL(!eid, true);
+ BOOST_CHECK_EQUAL(eid == nullptr, true);
+
+ this->advanceClocks(time::milliseconds(6), 2);
+ BOOST_CHECK(isCallbackInvoked);
+}
+
+BOOST_AUTO_TEST_CASE(ToString)
+{
+ std::string nullString = boost::lexical_cast<std::string>(shared_ptr<int>());
+
+ EventId eid;
+ BOOST_CHECK_EQUAL(boost::lexical_cast<std::string>(eid), nullString);
+
+ eid = scheduler.scheduleEvent(time::milliseconds(10), []{});
+ BOOST_TEST_MESSAGE("eid=" << eid);
+ BOOST_CHECK_NE(boost::lexical_cast<std::string>(eid), nullString);
+}
+
+BOOST_AUTO_TEST_SUITE_END() // EventId
+
+BOOST_AUTO_TEST_SUITE(ScopedEventId)
+
+using scheduler::ScopedEventId;
+
+BOOST_AUTO_TEST_CASE(Destruct)
{
int hit = 0;
{
@@ -230,7 +338,7 @@
BOOST_CHECK_EQUAL(hit, 0);
}
-BOOST_AUTO_TEST_CASE(ScopedEventIdAssign)
+BOOST_AUTO_TEST_CASE(Assign)
{
int hit1 = 0, hit2 = 0;
ScopedEventId se1(scheduler);
@@ -241,7 +349,7 @@
BOOST_CHECK_EQUAL(hit2, 1);
}
-BOOST_AUTO_TEST_CASE(ScopedEventIdRelease)
+BOOST_AUTO_TEST_CASE(Release)
{
int hit = 0;
{
@@ -253,7 +361,7 @@
BOOST_CHECK_EQUAL(hit, 1);
}
-BOOST_AUTO_TEST_CASE(ScopedEventIdMove)
+BOOST_AUTO_TEST_CASE(Move)
{
int hit = 0;
unique_ptr<scheduler::ScopedEventId> se2;
@@ -268,7 +376,8 @@
BOOST_AUTO_TEST_SUITE_END() // ScopedEventId
-BOOST_AUTO_TEST_SUITE_END() // UtilTestScheduler
+BOOST_AUTO_TEST_SUITE_END() // TestScheduler
+BOOST_AUTO_TEST_SUITE_END() // Util
} // namespace tests
} // namespace scheduler