Github user franz1981 commented on the issue:
https://github.com/apache/activemq-artemis/pull/1851
@wy96f If you want to take a look/review the code I have pushed
https://github.com/apache/activemq-artemis/pull/1895 to improve the volatile
usage witht the results I've attached in the
Github user franz1981 commented on the issue:
https://github.com/apache/activemq-artemis/pull/1851
@wy96f
> In your example
My example was just to show the method used to perform a padding with an
existing abstract class, has not values from the perf purposes: sorry, my
Github user wy96f commented on the issue:
https://github.com/apache/activemq-artemis/pull/1851
@franz1981 Hi, I'm a little puzzled about padding technique. In your
example, i know that stampedLock fileds are in different cache lines for
different sections but fields like capacity, si
Github user franz1981 commented on the issue:
https://github.com/apache/activemq-artemis/pull/1851
@michaelandrepearce Yes agree :+1:
I've performed a couple of benchs and that's what I'm getting (reusing this
benchmark: ):
```
master:
ConcurrentLongHashMapThroughput.r
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1851
@franz, agreed volatile hit is on the write wy96f only changed the capacity
to be volatile and changed the order theyâre set to fix a bug. this only
changes when the map is re
Github user franz1981 commented on the issue:
https://github.com/apache/activemq-artemis/pull/1851
@michaelandrepearce Agree about aggressive optimisations but not on
volatile operation's: isn't the volatile read that cost but the volatile store
that include an hidden cost a of a full
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1851
I would be careful of over optimizing this. They let reason for the use of
this class was to reduce the long autoboxing. Itâs used throughout the code
base where concurrent ha
Github user franz1981 commented on the issue:
https://github.com/apache/activemq-artemis/pull/1851
@wy96f
> Adding unsafe is not trivial and lazySet is enough
Probably we have already access to it thanks to
`io.netty.util.internal.shaded.org.jctools.util.UnsafeAccess` but
Github user wy96f commented on the issue:
https://github.com/apache/activemq-artemis/pull/1851
@franz1981 1. I agree 2. Adding unsafe is not trivial and lazySet is
enough 3. size() is seldomly called, isn't it? 4. We can add
@sun.misc.Contended in Section class, right?
---
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1851
There are tests that run as part of PR and then separately there are a
fuller test suite. Reason for this is the larger test suite takes much longer
to complete, as such itâs
Github user wy96f commented on the issue:
https://github.com/apache/activemq-artemis/pull/1851
@michaelandrepearce hi, i don't get what you mean the test. Why do we need
a suit test? And where should the test be put to stop from running by CI buid?
---
Github user franz1981 commented on the issue:
https://github.com/apache/activemq-artemis/pull/1851
@wy96f Np you can pick up the master and push a new PR with the new changes
that are more improvements/gardening than fixes.
IMO would be interesting to:
- drop the (completly use
Github user wy96f commented on the issue:
https://github.com/apache/activemq-artemis/pull/1851
Oops, i deleted the branch :( It seems not possible to reopen. I'll create
a new one.
---
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1851
@wy96f could you rebase (it seems clebert merged my branch (which had your
original code) i used for checking that the build passed in the night)
Also once rebased could
Github user wy96f commented on the issue:
https://github.com/apache/activemq-artemis/pull/1851
@franz1981 lazySet is used to update capacity. Please review it :)
---
Github user wy96f commented on the issue:
https://github.com/apache/activemq-artemis/pull/1851
Sorry for the late reply. Using lazySet to update capacity filed is a good
point. As for the test, I modified test as follows:
@Test
public void concurrentInsertionsAndReads()
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1851
Seems first time travis build failed for unrelated reason second attempt
https://github.com/apache/activemq-artemis/pull/1892
is fine.
---
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1851
@jbertram i have taken @wy96f just rebased and done a PR so travis CI will
build see https://github.com/apache/activemq-artemis/pull/1891
If that passes we can merge thi
Github user jbertram commented on the issue:
https://github.com/apache/activemq-artemis/pull/1851
Please rebase this and push -f so the new Travis CI build for the PR will
run. Thanks!
---
Github user franz1981 commented on the issue:
https://github.com/apache/activemq-artemis/pull/1851
@wy96f Well done, please address the changes I've proposed re lazySet and
test :+1:
---
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1851
@wy96f could you update your commit message with that.
---
Github user wy96f commented on the issue:
https://github.com/apache/activemq-artemis/pull/1851
Hi, franz1981. Thanks for the review. The jira issue is ARTEMIS-1664.
---
22 matches
Mail list logo
