RFR: JDK-8153203: Remove liveRange.hpp

[Bengt Rutisson]

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
>>