[ 
https://issues.apache.org/jira/browse/QPID-8238?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16610324#comment-16610324
 ] 

Alex Rudyy edited comment on QPID-8238 at 9/11/18 9:06 AM:
-----------------------------------------------------------

Keith,

Thanks a lot for the review comments. 

My changes for logback filter and fanout exchange routing functionality were 
driven by jprofiler analysis indicating about performance being lost on 
unnecessary {{Map}} object creation. I committed them against this JIRA as they 
improve the performance a bit whilst routing the messages with fanout exchange 
but the performance of other exchange implementation should improve as well. 
Though, I am happy to revert the mentioned changes and commit them against 
separate JIRAs.

As for exchange changes, they not only optimize the code but make it much 
simpler. Effectively, the changed code converts the collection of 
{{TopicMatcherResult}} got based on message routing key into a map where keys 
are {{route destinations}} and values are {{routing/replacement keys}}. Before 
my change the conversions was done by using intermediate map containing the 
destination as a key and routing replacement key as a value. Than, multiple 
maps (representing each {{TopicMatcherResult}} ) were merged into a single map.
My changes eliminated the need for intermediate map creation. I also do hope 
that code is a bit simpler now. I left the old implementation untouched and 
marked is as {{Deprecated}} in order to keep public methods unchanged as the 
next version is minor release rather major one. (Though, I did not check 
whether we made other breakable changes in the code in general. Perhaps, 
keeping dead-code there is a waist). I am planning to merge committed changes 
into 7.0 and do not want break backward compatibility there.

I liked  Rob's idea of making {{FieldTable}} immutable and do on-demand 
parsing. I will try to implement the corresponding changes as part of this 
JIRA, however, making {{FieldTable}} immutable most likely will not have any 
effect on performance and performance is the main concern for me right now. I 
do not think I can port on-demand parsing changes into 7.0. Thus, I am 
currently looking into other performance optimization alternatives. I have not 
looked into on-demand parsing yet.

I played a bit with replacing of {{AMQShortStrings}} with java {{String}} in 
{{BasicContentHeaderProperties}} (I have a patch a corresponding changes). My 
performance tests did not revealed much performance improvements due to using 
java {{String}} in {{BasicContentHeaderProperties}}. However, they did improve 
the performance a bit. Potentially, making {{AMQShortString}}s to be a wrapper 
of {{String}} and replacing usage of  {{AMQShortString}}s with java {{String}} 
where possible could be another refactoring worth doing.

The current routing code is very well tuned. I do not see any other routing 
functionality to optimize. As a next steps I am going to add new topic 
performance tests into existing perf test suite and look into making 
{{FieldTable}} immutable and implementing on-demand parsing. 




was (Author: alex.rufous):
Keith,

Thanks a lot for the review comments. 

My changes for logback filter and fanout exchange routing functionality were 
driven by jprofiler analysis indicating about performance being lost on 
unnecessary {{Map}} object creation. I committed them against this JIRA as they 
improve the performance a bit whilst routing the messages with fanout exchange 
but the performance of other exchange implementation should improve as well. 
Though, I am happy to revert the mentioned changes and commit them against 
separate JIRAs.

As for exchange changes, they not only optimize the code but make it much 
simpler. Effectively, the changed code converts the collection of 
{{TopicMatcherResult}} got based on message routing key into a map where keys 
are {{route destinations}} and values are {{routing/replacement keys}}. Before 
my change the conversions was done by using intermediate map containing the 
destination as a key and routing replacement key as a value. Than, multiple 
maps (representing each {{TopicMatcherResult}} ) were merged into a single map.
My changes eliminated the need for intermediate map creation. I also do hope 
that code is a bit simpler now. I left the old implementation untouched and 
marked is as {{Deprecated}} in order to keep public methods unchanged as the 
next version is minor release rather major one. (Though, I did not check 
whether we made other breakable changes in the code in general. Perhaps, 
keeping dead-code there is a waist). I am planning to merge committed changes 
into 7.0 and do not want break backward compatibility there.

I liked  Rob's idea of making {{FieldTable}} immutable and do on-demand 
parsing. I will try to implement the corresponding changes as part of this 
JIRA, however, making {{FieldTable}} immutable most likely will not have any 
effect on performance and performance is the main concern for me right now. I 
do not think I can port on-demand parsing changes into 7.0. Thus, I am 
currently looking into other performance optimization alternatives. I have not 
looked into on-demand parsing yet.

I played a bit with replacing of {{AMQShortString}}s with java {{String}} in 
{{BasicContentHeaderProperties}} (I have a patch a corresponding changes). My 
performance tests did not revealed much performance improvements due to using 
java {{String}} in {{BasicContentHeaderProperties}}. However, they did improve 
the performance a bit. Potentially, making {{AMQShortString}}s to be a wrapper 
of {{String}} and replacing usage of  {{AMQShortString}}s with java {{String}} 
where possible could be another refactoring worth doing.

The current routing code is very well tuned. I do not see any other routing 
functionality to optimize. As a next steps I am going to add new topic 
performance tests into existing perf test suite and look into making 
{{FieldTable}} immutable and implementing on-demand parsing. 



> [Broker-J] Improve performance of asynchronous publishing of transient 
> messages into topic exchange having queues bound using non-overlapping 
> selectors 
> --------------------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: QPID-8238
>                 URL: https://issues.apache.org/jira/browse/QPID-8238
>             Project: Qpid
>          Issue Type: Improvement
>          Components: Broker-J
>    Affects Versions: qpid-java-broker-7.0.6
>            Reporter: Alex Rudyy
>            Priority: Major
>             Fix For: qpid-java-broker-7.1.0, qpid-java-broker-7.0.7
>
>         Attachments: 
> 0001-QPID-8238-Use-java.lang.String-for-keys-and-values-i.patch
>
>
> The performance of asynchronous publishing of transient messages into topic 
> exchange which routes messages into queues bound using non-overlapping 
> selectors is 2-3 times slower than performance of 0.32 broker. The 
> performance degradation is observed with AMQP 0.9, though, I suspect that the 
> AMQP 0-10 protocol could be affected as well.
> I was running tests with 10 concurrent producers publishing messages  on 
> separate connections using the same routing key into 10 different queues 
> (subscribers queues) bound to the exchange using non-overlapping selectors.
> My testing showed that performance of 7.0 broker for this particular use case 
> was 2-3 times worse than performance of 0.32 broker.
> The following factors contributed to degradation of performance:
> •     Copying data from direct memory into heap memory whilst decoding 
> message headers. Due to this factor, the decoding of message headers is 
> around twice slower. It seems it contributes around 70% to total performance 
> degradation
> •     The message routing algorithm is slower due to need to support a new 
> feature to route messages into bound exchanges (in addition to queues) using 
> replacement routing key.
> •     AMQ short strings caching contributes 5-10% to total performance 
> degradation. The caching was added to manage heap space more efficiently.
> The numbers provided here could be inaccurate due instrumentation overhead 
> whilst profiling the issue.
> Potentially, caching can be turned off but that will not improve performance 
> much.
> On other hand, adding of additional caching of strings to amqp-short-strings 
> would improve the performance a bit. Whilst evaluating selectors, the fields 
> used in selector expressions are represented as java strings but they get 
> converted every time into amqp-short-strings when looking up for message 
> header values. If 10 queues are bound to the exchange using the same binding 
> key, the selector expression is evaluated 10 times for the incoming message. 
> Thus, all selector field names are get converted into amqp-short-strings 10 
> times as well. It seems adding caching here can improve the performance. 



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to