RFR(xxxs, trivial): 8203865: Metaspace cleanup: Remove unused MemRegion in VirtualSpaceNode

David Holmes david.holmes at oracle.com
Mon May 28 11:00:14 UTC 2018



On 28/05/2018 7:15 PM, Thomas Stüfe wrote:
> Hi David,
> 
> Thanks for the quick review!
> 
> On Mon, May 28, 2018 at 8:47 AM, David Holmes <david.holmes at oracle.com> wrote:
>> Hi Thomas,
>>
>> On 28/05/2018 4:28 PM, Thomas Stüfe wrote:
>>>
>>> Hi all,
>>>
>>> very tiny cleanup.
>>>
>>> May I get a review (I consider this trivial):
>>
>>
>> I'd like someone to confirm that the deleted asserts are still logically
>> being checked elsewhere.
>>
> 
>       set_top((MetaWord*)virtual_space()->low());
> -    set_reserved(MemRegion((HeapWord*)_rs.base(),
> -        (HeapWord*)(_rs.base() + _rs.size())));
> -
> -    assert(reserved()->start() == (HeapWord*) _rs.base(),
> -        "Reserved start was not set properly " PTR_FORMAT
> -        " != " PTR_FORMAT, p2i(reserved()->start()), p2i(_rs.base()));
> -    assert(reserved()->word_size() == _rs.size() / BytesPerWord,
> -        "Reserved size was not set properly " SIZE_FORMAT
> -        " != " SIZE_FORMAT, reserved()->word_size(),
> -        _rs.size() / BytesPerWord);
>     }
> 
> No need; the asserts check that the MemRegion object is initialized
> correctly; since it is unused and removed, the asserts can be removed
> too.

Sorry - yes I see that now.

Thanks,
David

> Thanks, Thomas
> 
> 
>> Otherwise this cleanup seems ok.
>>
>> Thanks,
>> David
>>
>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8203865
>>> Webrev:
>>> http://cr.openjdk.java.net/~stuefe/webrevs/8203865-Remove-unused-MemRegion-in-VirtualSpaceNode/webrev.00/webrev/
>>>
>>> VirtualSpaceNode contains: MemRegion _reserved;
>>>
>>> This member is not used and serves no purpose. It keeps track of the
>>> reserved memory location, but that is redundant since the underlying
>>> ReservedSpace does the same. It is also nowhere used. So it should be
>>> removed.
>>>
>>> Best Regards, Thomas
>>>
>>


More information about the hotspot-runtime-dev mailing list