On 01/06/2013 12:39 PM, Doug Lea wrote:
On 01/05/13 18:42, Remi Forax wrote:
The code is not very java-ish,
But is very consistent across j.u.c. As a convention,
inline assignments to locals are used to hold field reads
to visually ensure use of consistent snapshots. The C-like
look-and-feel is a less important than is ability to simply
check these cases by inspection.
I know that, I'm fine with inline assignment when needed.
But declaring variables on the first scope of a method or using a
boolean variable to test if a try succeed is for me just a good way to
make the code more obscure than it should.
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.
You wouldn't say this if you were on a 256-way machine with
millions of LongAdders, where only a thousand of them heavily
contended :-)
I'm fine with uncontended/contended switch if the CAS on base fails but
not with the fact that the array used in the contended case is not
created when the thread local object is initialized.
BTW, why do you use an array of objects (Cell) that are padded enough to
avoid false sharing instead of allocating an array big enough and to
store the values at indexes that are far enough from each others, that
should avoid one level of indirections.
Also, I've done a micro-test on my laptop (a modesr 4 cores machine),
trying to calculate the maximum of one miliion values using
LongAccumulator by 4 threads and the calls to Striped64::longAccumulate
are never inlined,
longAccumulate is too big, a better segregation between the fast paths
and the slow ones (that should be pushed in another method) should solve
the issue.
-Doug
Rémi
