Thanks! On Sun, Nov 29, 2015 at 5:29 AM, Christopher Shannon < christopher.l.shan...@gmail.com> wrote:
> 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 > > >
