On 01/05/2013 07:10 PM, Chris Hegarty wrote:
As part of JEP 155 we are proposing to add the following public
classes to support Scalable Updatable Variables, DoubleAccumulator,
DoubleAdder, LongAccumulator and LongAdder.
These have been written by Doug Lea, with assistance from members of
the former JCP JSR-166 Expert Group.
Webrev and javadoc are at:
http://cr.openjdk.java.net/~chegar/8005311/ver.00/
Since Doug is the author, I am taking a reviewer/sponsor role.
Here are my initial comments.
- There are various places in DoubleAccmulator where there are broken
links to #sum ( I think it is just a cut'n'paste error ). These
should be #get.
- Accumulators
{@link #get} may read somewhat better as {@link #get current value} ??
- Accumulators
Does the 'identity' value need further explanation?
Note: There is one minor change to the implementation. Currently in
the jdk8 repo j.u.f.DoubleBinaryOperator defines operateAsDouble. This
method has been renamed to applyAsDouble in the lambda/lambda repo.
When these changes are sync'ed from lambda/lambda this can be
reverted. A similar comment has been added to the code.
-Chris.
The code is not very java-ish,
by example, in Striped64,
Cell[] as; Cell a; int n; long v;
if ((as = cells) != null && (n = as.length) > 0) {
if ((a = as[(n - 1) & h]) == null) {
instead
int n;
Cell[] as = cells;
if (as != null && (n = as.length) > 0) {
Cell a = as[(n - 1) & h];
if ((a == null) {
also I think that the variable created (and 'init' after in the code)
are not needed.
boolean created = false;
try { // Recheck under lock
Cell[] rs; int m, j;
if ((rs = cells) != null &&
(m = rs.length) > 0 &&
rs[j = (m - 1) & h] == null) {
rs[j] = r;
created = true;
}
} finally {
cellsBusy = 0;
}
if (created)
break;
The code can become
try { // Recheck under lock
Cell[] rs = cells; int m, j;
if (rs != null &&
(m = rs.length) > 0 &&
rs[j = (m - 1) & h] == null) {
rs[j] = r;
break;
}
} finally {
cellsBusy = 0;
}
Overall, I think there are too many lazy initializations.
Unlike HashMap, if a developer uses let say LongAccumulator it's because
AtomicLong doesn't work well,
so not having the array of cells initialized by default seems weird.
Rémi
