RFR: 8025856 - Fix typos in the GC code (original) (raw)

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