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

[Ioi Lam]

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