RFR: JDK-8153203: Remove liveRange.hpp

[Bengt Rutisson]

Thanks for the review, Mikael!

Bengt

On 2016-04-01 10:16, Mikael Gerdin wrote:
> 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
>>>>
>>