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

[Ioi Lam]

On 1/16/18 12:27 AM, Aleksey Shipilev wrote:
> On 01/16/2018 01:40 AM, Ioi Lam 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/

Hi Aleksey, thanks for the review.

> Looks awesome.
>
> Nits:
>
>   *) Stray whitespace before field names?
>
>   229 class MetaspaceObj {
>   230   friend class MetaspaceShared;
>   231   static void*   _shared_metaspace_base;
>   232   static void*   _shared_metaspace_top;
I will fix the spaces.
>
>   *) I'd probably write this statement as math inequality, unless you think one condition fails
> overwhelmingly more frequently:
>
>     return (((void*)this) < _shared_metaspace_top && ((void*)this) >= _shared_metaspace_base);
>   =>
>     return (_shared_metaspace_base <= (void*)this) && ((void*)this < _shared_metaspace_top);
>
>     return (p < MetaspaceObj::_shared_metaspace_top && p >= MetaspaceObj::_shared_metaspace_base);
>   =>
>     return (MetaspaceObj::_shared_metaspace_base <= p) && (p < MetaspaceObj::_shared_metaspace_top);

I want to have fast rejects when CDS is not enabled (and thus both base 
and top are NULL). In this case, this comparison

     (p < MetaspaceObj::_shared_metaspace_top)

will always fail.

Alternatively, I could initialize _shared_metaspace_base to the largest 
possible pointer, but doing that correctly for both 32 and 64 bits is 
probably going to be messy.

>   *) Do you really need "this->" here?
>
>    2183   } else if (this->is_shared()) {

OK, I will remove the "this->".

Thanks
- Ioi
> -Aleksey
>