RFR (S): 8202021: Improve variable naming in ReferenceProcessor
Stefan Johansson
stefan.johansson at oracle.com
Mon Apr 23 14:02:15 UTC 2018
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
Potentially also:
_referent -> _current_referent
_referent_addr -> _current_referent_addr
Cheers,
Stefan
>>> Testing:
>>> local compilation, hs-tier1-4 with 8202017
>>>
>>> based on JDK-8202018, but I do not think there is overlap.
>>>
>>> Thanks,
>>> Thomas
>>>
>>
>> Looks good.
>> Just minor nits, so don't need extra webrev for these.
>>
>> /src/hotspot/share/gc/shared/referenceProcessor.cpp
>> 112 _discovery_is_mt = mt_discovery;
>> 113 _num_queues = MAX2(1U, mt_processing_degree);
>> 114 _max_num_queues = MAX2(_num_queues,
>> mt_discovery_degree);
>> - Align with line 112?
>>
>> 312 log_develop_trace(gc, ref)(" obj " INTPTR_FORMAT
>> "/next_d
>> " INTPTR_FORMAT, p2i(obj), p2i(next_discovered));
>> - next_d -> next_discovered
>>
>> src/hotspot/share/gc/shared/referenceProcessor.hpp
>> - Copyright year update
>>
>> src/hotspot/share/gc/shared/referenceProcessor.inline.hpp
>> - Copyright year update
>>
>> Thanks,
>> Sangheon
>>
>>
>>
>>
>
More information about the hotspot-gc-dev
mailing list