> On April 29, 2013, 3:02 p.m., Alan Conway wrote: > > 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.
The issue is that you need to keep both session order (for session::nextReceiver()) _and_ when requested process a specific receiver only. So if we kept a map it would need to be in addition to some other structure recording session order. It would also require a change in the interface to IncomingMessages (would need to signal closing of receivers and would need to pass in the destination to get), and therefore to SessionImpl also. I did consider doing that, but felt a fix with smaller impact was preferable at this stage. > On April 29, 2013, 3:02 p.m., Alan Conway wrote: > > /trunk/qpid/cpp/src/qpid/client/amqp0_10/IncomingMessages.cpp, line 135 > > <https://reviews.apache.org/r/10839/diff/1/?file=285725#file285725line135> > > > > Is this valid if timeout==INFINITE? Yes. (Annoyingly however you can't then get the correct Duration back via AbsTime(AbsTime::now(), deadline) - however thats a separate issue). > On April 29, 2013, 3:02 p.m., Alan Conway wrote: > > /trunk/qpid/cpp/src/tests/MessagingSessionTests.cpp, line 1234 > > <https://reviews.apache.org/r/10839/diff/1/?file=285726#file285726line1234> > > > > Do you have performance test results that verify we fetch faster in > > multiple threads? No. However to be clear the issue isn't one of performance, its that when you have concurrent fetches, they can incorrectly get blocked until the timeout. - Gordon ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10839/#review19858 ----------------------------------------------------------- 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 > >
