On Fri, Aug 23, 2013 at 12:17 AM, James Peach <jpe...@apache.org> wrote:
> On Aug 22, 2013, at 5:01 AM, yun...@apache.org wrote: > > > Updated Branches: > > refs/heads/master e5d27294b -> fa176442f > > > > > > TS-2137: Use relative timeout in EventNotify::timedwait() > > > > 1) Use relative timeout in EventNotify::timedwait() function. > > 2) Remove unused m_name variable. > > 3) Use the correct marco HAVE_EVENTFD instead of TS_HAS_EVENTFD which > > is outdated. > > Good catch! > > > > > Signed-off-by: Yunkai Zhang <qiushu....@taobao.com> > > > > > > Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo > > Commit: > http://git-wip-us.apache.org/repos/asf/trafficserver/commit/fa176442 > > Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/fa176442 > > Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/fa176442 > > > > Branch: refs/heads/master > > Commit: fa176442f45a3d5e8a9dce332775172192f53f34 > > Parents: e5d2729 > > Author: Yunkai Zhang <qiushu....@taobao.com> > > Authored: Thu Aug 22 16:13:36 2013 +0800 > > Committer: Yunkai Zhang <qiushu....@taobao.com> > > Committed: Thu Aug 22 19:56:02 2013 +0800 > > > > ---------------------------------------------------------------------- > > lib/ts/EventNotify.cc | 39 +++++++++++++++++++-------------------- > > lib/ts/EventNotify.h | 7 +++---- > > 2 files changed, 22 insertions(+), 24 deletions(-) > > ---------------------------------------------------------------------- > > > > > > > http://git-wip-us.apache.org/repos/asf/trafficserver/blob/fa176442/lib/ts/EventNotify.cc > > ---------------------------------------------------------------------- > > diff --git a/lib/ts/EventNotify.cc b/lib/ts/EventNotify.cc > > index 5d0cc53..3a499b6 100644 > > --- a/lib/ts/EventNotify.cc > > +++ b/lib/ts/EventNotify.cc > > @@ -30,14 +30,14 @@ > > #include "EventNotify.h" > > #include "ink_hrtime.h" > > > > -#ifdef TS_HAS_EVENTFD > > +#ifdef HAVE_EVENTFD > > #include <sys/eventfd.h> > > #include <sys/epoll.h> > > #endif > > > > -EventNotify::EventNotify(const char *name): m_name(name) > > +EventNotify::EventNotify() > > { > > -#ifdef TS_HAS_EVENTFD > > +#ifdef HAVE_EVENTFD > > int ret; > > struct epoll_event ev; > > > > @@ -59,14 +59,14 @@ EventNotify::EventNotify(const char *name): > m_name(name) > > ink_release_assert(ret != -1); > > #else > > ink_cond_init(&m_cond); > > - ink_mutex_init(&m_mutex, m_name); > > + ink_mutex_init(&m_mutex, NULL); > > #endif > > } > > > > void > > EventNotify::signal(void) > > { > > -#ifdef TS_HAS_EVENTFD > > +#ifdef HAVE_EVENTFD > > ssize_t nr; > > uint64_t value = 1; > > nr = write(m_event_fd, &value, sizeof(uint64_t)); > > @@ -79,7 +79,7 @@ EventNotify::signal(void) > > void > > EventNotify::wait(void) > > { > > -#ifdef TS_HAS_EVENTFD > > +#ifdef HAVE_EVENTFD > > ssize_t nr; > > uint64_t value = 0; > > nr = read(m_event_fd, &value, sizeof(uint64_t)); > > @@ -90,20 +90,13 @@ EventNotify::wait(void) > > } > > > > int > > -EventNotify::timedwait(ink_timestruc *abstime) > > +EventNotify::timedwait(int timeout) // milliseconds > > Unless -1 is a meaningful value, I'd prefer this was unsigned. > In some scenario, the upper layer store absolute time before calling timedwait, such as ProtectedQueue. By using int type, the upper layer can calculate timeout more simply: timeout = now - abstime; The upper layer needn't to care about whether the timeout is < 0. We will check it automatically inside the timedwait() function. I think use int type could be more user friendly. > > > { > > -#ifdef TS_HAS_EVENTFD > > - int timeout; > > +#ifdef HAVE_EVENTFD > > ssize_t nr, nr_fd = 0; > > uint64_t value = 0; > > - struct timeval curtime; > > struct epoll_event ev; > > > > - // Convert absolute time to relative time > > - gettimeofday(&curtime, NULL); > > - timeout = (abstime->tv_sec - curtime.tv_sec) * 1000 > > - + (abstime->tv_nsec / 1000 - curtime.tv_usec) / 1000; > > - > > // > > // When timeout < 0, epoll_wait() will wait indefinitely, but > > // pthread_cond_timedwait() will return ETIMEDOUT immediately. > > @@ -126,14 +119,20 @@ EventNotify::timedwait(ink_timestruc *abstime) > > > > return 0; > > #else > > - return ink_cond_timedwait(&m_cond, &m_mutex, abstime); > > + ink_timestruc abstime; > > + ink_hrtime curtime; > > + > > + curtime = ink_get_hrtime_internal() + timeout * HRTIME_MSECOND; > > + abstime = ink_based_hrtime_to_timespec(curtime); > > + > > + return ink_cond_timedwait(&m_cond, &m_mutex, &abstime); > > #endif > > } > > > > void > > EventNotify::lock(void) > > { > > -#ifdef TS_HAS_EVENTFD > > +#ifdef HAVE_EVENTFD > > // do nothing > > #else > > ink_mutex_acquire(&m_mutex); > > @@ -143,7 +142,7 @@ EventNotify::lock(void) > > bool > > EventNotify::trylock(void) > > { > > -#ifdef TS_HAS_EVENTFD > > +#ifdef HAVE_EVENTFD > > return true; > > #else > > return ink_mutex_try_acquire(&m_mutex); > > @@ -153,7 +152,7 @@ EventNotify::trylock(void) > > void > > EventNotify::unlock(void) > > { > > -#ifdef TS_HAS_EVENTFD > > +#ifdef HAVE_EVENTFD > > // do nothing > > #else > > ink_mutex_release(&m_mutex); > > @@ -162,7 +161,7 @@ EventNotify::unlock(void) > > > > EventNotify::~EventNotify() > > { > > -#ifdef TS_HAS_EVENTFD > > +#ifdef HAVE_EVENTFD > > close(m_event_fd); > > close(m_epoll_fd); > > #else > > > > > http://git-wip-us.apache.org/repos/asf/trafficserver/blob/fa176442/lib/ts/EventNotify.h > > ---------------------------------------------------------------------- > > diff --git a/lib/ts/EventNotify.h b/lib/ts/EventNotify.h > > index 16e4809..135f037 100644 > > --- a/lib/ts/EventNotify.h > > +++ b/lib/ts/EventNotify.h > > Please add header guards to this file. > I don't known what is header guards, can you give me an example? :D > > > @@ -33,18 +33,17 @@ > > class EventNotify > > { > > public: > > - EventNotify(const char *name = NULL); > > + EventNotify(); > > void signal(void); > > void wait(void); > > - int timedwait(ink_timestruc *abstime); > > + int timedwait(int timeout); // milliseconds > > void lock(void); > > bool trylock(void); > > void unlock(void); > > ~EventNotify(); > > > > private: > > - const char *m_name; > > -#ifdef TS_HAS_EVENTFD > > +#ifdef HAVE_EVENTFD > > int m_event_fd; > > int m_epoll_fd; > > #else > > > > -- Yunkai Zhang Work at Taobao
