RFR(XS): 8178497: Bug in MutableNUMASpace::ensure_parsability

[sangheon.kim]

Hi Thomas,

On 11/21/2017 03:27 AM, Thomas Schatzl wrote:
> Hi Sangheon,
>
> On Mon, 2017-11-20 at 11:05 -0800, sangheon.kim wrote:
>> Hi all,
>>
>> Can I have some reviews for this tiny pointer arithmetic change?
>> Current code doesn't use pointer arithmetic, so the the resulting
>> values are wrong(too small). i.e. adding two values first and then
>> typecast to HeapWord* which is wrong here.
>> e.g.
>> intptr_t cur_top = X;
>> size_t touched_words = XX;
>> ...
>> align_down((HeapWord*)(cur_top + touched_words), XXX);
>>
>> This should be 'align_down( (HeapWord*)cur_top + touched_words,
>> XXXX);'.
>>
>> As I don't see any good reason of using 'intptr_t', changed to use
>> 'HeapWord*' and changed related stuff.
>>
>> I didn't add regression test or some further investigation as this
>> is clear that the calculation is wrong. And hard to provoke the
>> problem outside.
>>
>> CR: https://bugs.openjdk.java.net/browse/JDK-8178497
>> Webrev: http://cr.openjdk.java.net/~sangheki/8178497/webrev.0
>> Testing: local building
>    actually after reading this code a bit in more detail I think the
> failure mode is "only" performance loss. I.e. the "invalid" part of a
> MutableNUMASpace is freed and reallocated for better NUMA locality.
Agree.

>
> With a too small invalid size what happens is that this locality
> improvement will not happen for that part of the MutableNUMASpace. I do
> not think there is a useful way of creating a reproducer.
>
> Looks good.
Thanks for the review.

Thanks,
Sangheon


>
> Thanks,
>    Thomas
>