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
>

Reply via email to