> On May 27, 2014, 8:40 a.m., Gordon Sim wrote: > > /trunk/qpid/cpp/src/qpid/broker/Message.h, line 117 > > <https://reviews.apache.org/r/21153/diff/2/?file=594322#file594322line117> > > > > Rather than have a hasExpired() method that takes the tie to compare > > against, it might be simpler to remove this method entirely and just use > > getExpiration() which can then be compared against whatever value is > > desired.
hasExpired(Message, AbsTime) is required by Queue::purgeExpired: uint32_t count = remove(0, boost::bind(&Message::hasExpired, _1, time), 0, CONSUMER, settings.autodelete); Or is there a way how to boost::bind getExpiration() method there? The implicit setting time = sys::AbsTime::now() can be removed as that is used only in Queue::getNextMessage, but how to deal with the bind? - Pavel ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21153/#review43955 ----------------------------------------------------------- On May 26, 2014, 12:14 p.m., Pavel Moravec wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/21153/ > ----------------------------------------------------------- > > (Updated May 26, 2014, 12:14 p.m.) > > > Review request for qpid, Gordon Sim and mick goulish. > > > Bugs: QPID-5748 > https://issues.apache.org/jira/browse/QPID-5748 > > > Repository: qpid > > > Description > ------- > > Queue::purgeExpired method (to remove messages from all queues with TTL > expired) currently calls Message::hasExpired() method that calls > AbsTime::now() i.e. ::clock_gettime for every individual message with TTL set. > > That system call is redundant to be called in that way. It is enough to call > it once before traversing the very first queue and use the value as an > argument. > > The patch is not ideal: I would rather see just one function like: > > bool ExpiryPolicy::hasExpired(const Message& m, qpid::sys::AbsTime time = 0) > { > if time==0 time=sys::AbsTime::now(); > return m.getExpiration() < sys::AbsTime::now(); > } > > but not sure what value from AbsTime to be used instead of 0 (note that such > value should be get in as short time as possible). And I havent played with > AbsTime class and dont have time now to do so :-/ > > > Diffs > ----- > > /trunk/qpid/cpp/src/CMakeLists.txt 1597555 > /trunk/qpid/cpp/src/qpid/broker/Broker.h 1597555 > /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1597555 > /trunk/qpid/cpp/src/qpid/broker/Message.h 1597555 > /trunk/qpid/cpp/src/qpid/broker/Message.cpp 1597555 > /trunk/qpid/cpp/src/qpid/broker/PagedQueue.h 1597555 > /trunk/qpid/cpp/src/qpid/broker/PagedQueue.cpp 1597555 > /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1597555 > /trunk/qpid/cpp/src/qpid/broker/QueueFactory.cpp 1597555 > /trunk/qpid/cpp/src/qpid/broker/RecoverableMessage.h 1597555 > /trunk/qpid/cpp/src/qpid/broker/RecoverableMessageImpl.h 1597555 > /trunk/qpid/cpp/src/qpid/broker/RecoveryManagerImpl.cpp 1597555 > /trunk/qpid/cpp/src/qpid/broker/SessionState.cpp 1597555 > /trunk/qpid/cpp/src/qpid/broker/amqp/Incoming.h 1597555 > /trunk/qpid/cpp/src/qpid/broker/amqp/Incoming.cpp 1597555 > /trunk/qpid/cpp/src/qpid/legacystore/MessageStoreImpl.cpp 1597555 > /trunk/qpid/cpp/src/qpid/linearstore/MessageStoreImpl.cpp 1597555 > /trunk/qpid/cpp/src/qpid/management/ManagementAgent.cpp 1597555 > /trunk/qpid/cpp/src/tests/MessageUtils.h 1597555 > /trunk/qpid/cpp/src/tests/QueueTest.cpp 1597555 > > Diff: https://reviews.apache.org/r/21153/diff/ > > > Testing > ------- > > Attached patch improved: > - _traversing_ queues and _checking_ messages with TTL by approx. 10% > - with purging _durable_ expired messages, performance improvement is just 1% > (still better than nothing) > > No automated tests run so far (lack of time+HW). > > > Thanks, > > Pavel Moravec > >
