Request for review: 8010506 Typos and errors in gc-related flags in globals.hpp

Jesper Wilhelmsson jesper.wilhelmsson at oracle.com
Wed Oct 2 14:16:44 UTC 2013


Looks good!
Ship it!
/Jesper

Tao Mao skrev 2/10/13 3:00 AM:
> 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