RFR(XS): 8178497: Bug in MutableNUMASpace::ensure_parsability
Stefan Johansson
stefan.johansson at oracle.com
Thu Nov 23 09:17:14 UTC 2017
On 2017-11-22 22:23, sangheon.kim wrote:
> Hi Stefan,
>
> Thank you for reviewing this.
>
> On 11/21/2017 05:29 AM, Stefan Johansson wrote:
>> Hi Sangheon,
>>
>> On 2017-11-20 20:05, 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
>> Thanks for taking care of this. I think the changes is good, but I
>> would feel even more certain if you ran some testing with NUMA and
>> Parallel to make sure we haven't overlooked anything.
> Okay, I added some explanation about what would happen.
> And also ran hundred runs of GCOld with NUMA enabled on Solaris, of
> course no problem.
>
> Hope this is sufficient to go. :)
>
Thanks Sangheon,
That's great.
Cheers,
Stefan
> Thanks,
> Sangheon
>
>
>>
>> Thanks,
>> StefanJ
>>> Testing: local building
>>>
>>> Thanks,
>>> Sangheon
>>
>
More information about the hotspot-gc-dev
mailing list