You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by franz1981 <gi...@git.apache.org> on 2017/01/02 14:54:32 UTC
[GitHub] activemq-artemis pull request #945: ARTEMIS-905 JCtools ConcurrentMap replac...
GitHub user franz1981 opened a pull request:
https://github.com/apache/activemq-artemis/pull/945
ARTEMIS-905 JCtools ConcurrentMap replacement
It's a total replacement of ConcurrentHasMap and ConcurrentHashSet usage with faster and lower memory footprint versions of jctools; a high performance concurrent collections library.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/franz1981/activemq-artemis chm_replacement
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/activemq-artemis/pull/945.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #945
----
commit fe40c1ba2153b110be329175142e494643602093
Author: Francesco Nigro <ni...@gmail.com>
Date: 2017-01-02T14:50:53Z
ARTEMIS-905 JCtools ConcurrentMap replacement
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #945: ARTEMIS-905 JCtools ConcurrentMap replacement
Posted by franz1981 <gi...@git.apache.org>.
Github user franz1981 commented on the issue:
https://github.com/apache/activemq-artemis/pull/945
@ig-michaelpearce Yes, are almost all replaced but I left it because of the tests that used them.
I'm evaluating if close or not the PR because even if there are (good) improvements, they are not so great if compared to the huge quantity of modifications along all the project code/tests!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #945: ARTEMIS-905 JCtools ConcurrentMap replacement
Posted by ig-michaelpearce <gi...@git.apache.org>.
Github user ig-michaelpearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/945
@franz1981 to me it looks like a clean replacement you've done.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #945: ARTEMIS-905 JCtools ConcurrentMap replacement
Posted by franz1981 <gi...@git.apache.org>.
Github user franz1981 commented on the issue:
https://github.com/apache/activemq-artemis/pull/945
@ig-michaelpearce No problem, I'll be more than happy to check it out and try to understand what could be happened; my personal email is on the GitHub profile
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #945: ARTEMIS-905 JCtools ConcurrentMap replacement
Posted by ig-michaelpearce <gi...@git.apache.org>.
Github user ig-michaelpearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/945
@franz1981 I was just looking at the results, p50 (50%) and mean are equal so one expects these to be equal, i notice in the results they have different values within the same test result.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #945: ARTEMIS-905 JCtools ConcurrentMap replacement
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/945
@franz1981 please take a look, the build doesn't complete on this PR.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request #945: ARTEMIS-905 JCtools ConcurrentMap replac...
Posted by franz1981 <gi...@git.apache.org>.
Github user franz1981 closed the pull request at:
https://github.com/apache/activemq-artemis/pull/945
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #945: ARTEMIS-905 JCtools ConcurrentMap replacement
Posted by ig-michaelpearce <gi...@git.apache.org>.
Github user ig-michaelpearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/945
Out of interest why JCTools over some other libs with similar goals? Agrona, Kolobroke, GS collections, fastutil, HPPC & Trove
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #945: ARTEMIS-905 JCtools ConcurrentMap replacement
Posted by jbertram <gi...@git.apache.org>.
Github user jbertram commented on the issue:
https://github.com/apache/activemq-artemis/pull/945
Does this yield any measurable performance gains? I'm curious about any benchmarks performed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #945: ARTEMIS-905 JCtools ConcurrentMap replacement
Posted by ig-michaelpearce <gi...@git.apache.org>.
Github user ig-michaelpearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/945
@franz1981 if all reference to ConcurrentHashSet are replaced, can the class be removed in org.apache.activemq.artemis.utils?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #945: ARTEMIS-905 JCtools ConcurrentMap replacement
Posted by franz1981 <gi...@git.apache.org>.
Github user franz1981 commented on the issue:
https://github.com/apache/activemq-artemis/pull/945
@jbertram
I've attached the first results on [localhost.txt](https://github.com/apache/activemq-artemis/files/687370/localhost.txt).
The throughputs (end to end) are pretty the same (a lil' adavantage for the JCtools maps) and the memory footprint is similar.
What is pretty different is the latency distribution: the JCtools version deliver a more stable throughput probably due to fewer and shorter GC pauses (there are less Long and Node instances produced and collected).
The test configuration is:
- Max throughput, single queue, concurrent send and receive
- Non-persistent messages, at-least-once delivery, auto acknowledge (JMS)
- same machine RHEL 6.9 OpenJDK 1.8.0
- 100000 warmup messages + 5 runs of 100000 messages
- 100 bytes per message
I've tested everything on the same machine with the default (AIO) persistence and the Artemis JMS client.
It is not the typical benchmark that helps the JCtools maps to shine against the vanilla one.
I'm building a micro-benchmark to stress the journal and provides best case results too :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #945: ARTEMIS-905 JCtools ConcurrentMap replacement
Posted by franz1981 <gi...@git.apache.org>.
Github user franz1981 commented on the issue:
https://github.com/apache/activemq-artemis/pull/945
@ig-michaelpearce Good libraries, but no one of them provide a concurrent map replacement AFAIK, this is one of the reasons behind the choice of JCtools.
In addition to that, JCtools has the fastest implementations of lock-free/wait-free queues; the next step IMHO will be to test if all the ConcurrentLinkedQueues/ArrayBlockingQueues used in Artemis could be replaced by these ones, hence we'll kill two birds with one stone (poor birds!)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #945: ARTEMIS-905 JCtools ConcurrentMap replacement
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/945
@franz1981 can you either rework this or close it?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #945: ARTEMIS-905 JCtools ConcurrentMap replacement
Posted by ig-michaelpearce <gi...@git.apache.org>.
Github user ig-michaelpearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/945
@franz1981 we patched the 1.x branch locally and ran this, we saw some unexpected jitter compared with 1.5.1 release. could be other 1.x code changes since 1.5.1, unfortunately cannot share result publicly but can share privately.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #945: ARTEMIS-905 JCtools ConcurrentMap replacement
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/945
+1 on this.
only thing is: make sure you run a full testsuite before running this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #945: ARTEMIS-905 JCtools ConcurrentMap replacement
Posted by franz1981 <gi...@git.apache.org>.
Github user franz1981 commented on the issue:
https://github.com/apache/activemq-artemis/pull/945
@clebertsuconic yes!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #945: ARTEMIS-905 JCtools ConcurrentMap replacement
Posted by franz1981 <gi...@git.apache.org>.
Github user franz1981 commented on the issue:
https://github.com/apache/activemq-artemis/pull/945
@jbertram I'm building some macro benchmark right know, as soon as I'll finish them I'll provide the results and metodology of the benchs :)
I suspect that I'll have to change a lil' bit the PR, just to be sure It doesn't corrupt any original behaviour...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---