[
https://issues.apache.org/jira/browse/ARTEMIS-2996?focusedWorklogId=512723&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-512723
]
ASF GitHub Bot logged work on ARTEMIS-2996:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 17/Nov/20 04:48
Start Date: 17/Nov/20 04:48
Worklog Time Spent: 10m
Work Description: franz1981 commented on a change in pull request #3343:
URL: https://github.com/apache/activemq-artemis/pull/3343#discussion_r524882100
##########
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/WildcardAddressManager.java
##########
@@ -221,19 +221,21 @@ private void addAddress(final SimpleString address, final
Address actualAddress)
}
}
- private synchronized void removeAndUpdateAddressMap(final Address address)
throws Exception {
+ private void removeAndUpdateAddressMap(final Address address) throws
Exception {
// we only remove if there are no bindings left
Bindings bindings =
super.getBindingsForRoutingAddress(address.getAddress());
- if (bindings == null || bindings.getBindings().size() == 0) {
- List<Address> addresses = address.getLinkedAddresses();
- for (Address address1 : addresses) {
- address1.removeLinkedAddress(address);
- Bindings linkedBindings =
super.getBindingsForRoutingAddress(address1.getAddress());
- if (linkedBindings == null || linkedBindings.getBindings().size()
== 0) {
- removeAddress(address1);
+ if (bindings == null || bindings.getBindings().isEmpty()) {
+ synchronized (this) {
+ List<Address> addresses = address.getLinkedAddresses();
Review comment:
> it seems to me that you can either move the synchronize to something
specific to the address (synchronize (address) { // the one you returned from
getLinkedAddresses();
or the whole thing should be synchronized...
I've no idea TBH: consider that in other methods eg `addAndUpdateAddressMap`
we treat changes in the 2 directions (the addresses referenced by
`address::getLinkedAddresses` and `address`) as a whole/atomically, so I wasn't
sure to further narrow the lock scope to something different.
Instead, I already have a little concern on the correctness as it is now,
but I need to write a concurrent test to verify this, so will update this
comment later.
> Are you sure this is the right thing?
Never :D
From a performance perspective this is already a nice step beyond, because
in the hot path of binding removal we're not anymore causing additions to be
blocked. The ideal thing would be to NOT have any sparse linked address list,
but an internal data structure on `WildcardAddressManager` (maybe a
concurrent/or not BiMap or something similar?)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 512723)
Time Spent: 9h (was: 8h 50m)
> Provide JMH Benchmarks for Artemis
> ----------------------------------
>
> Key: ARTEMIS-2996
> URL: https://issues.apache.org/jira/browse/ARTEMIS-2996
> Project: ActiveMQ Artemis
> Issue Type: Bug
> Components: Tests
> Reporter: Francesco Nigro
> Assignee: Francesco Nigro
> Priority: Major
> Time Spent: 9h
> Remaining Estimate: 0h
>
> In order to reliably measure performance of many Artemis component would be
> welcome to implement some https://github.com/openjdk/jmh benchmarks to be
> used for development purposes ie not part of the release
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
