JDK-7090324: gclog rotation via external tool (original) (raw)

Yasumasa Suenaga yasu at ysfactory.dip.jp
Wed Jan 29 14:28:16 UTC 2014

Hi Staffan,

Thank you for reviewing! I've uploaded new webrev. http://cr.openjdk.java.net/~ysuenaga/JDK-7090324/webrev.01/

On 2014/01/29 20:56, Staffan Larsen wrote:

Yasumasa,

src/share/vm/runtime/arguments.cpp no comments src/share/vm/runtime/safepoint.cpp I was surprised that gc log size was checked after each safe point. That seems an uneccssary burden to place on a safe point. Instead we should switch to a periodic task that checks the gc log size. However, this is unrelated to you patch, so please ignore for now.

Agree. However, I think that PeriodicTask also is not appropriate for this.

Size of GC log file is increased when GC is occurred. So I think rotate function should be called at entry of each GC events e.g. VM_GC_Operation::doit_prologue() etc...

src/share/vm/runtime/vmoperations.hpp line 402: nit: missing space before {

Fixed.

line 405: I think ‘force’ is a better name than ‘isforce’

I removed "force" option from DCmd. So I removed this from VMOperation.

src/share/vm/services/diagnosticCommand.cpp line 666: What does this do without the -force option? It looks to me that the non-force case will happen after each safe point (see above) and that there is no need to ever do this from a diagnostic command. Can we remove the option?

Indeed. I removed "force" option.

line 677: “Target VM does not support GC log file rotation."

Fixed.

nits: some missing spaces before ‘{' and after ‘if'

Fixed.

src/share/vm/services/diagnosticCommand.hpp I think RotateGCLogDCmd should require the “control” permission when executed via JMX, so please add: static const JavaPermission permission() { JavaPermission p = {"java.lang.management.ManagementPermission", "control", NULL}; return p; }

Added.

line 394: Maybe “Force the GC log file to be rotated.” is a better description?

Fixed.

src/share/vm/utilities/ostream.cpp line 662: I think ‘force’ is a better name than ‘isforce’ line 668: The comment says exactly the same thing as the code so I think it can be skipped line 671: “GC log file rotation occurs by external trigger ONLY." line 675: "not need” -> “no need” line 718: "GC log rotation request has been received”

Fixed them.

Thanks,

Yasumasa

src/share/vm/utilities/ostream.hpp no comments

Thanks, /Staffan On 24 jan 2014, at 14:50, Yasumasa Suenaga<yasu at ysfactory.dip.jp> wrote: Hi all,

I've created webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-7090324/webrev.00/ This patch works fine on current jdk9/hs-rt . Could you review this?

I am just an Author. So I need a sponsor. Could you help me? Please cooperate. Thanks, Yasumasa On 2013/12/06 0:05, Yasumasa Suenaga wrote: Hi all, Did someone read my email? I really hope to merge "JDK-7090324: gclog rotation via external tool" . I hear that someone need this RFE. So I want to discuss about this.

Thanks, Yasumasa On 2013/11/08 21:47, Yasumasa Suenaga wrote: Hi all, Did someone read my mail? I think that this RFE helps us to watch Java heap on production system. Also I think this RFE is able to be part of the JEP 158 (Unified JVM Logging) . I want to update this RFE in JDK Bug System, but I don't have account. So I've posted email at first.

Thanks, Yasumasa On 2013/09/30 21:10, Yasumasa Suenaga wrote: In previous email, I've attached new patch for this RFE. It works fine with current hsx.

Yasumasa On 2013/09/29 23:40, Yasu wrote: Hi all, We are using "logrotate" tool on RHEL for various log rotation. Current HotSpot has gclog rotation function for log size base, however I need to rotate gc log synchronizing with logrotate tool. So I've created RFE as "JDK-7090324: gclog rotation via external tool" . And Sr. Engineering Manager in Oracle said he use the essence of my patch in one of the jcmd subcommands. http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2011-September/003274.html 2 years ago, I posted a patch for this RFE. But this patch is too old to apply for current HotSpot. In last month, a similar discussion was appeared in ML. So I think it's time to discuss this RFE. http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2013-August/008029.html

Please cooperate. Best regards, Yasumasa

More information about the hotspot-gc-dev mailing list