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 >