RFR(S) 8178351 - Simplify MetaspaceShared::is_in_shared_space and MetaspaceObj::is_shared

[Jiangli Zhou]

Ok, good.

Thanks,
Jiangli

> On Jan 16, 2018, at 11:29 AM, Ioi Lam <ioi.lam at oracle.com> wrote:
> 
> Hi Jiangli,
> 
> 
> Thanks for the review.
> 
> NULL will not pass the test because _shared_metaspace_top is the exclusive high address:
> 
>     return (((void*)this) < _shared_metaspace_top && ((void*)this) >= _shared_metaspace_base);
> 
> so (NULL < NULL) ==> false.
> 
> Thanks
> 
> - Ioi
> 
> 
> 
> On 1/16/18 9:13 AM, Jiangli Zhou wrote:
>> Hi Ioi,
>> 
>> Looks good!
>> 
>> In is_in_shared_metaspace(), can you please add an assert(p !=NULL, “can’t be NULL”)? When the shared spaces are not mapped, MetaspaceObj::_shared_metaspace_base and MetaspaceObj::_shared_metaspace_top are both NULL. In which case a NULL pointer could be mistakenly determined as in shared metaspace.
>> 
>> Thanks,
>> Jiangli
>> 
>>> On Jan 15, 2018, at 4:40 PM, Ioi Lam <ioi.lam at oracle.com> wrote:
>>> 
>>> Hi,
>>> 
>>> Please review the following simple fix for improving CDS start-up time:
>>> 
>>> https://bugs.openjdk.java.net/browse/JDK-8178351
>>> http://cr.openjdk.java.net/~iklam/jdk11/8178351-simplify-is-shared.v01/
>>> 
>>> Since all the CDS shared metaspace regions are consecutively allocated,
>>> and we can guarantee there are no gaps where the OS can allocate new
>>> memory into, we can just do comparisons between 2 pointers. See
>>> allocations.hpp and metaspaceShared.hpp where these methods can be
>>> inlined to improve start-up time when CDS is enabled.
>>> 
>>> For a simple HelloWorld test
>>> 
>>>     java -Xshare:dump
>>>     perf stat -r java -Xshare:on -cp . HelloWorld > /dev/null
>>> 
>>> We see about 1% speed up on my dual-core Haswell box. As usual, YMMV.
>>> 
>>>     BEFORE: 187,757,752 cycles, 0.048169533 sec
>>>     AFTER:  185,770,302 cycles, 0.047497524 sec
>>> 
>>> I also did some incidental code clean up.
>>> 
>>> Testing with hs_tier{1,2,3} now.
>>> 
>>> Thanks
>>> - Ioi
>