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