RFR (XS): 8203262: Incorrect cmpxchg usage in MetaspaceGC::inc_capacity_until_GC

[Per Liden]

On 2018-05-20 11:07, Aleksey Shipilev wrote:
> On 05/20/2018 10:04 AM, Thomas Schatzl wrote:
>> Webrev:
>> http://cr.openjdk.java.net/~tschatzl/8203262/webrev
> 
> Looks good.
> 
> I would probably rename "old_capacity_until_GC" to just "old_value" to reduce noise and provide
> visible symmetries. And avoided "actual" because it is confusing -- it is not an actual value anymore :)

Looks good Thomas. And I agree with Aleksey's naming suggestion. But I 
would go one step further calling it "prev_value" instead of "actual" or 
"res". Maybe it's just my style, but I always try to use the 
new/old/prev naming scheme in CAS-loops as I find it very clarifying.

Anyway, whatever name you pick, I don't need to see a new webrev.

cheers,
Per

> 
> E.g.:
> 
> 2420   size_t old_value = _capacity_until_GC;
> 2421   size_t new_value = old_value + v;
> 2422
> 2423   if (new_value < old_value) {
> 2424     // The addition wrapped around, set new_value to aligned max value.
> 2425     new_value = align_down(max_uintx, Metaspace::commit_alignment());
> 2426   }
> 2427
> 2428   size_t res = Atomic::cmpxchg(new_value, &_capacity_until_GC, old_value);
> 2429
> 2430   if (old_value != res) {
> 2431     return false;
> 2432   }
> 2433
> 2434   if (new_cap_until_GC != NULL) {
> 2435     *new_cap_until_GC = new_value;
> 2436   }
> 2437   if (old_cap_until_GC != NULL) {
> 2438     *old_cap_until_GC = old_value;
> 2439   }
> 2440   return true;
> 
> -Aleksey
> 
>