> On Aug. 3, 2016, 6:33 p.m., Andrew Stitcher wrote: > > src/qpid/sys/epoll/EpollPoller.cpp, line 669 > > <https://reviews.apache.org/r/50759/diff/1/?file=1461569#file1461569line669> > > > > I think it would be easier to understand all the timer manipulations to > > put something like: > > > > timeoutMs = std::min(Duration(now(), targetTimeout)/TIME_MSEC, > > maxEpollWait); > > > > [Or you could make maxEpollWait a Duration which might be neater and do: > > > > timeoutMs = std::min(Duration(now(), targetTimeout), maxEpollWait) / > > TIME_MSEC; > > > > which I think should work because of the implicit conversion from > > Duration->int64_t.] > > > > at the top of the loop and remove the recalculation from here.
I tried the latter, but ran into testing problems. If Duration(now(), targetTimeout) is not a multiple of TIME_MSEC, the wakeup is just before the targetTimeout and the code does successive waits with timeoutMs=0 until the clock moves past targetTimeout. I also had some weird example where now() was past targetTimeout and caused a negative duration. I will load a newer version for review. - Cliff ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50759/#review144631 ----------------------------------------------------------- On Aug. 3, 2016, 5:43 p.m., Cliff Jansen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50759/ > ----------------------------------------------------------- > > (Updated Aug. 3, 2016, 5:43 p.m.) > > > Review request for qpid and Andrew Stitcher. > > > Bugs: qpid-7373 > https://issues.apache.org/jira/browse/qpid-7373 > > > Repository: qpid-cpp > > > Description > ------- > > If a C++ broker is lightly loaded with many short lived connections such that > at least one worker thread remains unwoken from the epoll_wait, its > DeletionManager::ThreadStatus::handles grows without bound and the associated > handles retain a shared_ptr ref and never go away. > > This patch allows IO threads a maximum of one minute to accumulate > DeletionManager shared pointer references before resuming the wait loop which > involves calling > > PollerHandleDeletionManager.markAllUnusedInThisThread(); > > The overhead is small on brokers light thread load and non-existent on busy > brokers. > > > Diffs > ----- > > src/qpid/sys/epoll/EpollPoller.cpp 6fdf996 > > Diff: https://reviews.apache.org/r/50759/diff/ > > > Testing > ------- > > Jira test case, PollerTest.cpp > > > Thanks, > > Cliff Jansen > >
