Request for review: 8010506 Typos and errors in gc-related flags in globals.hpp
Jesper Wilhelmsson
jesper.wilhelmsson at oracle.com
Wed Oct 2 23:54:31 UTC 2013
Hi Tao,
I noticed that in a few places there is a space between the description string
and the right parenthesis. E.g.
product(intx, SyncVerbose, 0, "(Unstable)" )
Search for '" )' and you will find a few.
There is also a capital S in "LAB'S" in the description of
CMSParPromoteBlocksToClaim.
I didn't mention these before since I really want this to be pushed asap, but if
you decide to act on Mikael's comments about Print vs Prints maybe you can fix
these minor things as well?
I don't think a new webrev is needed for these changes.
Thanks,
/Jesper
Mikael Vidstedt skrev 3/10/13 1:40 AM:
>
> Tao,
>
> I sampled the changes and this is definitely a great clean up!
>
> Question: Do you want to attack inconsistencies like "Print" vs "Prints" etc.
> since you're changing so much of this anyway?
>
> For example I see that you changed this:
>
> product(bool, ProfileIntervals, false, \
> - "Prints profiles for each interval (see ProfileIntervalsTicks)") \
> + "Print profiles for each interval (see ProfileIntervalsTicks)") \
>
>
> But there's still this:
>
> develop(bool, TraceClearedExceptions, false, \
> "Prints when an exception is forcibly cleared") \
>
>
> and this:
>
> 3237 develop(intx, TraceBytecodesAt, 0, \
> 3238 "Traces bytecodes starting with specified bytecode number") \
>
> 3596 develop(bool, PerfTraceDataCreation, false, \
> 3597 "Trace creation of Performance Data Entries") \
>
> Cheers,
> Mikael
>
> On 2013-10-01 18:00, Tao Mao wrote:
>> Hi all,
>>
>> Jesper's suggestion is integrated.
>>
>> new webrev: "Frame" seems to be the best way to review it.
>>
>> http://cr.openjdk.java.net/~tamao/8010506/webrev.03/
>>
>> Please review it so that I can get it in before ZBB.
>>
>> Thanks.
>> Tao
>>
>> On 9/24/13 12:47 PM, Jesper Wilhelmsson wrote:
>>> Hi Tao,
>>>
>>> The change looks good. Thanks for fixing this!
>>>
>>> Purely esthetic's:
>>> Lines 1820, 1821, 2221 and 2308 in your new globals.hpp needs their \ aligned.
>>>
>>> Since you are touching pretty much the whole file there are an inconsistency
>>> that would be nice to get rid of once and for all. The following flags have
>>> their descriptions end with a period although they are only one sentence.
>>> Most descriptions do not have the period at the end.
>>>
>>> TracePageSizes
>>> InlineMathNatives
>>> ShowSafepointMsgs
>>> MaxFDLimit
>>> PredictedLoadedClassCount
>>> ParallelOldDeadWoodLimiterMean
>>> ParallelOldDeadWoodLimiterStdDev
>>> ScavengeWithObjectsInToSpace
>>> UseParNewGC
>>> ParallelGCVerbose
>>> ParallelGCBufferWastePct
>>> ParallelGCRetainPLAB
>>> PLABWeight
>>> CMSParPromoteBlocksToClaim
>>> OldPLABWeight
>>> AlwaysPreTouch
>>> CMSExpAvgFactor
>>> CMS_FLSWeight
>>> CMS_FLSPadding
>>> CMS_SweepPadding
>>> CMSDumpAtPromotionFailure
>>> CMSPrintChunksInDump
>>> CMSPrintObjectsInDump
>>> CMSVerifyReturnedBytes
>>> ExecuteInternalVMTests
>>> ConcGCYieldTimeout
>>> PrintParallelOldGCPhaseTimes
>>> ObjectCountCutOffPercent
>>> AbortVMOnExceptionMessage
>>> MaxBCEAEstimateLevel
>>> MaxBCEAEstimateSize
>>> CodeCacheMinimumFreeSpace
>>> CodeCacheMinBlockLength
>>> ExitOnFullCodeCache
>>> Tier0InvokeNotifyFreqLog
>>> Tier2InvokeNotifyFreqLog
>>> Tier3InvokeNotifyFreqLog
>>> Tier0BackedgeNotifyFreqLog
>>> Tier2BackedgeNotifyFreqLog
>>> Tier3BackedgeNotifyFreqLog
>>> Tier3CompileThreshold
>>> Tier4CompileThreshold
>>> IncreaseFirstTierCompileThresholdAt
>>> UsePerfData
>>> PerfDataMemorySize
>>> DumpSharedSpaces
>>>
>>>
>>> Thanks!
>>> /Jesper
>>>
>>>
>>> Tao Mao skrev 27/6/13 7:05 PM:
>>>> new webrev: "Frame" seems to be the best way to review it.
>>>>
>>>> http://cr.openjdk.java.net/~tamao/8010506/webrev.02/
>>>>
>>>> Suggestion are taken mostly from Thomas.
>>>>
>>>> Thanks.
>>>> Tao
>>>>
>>>> On 5/14/13 2:46 PM, Jon Masamitsu wrote:
>>>>>
>>>>> On 4/30/13 4:58 PM, Tao Mao wrote:
>>>>>> new webrev
>>>>>> http://cr.openjdk.java.net/~tamao/8010506/webrev.01/
>>>>>>
>>>>>> It needs reviews.
>>>>>
>>>>> Tao,
>>>>>
>>>>> Changes you made look good. I did not see a list of the
>>>>> suggested changes in the CR
>>>>>
>>>>> https://jbs.oracle.com/bugs/browse/JDK-8010506
>>>>>
>>>>> so don't have any comment about what you decided not
>>>>> to change.
>>>>>
>>>>>>
>>>>>> Changeset:
>>>>>> Thank you for the thorough report of all these typos/errros, Thomas.
>>>>>>
>>>>>> I changed most places according to your suggestions (for the rest few, I did
>>>>>> not change because it may slightly change the meaning of the definition).
>>>>>> Also, I went through the file to change some similar common errors that
>>>>>> occurred.
>>>>>>
>>>>>> Several issues:
>>>>>>
>>>>>> (1) parallel gc should be lower case or upper case?
>>>>>> As I know and, perhaps, someone told me, the term "parallel gc" sometimes
>>>>>> refers to a more generic terminology rather than ParallelGC or ParallelOldGC,
>>>>>> if it doesn't imply these two. Basically, any GC algorithms that can be
>>>>>> parallelized could fall into this category. Right now, we probably refer
>>>>>> "parallel gc" to ParallelGC or ParallelOldGC. But, we reserve the right to
>>>>>> change it to something else. That's why "fuzziness" exists here. I guess the
>>>>>> same way to deal with "concurrent gc". So, I didn't make these words upper
>>>>>> cased.
>>>>>>
>>>>>> (2) Identification:
>>>>>> The general rule is to follow the example (product()'s identification). The
>>>>>> whole file checked and space adjusted accordingly.
>>>>>>
>>>>>> (3) Line breaks
>>>>>> A space should be provided at the line breaks if multiple lines is needed.
>>>>>> Some lines that are slightly longer than normal length is kept untouched as
>>>>>> long as they read well.
>>>>>>
>>>>>> *(4) Why does the VM option hashCode have lower case initial?
>>>>>> The direct answer is "I don't know". (But it is understandable that hashCode
>>>>>> is "legacy" such that it has it own style) Anyone knows the answer?
>>>>>
>>>>> I don't know for sure either but might be because of the Java code
>>>>>
>>>>> public int*hashCode*()
>>>>>
>>>>>
>>>>> Jon
>>>>>
>>>>>>
>>>>>> Thanks.
>>>>>> Tao
>>>>>>
>>>>>> On 3/21/13 3:09 PM, Tao Mao wrote:
>>>>>>> 8010506 Typos and errors in gc-related flags in globals.hpp
>>>>>>> https://jbs.oracle.com/bugs/browse/JDK-8010506
>>>>>>>
>>>>>>> webrev:
>>>>>>> http://cr.openjdk.java.net/~tamao/8010506/webrev.00/
>>>>>>>
>>>>>>> Changeset:
>>>>>>> NewRatio should defined as
>>>>>>> product(uintx, NewRatio, 2, \
>>>>>>> "Ratio of old/new generation sizes") \
>>>>>>> instead of
>>>>>>> product(uintx, NewRatio, 2, \
>>>>>>> "Ratio of new/old generation sizes") \
>>>>>>>
>>>>>>> PLUS other typos found by Jesper.
>>>>>
>
More information about the hotspot-gc-dev
mailing list