RFR: 8025856 - Fix typos in the GC code

John Coomes John.Coomes at oracle.com
Thu Jan 23 02:50:47 UTC 2014


Jesper Wilhelmsson (jesper.wilhelmsson at oracle.com) wrote:
> Hi Thomas,
> 
> Thanks for looking at this!
> 
> I have fixed the things you pointed out. The webrev is updated (same link). I 
> mainly fixed spelling errors in the runtime code to minimize the diff. I have 
> fixed a few capitalizations but I didn't go all-in as I did in some GC files :-)

Hi Jesper,

Found just a few things:

gc_implementation/shared/mutableNUMASpace.cpp:

   // so an object can cross the chunk boundary. We ensure the parseability

   parseability -> parsability

opto/runtime.cpp:

   fields[TypeFunc::Parms+0] = TypeInt::INT; // trap_reason (deopt reason and action)

   The comment you added is not obviously correct; did someone from
   the compiler team check this?  If the comment is correct, the code
   is crazily obscure!

runtime/frame.cpp:

   // Return whether the frame is in the VM or os indicating a HotSpot problem
      ^^^^^                                   ^^^

   Change Return -> Print (the method doesn't return anything).  Also
   Change os -> O/S.

General:

   The addition/removal of periods ('.') isn't totally consistent, but
   please go ahead with what you have.

   This kind of thing would be helped by a rule/guideline like like
   "use a period only after a sentence (i.e., if it has a subject and
   a verb)."  But that's a discussion for another day.

-John

> Thomas Schatzl skrev 13/1/14 2:42 PM:
> > On Wed, 2014-01-08 at 16:37 +0100, Jesper Wilhelmsson wrote:
> >> Hi,
> >>
> >> Anyone up for a really tedious review?
> >>
> >> This change fixes about 300 typos in comments in the GC code. There should only
> >> be changes in comments in this change.
> >>
> >> I had this out for review a while ago but it was a bit late in 8u20 to push a
> >> large change like this. I got a few comments last time that have been fixed in
> >> this new version.
> >>
> >> This new webrev is based on jdk9/hs-gc and once approved it will be backported
> >> to jdk8 (to minimize the diff between jdk8 and jdk9).
> >>
> >> Let me know if you have some change out that you want to push before I push this
> >> change. The risk of merge conflicts is high since the change touches a lot of code.
> >>
> >> Bug: https://bugs.openjdk.java.net/browse/JDK-8025856
> >>
> >> Webrev: http://cr.openjdk.java.net/~jwilhelm/8025856/webrev.1/
> >>
> >
> > Looks good.
> >
> > A few additional nitpicks:
> >
> > concurrentMark.hpp, line 443: // true: -> // True:
> >
> > g1CollectorPolicy.cpp, line 1078: // do that -> // Do that
> >
> > adaptiveSizePolicy.hpp: there are a lot of initial lower case letters in
> > descriptions beginning from around line 150
> > arguments.cpp, line 181:   // following are JVMTI a -> // Following
> > are...
> >
> > arguments.cpp, line 3679:   // which are subtlety different from each
> > other but neither works with
> >
> > "Subtlety" is the noun, I think the adjective is required here (subtly)
> >
> > mutex.cpp, line 966: I think it is okay to remove the [sic] comment -
> > [sic] comments on the wrong spelling of some word, and since now the
> > spelling of the word the [sic] refers to is gone, this should be removed
> > as well. Or I just did not get the joke :)
> >
> > It should be "succeeds" too (thread is third person singular)
> >
> > mutexLocker.hpp, line 348: // Only too be used -> // Only to be used
> >
> > safepoint.hpp, line 158: voluntary -> voluntarily
> >
> > I saw that in runtime/compiler code there were many cases where the
> > First letter after the comment uses lowercase (I mentioned a few above
> > though). Feel free to apply them or not depending on what the
> > runtime/compiler team thinks.
> >
> > Otherwise looks good.
> >
> > Thomas
> >
> >

-- 
John Coomes                            Oracle, MS USCA22-3??
john.coomes at oracle.com                 4220 Network Circle
408-276-7048                           Santa Clara, CA 95054-1778
	 *** Support GreenPeace and we'll all breathe easier. ***



More information about the hotspot-gc-dev mailing list