I have merged this into master so it will go into 5.13.0 on Monday. I will let Jenkins run our builds today to catch any problems but I don't expect any issues.
On Sat, Nov 28, 2015 at 2:25 AM, Claus Ibsen <claus.ib...@gmail.com> wrote: > 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 >
