RFR (S): 8202021: Improve variable naming in ReferenceProcessor

Stefan Johansson stefan.johansson at oracle.com
Tue Apr 24 06:59:43 UTC 2018



On 2018-04-23 22:23, sangheon.kim wrote:
> Hi Thomas,
> 
> On 04/23/2018 07:33 AM, Thomas Schatzl wrote:
>> On Mon, 2018-04-23 at 16:02 +0200, Stefan Johansson wrote:
>>> Hi Thomas,
>>>
>>> On 2018-04-19 22:08, Thomas Schatzl wrote:
>>>> Hi Sangheon,
>>>>
>>>>     thanks for your review. I updated the change in-place.
>>>>
>>>> Thomas
>>>>
>>>> On Thu, 2018-04-19 at 12:38 -0700, sangheon.kim wrote:
>>>>> Hi Thomas,
>>>>>
>>>>> On 04/19/2018 12:01 PM, Thomas Schatzl wrote:
>>>>>> Hi all,
>>>>>>
>>>>>>      can I have reviews for this cleanup change that improves
>>>>>> variable
>>>>>> naming in ReferenceProcessor code.
>>>>>>
>>>>>> The main change is about renaming some member variables from
>>>>>> something
>>>>>> really confusing to me to something useful (to me again :)) in
>>>>>> the
>>>>>> DiscoveredListIterator class.
>>>>>>
>>>>>> The other change is related to changing
>>>>>> ReferenceProcessor::num_q
>>>>>> to
>>>>>> num_queues, i.e. spelling out what the "q" is supposed to mean.
>>>>>>
>>>>>> That imho improves readability quite a bit.
>>>>>>
>>>>>> CR:
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8202021
>>>>>> Webrev:
>>>>>> http://cr.openjdk.java.net/~tschatzl/8202021/webrev/
>>> Thanks for doing this, having better names really help when working
>>> with this code. The change looks ok, but I still struggle a bit with
>>> what all members are. I think my problem is seeing that _ref and
>>> _discovered_addr go together. What do you think about renaming:
>>> _ref -> _current
>>> _discovered_addr -> _current_discovered_addr
>> Done.
>>
>>> Potentially also:
>>> _referent -> _current_referent
>>> _referent_addr -> _current_referent_addr
>>>
>> I think the latter renaming is unnecessary as there is no
>> prev/current/next distinction.
>>
>> http://cr.openjdk.java.net/~tschatzl/8202021/webrev.1/ (full)
>> http://cr.openjdk.java.net/~tschatzl/8202021/webrev.0_to_1/ (diff)
> webrev.1 still looks good.
Thanks for doing the additional changes, looks good!

Cheers,
Stefan

> 
> Thanks,
> Sangheon
> 
> 
>>
>> Thanks,
>>    Thomas
> 


More information about the hotspot-gc-dev mailing list