RFR (S): 8008314 - Unimplemented() Atomic::load breaks the applications
David Holmes
david.holmes at oracle.com
Mon Feb 18 23:08:43 PST 2013
Hi Jesper,
The jlong version of Atomic::add not only relies on a jlong version of
Atomic::load but also a jlong version of Atomic::cmpxchg - facilities
that are only conditionally available on 32-bit platforms. Further, the
jlong version of add implements the wrong semantics by returning the
previous value rather than the updated value as it should (as per
atomic.hpp).
Given that the jlong Atomic::add will not be used once this is pushed
(assuming Bengt pushes 8008382 first), could I ask that you also delete
it from atomic.cpp/hpp as part of this change. That way no one will be
tempted to use it again. I'd also suggest adding a comment in atomic.hpp:
// Atomic operations on jlong types are not available on all 32-bit
// platforms. If atomic ops on jlongs are defined here they must only
// be used from code that verifies they are available at runtime and
// can provide an alternative action if not - see supports_cx8() for
// a means to test availability
There remain Atomic::store(jlong) and Atomic::load(jlong) operations
that violate this rule but the code involved is not presently used on
platforms that don't support them. This obviously needs fixing.
Thanks,
David
On 19/02/2013 4:25 PM, Jesper Wilhelmsson wrote:
> Hi,
>
> Please review the following change:
>
> Webrev: http://cr.openjdk.java.net/~jwilhelm/8008314/
>
> Bug: https://jbs.oracle.com/bugs/browse/JDK-8008314
>
>
> The use of atomic::add(jlong,jlong*) breaks everything on ARM since it
> uses atomic::load(jlong) which isn't implemented.
>
> The add was used to update a counter in rotating log file handling, but
> it turns out that the counter that was updated doesn't need to be a jlong.
>
> The counter is used to indicate that it is time to do a log file
> rotation and by adding a saftey catch for overflow, we can use an uintx
> for the counter.
>
> In practice this means that the upper "limit" for the log file size
> becomes UINX_MAX-1 (used to be UINT_MAX) since I change from < to <= in
> the comparison to the limit, but since this is not a hard limit the
> difference will only be noticeable when someone has written UINT_MAX
> characters to the log file and the next write will end up in the next
> log file part instead of in the same (which one could argue is a more
> correct behavior).
>
> Sine I changed all places where the counter was used I also fixed a typo
> in its name.
>
> Thanks,
> /Jesper
More information about the hotspot-dev
mailing list