(10) RFR: 8157709: NMT should use size_t version of Atomic::add

[David Holmes]

Hi Kim,

On 13/02/2017 2:45 PM, Kim Barrett wrote:
>> On Feb 12, 2017, at 8:46 PM, David Holmes <david.holmes at oracle.com> wrote:
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8157709
>>
>> webrev: http://cr.openjdk.java.net/~dholmes/8157709/webrev/
>>
>> NMT typdef'd MemoryCounterType to jlong on 64-bit and jint on 32-bit. Really it should just be size_t and in fact the typedef is not needed at all as it was only used for casting size_t's when invoking Atomic::add. So MemoryCounterType is removed. Only glitch was that using negation of an unsigned variable triggered a warning on Windows which I have had to silence with a pragma. (As unknown pragma's must be ignored I have not ifdef'd the pragma - doing so would also be a problem as warning suppression only operates on the next line, so then I'd have to push and pop the warning which would be really, really ugly.)
>>
>>
>> Also as a result of this change there is no longer any use of the buggy Atomic::add(jlong) and as discussed here:
>>
>> http://mail.openjdk.java.net/pipermail/hotspot-dev/2016-February/021620.html
>>
>> it is simply removed.
>>
>> Testing: JPRT
>>         runtime/NMT
>>
>> Thanks,
>> David
>
> gcc has -Wunknown-pragmas, which is enabled by -Wall.  I suspect we're
> still some way from being able to build with -Wall though...

Aside: -Wunknown-pragmas seems contrary to whole idea that unrecognized 
pragmas are supposed to be ignored! Perhaps useful as a typo checker 
during development, but problematic for cross-platform sources IMHO. But 
we don't turn it on so I'm not going to worry about that.

> My experiments indicate that it does work to #ifdef
> TARGET_COMPILER_visCPP the #pragma, even though that puts the #endif
> between the #pragma and the line to be affected.  If the #ifdef causes
> the #pragma to be kept, the #endif becomes a blank line, and an
> intervening blank line seems to be permitted for such a #pragma.  At
> least, that worked for me...

I was basing things on the stated behaviour in the VS docs, and also 
some things I found online. That said, I initially had the pragma before 
the function (not realising it only affected the next line) and that 
worked as well!!! So given this pragma seems to be behaving somewhat 
contrary to documentation I opted for the simplest, most 'correct' form.

> I'm okay with that part as-is though; your call.

Thanks I will leave as-is unless someone comes up with a good reason to 
do otherwise.

> The rest looks good.

Thanks for the review!

David