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

[Thomas Stüfe]

np

thanks for the review. I'll wait my 24 hours and submit then.

..Thomas

On Mon, May 28, 2018 at 1:00 PM, David Holmes <david.holmes at oracle.com> wrote:
>
>
> 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
>>>>
>>>
>