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

Thomas Schatzl thomas.schatzl at oracle.com
Mon Apr 23 14:33:05 UTC 2018


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)

Thanks,
  Thomas


More information about the hotspot-gc-dev mailing list