activemq-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Christopher Shannon <christopher.l.shan...@gmail.com>
Subject Re: Performance issue in OrderedPendingList
Date Sat, 28 Nov 2015 18:29:48 GMT
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.ibsen@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.sitsky@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.shannon@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.ibsen@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.sitsky@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.sitsky@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
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message