[jira] [Commented] (AMQ-6239) Performance issue in PrioritizedPendingListIterator
[ https://issues.apache.org/jira/browse/AMQ-6239?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15237394#comment-15237394 ] ASF subversion and git services commented on AMQ-6239: -- Commit c1b58d3373746eda525e6c3b3ab04eb747e9674b in activemq's branch refs/heads/master from [~tabish121] [ https://git-wip-us.apache.org/repos/asf?p=activemq.git;h=c1b58d3 ] https://issues.apache.org/jira/browse/AMQ-6239 Refactor the iterator implementation in the PrioritizedPendingList to not copy elements and instead use the level iterators. Add some additional tests. > Performance issue in PrioritizedPendingListIterator > --- > > Key: AMQ-6239 > URL: https://issues.apache.org/jira/browse/AMQ-6239 > Project: ActiveMQ > Issue Type: Improvement > Components: Broker >Affects Versions: 5.12.2 >Reporter: Martin Lichtin > Attachments: AMQ-6239-yourkit-1.jpg, PrioritizedPendingList.java.patch > > > Sending and consuming 5000 messages to/from a queue, one can see heavy CPU > use on the broker side (v 5.12.2). > Yourkit shows > PrioritizedPendingList$PrioritizedPendingListIterator. > as a hot spot method. It calls ArrayList.add(Object) around 12 mio times. > Situation is that FilePendingMessageCursor.isEmpty() iterates over in-memory > messages and therefore (as it is a prioritized queue) uses > PrioritizedPendingListIterator which uses OrderedPendingList.getAsList() > which overall turns out to be an expensive method as it converts the > self-managed linked list to a Java ArrayList and then this list is filled > into another ArrayList managed by PrioritizedPendingListIterator. > PrioritizedPendingListIterator could be improved to walk the priority lists > via OrderedPendingList iterators, as these are implemented efficiently. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (AMQ-6239) Performance issue in PrioritizedPendingListIterator
[ https://issues.apache.org/jira/browse/AMQ-6239?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15235766#comment-15235766 ] Martin Lichtin commented on AMQ-6239: - Ok, sounds good. Let me know in case there's anything I should still provide. > Performance issue in PrioritizedPendingListIterator > --- > > Key: AMQ-6239 > URL: https://issues.apache.org/jira/browse/AMQ-6239 > Project: ActiveMQ > Issue Type: Improvement > Components: Broker >Affects Versions: 5.12.2 >Reporter: Martin Lichtin > Attachments: AMQ-6239-yourkit-1.jpg, PrioritizedPendingList.java.patch > > > Sending and consuming 5000 messages to/from a queue, one can see heavy CPU > use on the broker side (v 5.12.2). > Yourkit shows > PrioritizedPendingList$PrioritizedPendingListIterator. > as a hot spot method. It calls ArrayList.add(Object) around 12 mio times. > Situation is that FilePendingMessageCursor.isEmpty() iterates over in-memory > messages and therefore (as it is a prioritized queue) uses > PrioritizedPendingListIterator which uses OrderedPendingList.getAsList() > which overall turns out to be an expensive method as it converts the > self-managed linked list to a Java ArrayList and then this list is filled > into another ArrayList managed by PrioritizedPendingListIterator. > PrioritizedPendingListIterator could be improved to walk the priority lists > via OrderedPendingList iterators, as these are implemented efficiently. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (AMQ-6239) Performance issue in PrioritizedPendingListIterator
[ https://issues.apache.org/jira/browse/AMQ-6239?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15235689#comment-15235689 ] Christopher L. Shannon commented on AMQ-6239: - I don't think we need to worry about the thread safety issue. As Tim Bish pointed out and as I alluded to, there is already external locking going on where the pending lists are used. If you look at the current implementations of all of the PendingList classes, none of them are thread safe on their own at all (they don't lock, use HashMap and not ConcurrentHashMap, etc). > Performance issue in PrioritizedPendingListIterator > --- > > Key: AMQ-6239 > URL: https://issues.apache.org/jira/browse/AMQ-6239 > Project: ActiveMQ > Issue Type: Improvement > Components: Broker >Affects Versions: 5.12.2 >Reporter: Martin Lichtin > Attachments: AMQ-6239-yourkit-1.jpg, PrioritizedPendingList.java.patch > > > Sending and consuming 5000 messages to/from a queue, one can see heavy CPU > use on the broker side (v 5.12.2). > Yourkit shows > PrioritizedPendingList$PrioritizedPendingListIterator. > as a hot spot method. It calls ArrayList.add(Object) around 12 mio times. > Situation is that FilePendingMessageCursor.isEmpty() iterates over in-memory > messages and therefore (as it is a prioritized queue) uses > PrioritizedPendingListIterator which uses OrderedPendingList.getAsList() > which overall turns out to be an expensive method as it converts the > self-managed linked list to a Java ArrayList and then this list is filled > into another ArrayList managed by PrioritizedPendingListIterator. > PrioritizedPendingListIterator could be improved to walk the priority lists > via OrderedPendingList iterators, as these are implemented efficiently. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (AMQ-6239) Performance issue in PrioritizedPendingListIterator
[ https://issues.apache.org/jira/browse/AMQ-6239?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15235643#comment-15235643 ] Martin Lichtin commented on AMQ-6239: - By the same logic, should we handle the situation of the underlying list getting extended concurrently? If so, then the "if (!orderedPendingList.isEmpty())" optimization (which was already there) in the ctor should be removed. I wonder if we are contemplating a use case that's not actually required.. > Performance issue in PrioritizedPendingListIterator > --- > > Key: AMQ-6239 > URL: https://issues.apache.org/jira/browse/AMQ-6239 > Project: ActiveMQ > Issue Type: Improvement > Components: Broker >Affects Versions: 5.12.2 >Reporter: Martin Lichtin > Attachments: AMQ-6239-yourkit-1.jpg, PrioritizedPendingList.java.patch > > > Sending and consuming 5000 messages to/from a queue, one can see heavy CPU > use on the broker side (v 5.12.2). > Yourkit shows > PrioritizedPendingList$PrioritizedPendingListIterator. > as a hot spot method. It calls ArrayList.add(Object) around 12 mio times. > Situation is that FilePendingMessageCursor.isEmpty() iterates over in-memory > messages and therefore (as it is a prioritized queue) uses > PrioritizedPendingListIterator which uses OrderedPendingList.getAsList() > which overall turns out to be an expensive method as it converts the > self-managed linked list to a Java ArrayList and then this list is filled > into another ArrayList managed by PrioritizedPendingListIterator. > PrioritizedPendingListIterator could be improved to walk the priority lists > via OrderedPendingList iterators, as these are implemented efficiently. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (AMQ-6239) Performance issue in PrioritizedPendingListIterator
[ https://issues.apache.org/jira/browse/AMQ-6239?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15235129#comment-15235129 ] Martin Lichtin commented on AMQ-6239: - Tim is right that the next() method has a in issue in case the underlying OrderedPendingList changes during iterating. > Performance issue in PrioritizedPendingListIterator > --- > > Key: AMQ-6239 > URL: https://issues.apache.org/jira/browse/AMQ-6239 > Project: ActiveMQ > Issue Type: Improvement > Components: Broker >Affects Versions: 5.12.2 >Reporter: Martin Lichtin > Attachments: AMQ-6239-yourkit-1.jpg, PrioritizedPendingList.java.patch > > > Sending and consuming 5000 messages to/from a queue, one can see heavy CPU > use on the broker side (v 5.12.2). > Yourkit shows > PrioritizedPendingList$PrioritizedPendingListIterator. > as a hot spot method. It calls ArrayList.add(Object) around 12 mio times. > Situation is that FilePendingMessageCursor.isEmpty() iterates over in-memory > messages and therefore (as it is a prioritized queue) uses > PrioritizedPendingListIterator which uses OrderedPendingList.getAsList() > which overall turns out to be an expensive method as it converts the > self-managed linked list to a Java ArrayList and then this list is filled > into another ArrayList managed by PrioritizedPendingListIterator. > PrioritizedPendingListIterator could be improved to walk the priority lists > via OrderedPendingList iterators, as these are implemented efficiently. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (AMQ-6239) Performance issue in PrioritizedPendingListIterator
[ https://issues.apache.org/jira/browse/AMQ-6239?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15235106#comment-15235106 ] Timothy Bish commented on AMQ-6239: --- As a point of reference the previous algorithm wasn't thread safe either, as concurrent calls to next could return the same node value under the right circumstances, so you could make some reasonable assumptions that the usage is done within proper locks. > Performance issue in PrioritizedPendingListIterator > --- > > Key: AMQ-6239 > URL: https://issues.apache.org/jira/browse/AMQ-6239 > Project: ActiveMQ > Issue Type: Improvement > Components: Broker >Affects Versions: 5.12.2 >Reporter: Martin Lichtin > Attachments: AMQ-6239-yourkit-1.jpg, PrioritizedPendingList.java.patch > > > Sending and consuming 5000 messages to/from a queue, one can see heavy CPU > use on the broker side (v 5.12.2). > Yourkit shows > PrioritizedPendingList$PrioritizedPendingListIterator. > as a hot spot method. It calls ArrayList.add(Object) around 12 mio times. > Situation is that FilePendingMessageCursor.isEmpty() iterates over in-memory > messages and therefore (as it is a prioritized queue) uses > PrioritizedPendingListIterator which uses OrderedPendingList.getAsList() > which overall turns out to be an expensive method as it converts the > self-managed linked list to a Java ArrayList and then this list is filled > into another ArrayList managed by PrioritizedPendingListIterator. > PrioritizedPendingListIterator could be improved to walk the priority lists > via OrderedPendingList iterators, as these are implemented efficiently. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (AMQ-6239) Performance issue in PrioritizedPendingListIterator
[ https://issues.apache.org/jira/browse/AMQ-6239?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15235073#comment-15235073 ] Christopher L. Shannon commented on AMQ-6239: - I'm not sure if thread safety is a big issue here without looking deeper into it. I believe every place the pending lists are used either have new lists allocated for that thread, or there is synchronization done at a higher level, such as the FilePendingMessageCursor which uses synchronized on adds and removes. > Performance issue in PrioritizedPendingListIterator > --- > > Key: AMQ-6239 > URL: https://issues.apache.org/jira/browse/AMQ-6239 > Project: ActiveMQ > Issue Type: Improvement > Components: Broker >Affects Versions: 5.12.2 >Reporter: Martin Lichtin > Attachments: AMQ-6239-yourkit-1.jpg, PrioritizedPendingList.java.patch > > > Sending and consuming 5000 messages to/from a queue, one can see heavy CPU > use on the broker side (v 5.12.2). > Yourkit shows > PrioritizedPendingList$PrioritizedPendingListIterator. > as a hot spot method. It calls ArrayList.add(Object) around 12 mio times. > Situation is that FilePendingMessageCursor.isEmpty() iterates over in-memory > messages and therefore (as it is a prioritized queue) uses > PrioritizedPendingListIterator which uses OrderedPendingList.getAsList() > which overall turns out to be an expensive method as it converts the > self-managed linked list to a Java ArrayList and then this list is filled > into another ArrayList managed by PrioritizedPendingListIterator. > PrioritizedPendingListIterator could be improved to walk the priority lists > via OrderedPendingList iterators, as these are implemented efficiently. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (AMQ-6239) Performance issue in PrioritizedPendingListIterator
[
https://issues.apache.org/jira/browse/AMQ-6239?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15235057#comment-15235057
]
Tim Bain commented on AMQ-6239:
---
This algorithm isn't thread-safe in the face of concurrent removals, so if we
expect to use it when not holding an exclusive lock, we need a change. The
find-the-next-iterator logic needs to exist in next() as well (and so should be
refactored out into a helper method that I've called getIteratorThatHasNext()
in the code below), to handle the case where the current iterator has a
remaining item when hasNext() is called but no longer does when next() is
called.
The logic of next() should be:
while (true) {
Iterator iterator = getIteratorThatHasNext();
if (iterator == null) {
return null;
}
MessageReference messageReference = iterator.next;
if (messageReference != null) {
return messageReference;
}
}
This algorithm will keep getting the next iterator that should have a next
item, and if it turns out not to then we'll keep getting the next one, until we
either fine one with a non-null next() value or we run out of iterators to try.
> Performance issue in PrioritizedPendingListIterator
> ---
>
> Key: AMQ-6239
> URL: https://issues.apache.org/jira/browse/AMQ-6239
> Project: ActiveMQ
> Issue Type: Improvement
> Components: Broker
>Affects Versions: 5.12.2
>Reporter: Martin Lichtin
> Attachments: AMQ-6239-yourkit-1.jpg, PrioritizedPendingList.java.patch
>
>
> Sending and consuming 5000 messages to/from a queue, one can see heavy CPU
> use on the broker side (v 5.12.2).
> Yourkit shows
> PrioritizedPendingList$PrioritizedPendingListIterator.
> as a hot spot method. It calls ArrayList.add(Object) around 12 mio times.
> Situation is that FilePendingMessageCursor.isEmpty() iterates over in-memory
> messages and therefore (as it is a prioritized queue) uses
> PrioritizedPendingListIterator which uses OrderedPendingList.getAsList()
> which overall turns out to be an expensive method as it converts the
> self-managed linked list to a Java ArrayList and then this list is filled
> into another ArrayList managed by PrioritizedPendingListIterator.
> PrioritizedPendingListIterator could be improved to walk the priority lists
> via OrderedPendingList iterators, as these are implemented efficiently.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (AMQ-6239) Performance issue in PrioritizedPendingListIterator
[ https://issues.apache.org/jira/browse/AMQ-6239?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15234955#comment-15234955 ] Christopher L. Shannon commented on AMQ-6239: - Thanks for the patch. This makes a lot of sense to me, no real reason I can see to create an array list when we can just use iterators. The only thing would be is if for some reason we wanted the iterator to not see future modifications. (since items are copied into lists, any future adds or removes while using the iterator wouldn't be seen). What do you think [~tabish121]? > Performance issue in PrioritizedPendingListIterator > --- > > Key: AMQ-6239 > URL: https://issues.apache.org/jira/browse/AMQ-6239 > Project: ActiveMQ > Issue Type: Improvement > Components: Broker >Affects Versions: 5.12.2 >Reporter: Martin Lichtin > Attachments: AMQ-6239-yourkit-1.jpg, PrioritizedPendingList.java.patch > > > Sending and consuming 5000 messages to/from a queue, one can see heavy CPU > use on the broker side (v 5.12.2). > Yourkit shows > PrioritizedPendingList$PrioritizedPendingListIterator. > as a hot spot method. It calls ArrayList.add(Object) around 12 mio times. > Situation is that FilePendingMessageCursor.isEmpty() iterates over in-memory > messages and therefore (as it is a prioritized queue) uses > PrioritizedPendingListIterator which uses OrderedPendingList.getAsList() > which overall turns out to be an expensive method as it converts the > self-managed linked list to a Java ArrayList and then this list is filled > into another ArrayList managed by PrioritizedPendingListIterator. > PrioritizedPendingListIterator could be improved to walk the priority lists > via OrderedPendingList iterators, as these are implemented efficiently. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (AMQ-6239) Performance issue in PrioritizedPendingListIterator
[ https://issues.apache.org/jira/browse/AMQ-6239?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15234266#comment-15234266 ] Martin Lichtin commented on AMQ-6239: - With the patch applied, these methods no longer appear as hot spots. Method OrderedPendingList.getAsList() can be removed. > Performance issue in PrioritizedPendingListIterator > --- > > Key: AMQ-6239 > URL: https://issues.apache.org/jira/browse/AMQ-6239 > Project: ActiveMQ > Issue Type: Improvement > Components: Broker >Affects Versions: 5.12.2 >Reporter: Martin Lichtin > Attachments: AMQ-6239-yourkit-1.jpg, PrioritizedPendingList.java.patch > > > Sending and consuming 5000 messages to/from a queue, one can see heavy CPU > use on the broker side (v 5.12.2). > Yourkit shows > PrioritizedPendingList$PrioritizedPendingListIterator. > as a hot spot method. It calls ArrayList.add(Object) around 12 mio times. > Situation is that FilePendingMessageCursor.isEmpty() iterates over in-memory > messages and therefore (as it is a prioritized queue) uses > PrioritizedPendingListIterator which uses OrderedPendingList.getAsList() > which overall turns out to be an expensive method as it converts the > self-managed linked list to a Java ArrayList and then this list is filled > into another ArrayList managed by PrioritizedPendingListIterator. > PrioritizedPendingListIterator could be improved to walk the priority lists > via OrderedPendingList iterators, as these are implemented efficiently. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
