You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@activemq.apache.org by David Sitsky <da...@gmail.com> on 2015/11/26 01:44:58 UTC
Performance issue in OrderedPendingList
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
Re: Performance issue in OrderedPendingList
Posted by David Sitsky <da...@gmail.com>.
Thanks!
On Sun, Nov 29, 2015 at 5:29 AM, Christopher Shannon <
christopher.l.shannon@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 <cl...@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 <da...@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 <cl...@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
> >
>
Re: Performance issue in OrderedPendingList
Posted by Christopher Shannon <ch...@gmail.com>.
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 <cl...@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 <da...@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 <cl...@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
>
Re: Performance issue in OrderedPendingList
Posted by Claus Ibsen <cl...@gmail.com>.
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 <da...@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 <cl...@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 <da...@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
Re: Performance issue in OrderedPendingList
Posted by David Sitsky <da...@gmail.com>.
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 <cl...@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 <da...@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
> >
>
Re: Performance issue in OrderedPendingList
Posted by Christopher Shannon <ch...@gmail.com>.
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 <cl...@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 <da...@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 <da...@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
>
Re: Performance issue in OrderedPendingList
Posted by Claus Ibsen <cl...@gmail.com>.
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 <da...@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 <da...@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
Re: Performance issue in OrderedPendingList
Posted by David Sitsky <da...@gmail.com>.
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 <da...@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
>