----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10839/#review19858 -----------------------------------------------------------
I don't see any bugs but it is more complicated than I'd like. Would it be simpler to keep a map of thread safe dequeues by destination? As well as simplifying the locking logic it would get rid of the liner search that is there now. So you'd have a lock for the overall IncomingMessages protecting the map and a lock per destination for threads waiting to push or pull on that destination. /trunk/qpid/cpp/src/qpid/client/amqp0_10/IncomingMessages.cpp <https://reviews.apache.org/r/10839/#comment40952> Is this valid if timeout==INFINITE? /trunk/qpid/cpp/src/tests/MessagingSessionTests.cpp <https://reviews.apache.org/r/10839/#comment40954> Do you have performance test results that verify we fetch faster in multiple threads? - Alan Conway On April 29, 2013, 2:18 p.m., Gordon Sim wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/10839/ > ----------------------------------------------------------- > > (Updated April 29, 2013, 2:18 p.m.) > > > Review request for qpid, Alan Conway and Kenneth Giusti. > > > Description > ------- > > Only one thread at a time now pops off the underlying incoming session queue. > Other threads either wait for that to complete or for it to queue up some > received messages that it was not interested in. (Note: this could be made > more efficient where either you have a very large number of concurrent > fetches, or where there are a large number of unreceived messages that are > not of interest to any of the fetching threads. However this seemed to be > less of a concern at present, so I've left further optimisation as a separate > task for the future). > > > This addresses bug QPID-4786. > https://issues.apache.org/jira/browse/QPID-4786 > > > Diffs > ----- > > /trunk/qpid/cpp/src/qpid/client/amqp0_10/IncomingMessages.h 1475717 > /trunk/qpid/cpp/src/qpid/client/amqp0_10/IncomingMessages.cpp 1475717 > /trunk/qpid/cpp/src/tests/MessagingSessionTests.cpp 1475717 > > Diff: https://reviews.apache.org/r/10839/diff/ > > > Testing > ------- > > New test added; make test passes. > > > Thanks, > > Gordon Sim > >
