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

[Jesper Wilhelmsson]

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
>>>>>