RFR: JDK-8153203: Remove liveRange.hpp

[Mikael Gerdin]

Hi Bengt,

The change looks good to me.

/Mikael

On 2016-04-01 07:25, Bengt Rutisson wrote:
>
> Hi Jesper,
>
> On 2016-03-31 21:23, Jesper Wilhelmsson wrote:
>> Den 31/3/16 kl. 21:05, skrev Bengt Rutisson:
>>>
>>> Hi Jesper,
>>>
>>> On 31/03/16 20:08, Jesper Wilhelmsson wrote:
>>>> Hi Bengt,
>>>>
>>>> Nice cleanup!
>>>
>>> Thanks! And thanks for looking at this!
>>>
>>>>
>>>> The comment in psMarkSweepDecorator.cpp:294 is slightly confusing:
>>>> "The first
>>>> dead object should contain a pointer to the first live object"
>>>>
>>>> Does it mean "The first object is dead and should contain a pointer
>>>> to the
>>>> first live object"?
>>>
>>> It actually means that the first dead object is no longer an object.
>>> Instead, at
>>> that memory address, there is just a pointer to the first live object
>>> that the
>>> previous phase found. So, I think the comment is correct, but I
>>> understand why
>>> it is confusing. "First dead" here, refers to the variable _first_dead.
>>>
>>> Maybe this would be clearer:
>>>
>>> "The first dead object is no longer an object. At that memory
>>> address, there is
>>> a pointer to the first live object that the previous phase found."
>>
>> Yes, that's more clear. Thanks for clarifying that!
>
> OK. Thanks, I'll change to that comment then.
>
> Thanks for reviewing!
> Bengt
>
>> /Jesper
>>
>>>
>>> Thanks,
>>> Bengt
>>>
>>>>
>>>> Besides that the change looks good.
>>>> /Jesper
>>>>
>>>>
>>>> Den 31/3/16 kl. 17:46, skrev Bengt Rutisson:
>>>>>
>>>>> Hi everyone,
>>>>>
>>>>> Could I have a couple of reviews for this change?
>>>>>
>>>>> http://cr.openjdk.java.net/~brutisso/8153203/webrev.00/
>>>>> https://bugs.openjdk.java.net/browse/JDK-8153203
>>>>>
>>>>> The methods CompactibleSpace::scan_and_forward() and
>>>>> PSMarkSweepDecorator::precompact() are more or less copy-paste
>>>>> versions of each
>>>>> other. Both these use the LiveRange class.
>>>>>
>>>>> The LiveRange class is used to write to the mark word of a dead
>>>>> object. But no
>>>>> one really cares that the LiveRange class is used. Instead it just
>>>>> gives an
>>>>> extra level of indirection to already complicated code. We also do
>>>>> unnecessary
>>>>> work to keep track of the end of the range even though no one ever
>>>>> gets the end
>>>>> value.
>>>>>
>>>>> We also do duplicate stores to _first_dead in these methods.
>>>>>
>>>>> The methods that consume the values from the LiveRange
>>>>> (CompactibleSpace::scan_and_adjust_pointers() and
>>>>> PSMarkSweepDecorator::adjust_pointers()) don't use the LiveRange
>>>>> class. Instead
>>>>> they use oop()->mark()->decode_pointer(), which is kind of odd
>>>>> considering that
>>>>> this is normally used for forwarded objects.
>>>>>
>>>>> The code would be simpler if we just store and load directly from
>>>>> the memory
>>>>> addresses we are working with.
>>>>>
>>>>> Improving the assert at the end of the
>>>>> CompactibleSpace::scan_and_adjust_pointers() and
>>>>> PSMarkSweepDecorator::adjust_pointers() methods to log the values
>>>>> of q and
>>>>> prev_q will hopefully improve the chances of understanding
>>>>> JDK-8073321 if that
>>>>> ever happens again.
>>>>>
>>>>> Thanks,
>>>>> Bengt
>>>
>