Request for review: 6941923: RFE: Handling large log files produced by long running Java Applications

[yumin.qi at oracle.com]

  Thanks!

Yumin
On 5/3/2011 2:04 AM, Jesper Wilhelmsson wrote:
> Looks good.
> /Jesper
>
> On 05/03/2011 01:56 AM, yumin.qi at oracle.com wrote:
>> Hi, Jesper and all
>>
>> This is third version, removed the flag GCLogFile and related code 
>> changes.
>> Also restore the code inside ostream_init_log.
>> http://cr.openjdk.java.net/~minqi/6941923/webrev.03
>>
>> Thanks
>> Yumin
>>
>> On 5/2/2011 8:33 AM, Jesper Wilhelmsson wrote:
>>> Yumin,
>>>
>>> OK. I think a comment in both places mentioned below would be great 
>>> since it
>>> is code that may make sense in the future, but does not right now.
>>>
>>> I'm personally not a fan of checking in code that may be needed in the
>>> future, the risk of code rot is overwhelming. But if it is decided 
>>> that the
>>> log file name should be changeable fairly soon, I have no objections 
>>> in this
>>> case.
>>>
>>> What kind of testing have you done to make sure everything works?
>>> Is there any new tests for this feature that should be added to some 
>>> test
>>> suite?
>>> /Jesper
>>>
>>>
>>> On 05/02/2011 05:14 PM, yumin.qi at oracle.com wrote:
>>>> Jesper,
>>>>
>>>> On 5/2/2011 7:05 AM, Jesper Wilhelmsson wrote:
>>>>> Yumin,
>>>>>
>>>>> Took a closer look this time and noticed that you introduce a 
>>>>> fourth new
>>>>> flag in addition to the three mentioned in the CR, -XX:GCLogFile. 
>>>>> I have to
>>>>> admit that I like the new name better than the old -Xloggc, but do 
>>>>> we really
>>>>> want to introduce a new flag with identical behavior as the old?
>>>>>
>>>> In case we turn this flag into manageable, that is, we supply 
>>>> interface in
>>>> JVMTI, it can be changed outside. Currently it is only a product 
>>>> flag. The log
>>>> file name cannot be changed currently but future it should be 
>>>> changeable via
>>>> JVMTI if debugger tools wants to change log rotation and file name.
>>>>
>>>>> If we can deprecate the old flag and remove it in a few releases I 
>>>>> would be
>>>>> happy to endorse the new flag, but I suspect that -Xloggc is quite 
>>>>> heavily
>>>>> used in production environments.
>>>>>
>>>>>
>>>>> I am a bit puzzled by a change in ostream.cpp, in ostream_init_log():
>>>>>
>>>>> 807 if (gclog_or_tty != NULL && gclog_or_tty != tty) {
>>>>> 808 delete gclog_or_tty;
>>>>> 809 }
>>>>>
>>>> Yes, you are right. I add this part to prevent if it is already 
>>>> init'ed ---
>>>> now it is only initailized once in vm creation. This function is 
>>>> only called
>>>> once at present. With log name changeable, it can be called multiples.
>>>>
>>>> Thanks
>>>> Yumin
>>>>> Why is this needed? As far as I can tell gclog_or_tty will never 
>>>>> have a
>>>>> value here, the only assignment to that variable is made on the 
>>>>> next line in
>>>>> the same function and the function will only be called once during
>>>>> initialization of the jvm. Have you seen cases where this delete 
>>>>> is executed?
>>>>> /Jesper
>>>>>
>>>>>
>>>>> On 04/29/2011 07:15 PM, yumin.qi at oracle.com wrote:
>>>>>> Jesper,
>>>>>>
>>>>>> Thanks. Deleted the comments part, this is the new version:
>>>>>>
>>>>>> http://cr.openjdk.java.net/~minqi/6941923/webrev.02
>>>>>>
>>>>>> Thanks
>>>>>> Yumin
>>>>>>
>>>>>> On 4/29/2011 5:25 AM, Jesper Wilhelmsson wrote:
>>>>>>> Yumin,
>>>>>>>
>>>>>>> In ostream.hpp lines 199 - 215 you have added a block of code 
>>>>>>> that is
>>>>>>> commented out. Personally I don't think we should have code that is
>>>>>>> commented out in there unless there is a good documentation 
>>>>>>> reason for
>>>>>>> it. I
>>>>>>> don't see such a reason here.
>>>>>>>
>>>>>>> Looks good otherwise.
>>>>>>> /Jesper
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 04/28/2011 11:18 PM, yumin.qi at oracle.com wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> Need your review on the second time changes:
>>>>>>>>
>>>>>>>> http://cr.openjdk.java.net/~minqi/6941923/webrev.01
>>>>>>>>
>>>>>>>> Any comments on the revised version? thanks in advance.
>>>>>>>>
>>>>>>>> Yumin
>>>>>>>>