Yeah I agree. I just wonder if that loop was using equals and not comparing just the message id, maybe there was a purpose of the old code. But a git blame can maybe tell us more.
On Sat, Nov 28, 2015 at 5:34 AM, David Sitsky <david.sit...@gmail.com> wrote: > Done: https://issues.apache.org/jira/browse/AMQ-6066 > > If you can incorporate the patch in for 5.13.0 I'd be very grateful.. as it > is a pain for us to not use an official release. Also I believe this is a > really important performance regression that we'd want to stomp out quickly > for ActiveMQ.. > > Many thanks in advance. > > Cheers, > David > > On Sat, Nov 28, 2015 at 1:25 AM, Christopher Shannon < > christopher.l.shan...@gmail.com> wrote: > >> Claus is right, open up a Jira and I or someone else can take a look at >> this. I don't know if there will be enough time to put this in before >> 5.13.0 because I plan on starting the release Monday for that and I'd want >> to make sure all the tests run and there would be no unintended issues by >> making a change like this. >> >> However, even if this doesn't go in for 5.13.0, I would expect a bug fix >> release (5.13.1) to follow shortly in a month or two and it could be >> included in that. It would also be a candidate to be merged into a 5.12.2 >> release. >> >> On Fri, Nov 27, 2015 at 7:40 AM, Claus Ibsen <claus.ib...@gmail.com> >> wrote: >> >> > Hi >> > >> > Well spotted. I think a good idea is to log a JIRA ticket about this >> > so its not forgotten and so the AMQ team can look at it and get it >> > into the next release. >> > >> > On Fri, Nov 27, 2015 at 2:39 AM, David Sitsky <david.sit...@gmail.com> >> > wrote: >> > > FWIW I changed the contains method as follows: >> > > >> > > @Override >> > > public boolean contains(MessageReference message) { >> > > if (message != null) { >> > > return map.containsKey(message.getMessageId()); >> > > } >> > > return false; >> > > } >> > > >> > > I got a speedup for my test taking 29 minutes from 41 minutes. Can we >> > get >> > > this change in to the upcoming 5.13 release? >> > > >> > > On Thu, Nov 26, 2015 at 11:44 AM, David Sitsky <david.sit...@gmail.com >> > >> > > wrote: >> > > >> > >> Hi, >> > >> >> > >> I have updated my application from ActiveMQ 5.3 to 5.11.1 and have >> > noticed >> > >> a performance degregation issue. Running a number of jstacks I can >> see >> > the >> > >> broker is often stuck here: >> > >> >> > >> "Queue:master-items" Id=122 RUNNABLE >> > >> at >> > >> >> > >> org.apache.activemq.broker.region.cursors.OrderedPendingList.contains(OrderedPendingList.java:144) >> > >> at >> > >> >> > >> org.apache.activemq.broker.region.Queue.doPageInForDispatch(Queue.java:1930) >> > >> at >> > org.apache.activemq.broker.region.Queue.pageInMessages(Queue.java:2119) >> > >> at org.apache.activemq.broker.region.Queue.iterate(Queue.java:1596) >> > >> - locked java.lang.Object@253c3089 >> > >> at >> > >> >> > >> org.apache.activemq.thread.DedicatedTaskRunner.runTask(DedicatedTaskRunner.java:112) >> > >> at >> > >> >> > >> org.apache.activemq.thread.DedicatedTaskRunner$1.run(DedicatedTaskRunner.java:42) >> > >> >> > >> Number of locked synchronizers = 1 >> > >> - >> java.util.concurrent.locks.ReentrantReadWriteLock$NonfairSync@2eb46567 >> > >> >> > >> For this specific queue, there are a large number of items in it.. >> > around >> > >> 100,000. However I noticed the code for contains has: >> > >> >> > >> public boolean contains(MessageReference message) { >> > >> if (message != null) { >> > >> for (PendingNode value : map.values()) { >> > >> if (value.getMessage().equals(message)) { >> > >> return true; >> > >> } >> > >> } >> > >> } >> > >> return false; >> > >> } >> > >> >> > >> This will obviously be very slow. Given the Map is keyed by message >> ID, >> > >> can't we do a .contains(message.getMessageId()) instead? I noticed >> the >> > >> remove() method does this. I am not familiar with the internals of >> > >> ActiveMQ, so I don't know the ramifications of this. >> > >> >> > >> Cheers, >> > >> David >> > >> >> > >> > >> > >> > -- >> > Claus Ibsen >> > ----------------- >> > http://davsclaus.com @davsclaus >> > Camel in Action 2: https://www.manning.com/ibsen2 >> > >> -- Claus Ibsen ----------------- http://davsclaus.com @davsclaus Camel in Action 2: https://www.manning.com/ibsen2
