Request for review: 8010506 Typos and errors in gc-related flags in globals.hpp
Mikael Vidstedt
mikael.vidstedt at oracle.com
Wed Oct 2 23:40:58 UTC 2013
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