----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50759/#review144765 -----------------------------------------------------------
This is good. Some small things that would make the code easier to follow (for me anyway). See couple of comments below. src/qpid/sys/epoll/EpollPoller.cpp (line 567) <https://reviews.apache.org/r/50759/#comment210870> formatting suggestion: Move line 571 to here AbsTime now_(now()); Even though it's not logically needed here. Because, then you can make this neater by formatting it as if (timeout ...) { timeoutMs = ... } else if (now_ ...) { timeoutMs = ... } else { ... timeoutMS = ... } Which I think is a clearer expression of the code. src/qpid/sys/epoll/EpollPoller.cpp (line 572) <https://reviews.apache.org/r/50759/#comment210867> Tiny quibble: Could be if (now_ >= targetTimeout) ... - Andrew Stitcher On Aug. 4, 2016, 3:32 a.m., Cliff Jansen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50759/ > ----------------------------------------------------------- > > (Updated Aug. 4, 2016, 3:32 a.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 > >
