On Fri, 5 Apr 2024 22:47:55 GMT, Alex Menkov <amen...@openjdk.org> wrote:
> I don't have strong opinion if it's good to have default file path for `jcmd > GC.heap_dump`, just some thoughts. We have several ways to heap dump: > > 1. `-XX:+HeapDumpOnOutOfMemoryError` > 2. `-XX:+HeapDumpBeforeFullGC`, `-XX:+HeapDumpAfterFullGC` > in the case `HeapDumpPath` and `HeapDumpGzipLevel` are used (this is not > mentioned in the options' description, need to fix it) Was that ever a problem raised by users that the OOME and the GC related heap dumps end up at the same location ? > 3. `jcmd GC.heap_dump` > 4. `jmap -dump` > it uses "dumpheap" Attach operation, implemented by AttachListener directly > 5. `HotSpotDiagnosticMXBean.dumpHeap()`; > > Current patch looks inconsistent: If HeapDumpPath is used as default, > HeapDumpGzipLevel should be used as default too (note that default path has > different extension depending on HeapDumpGzipLevel); If (3) has defaults, why > (4) and (maybe) (5) don't have the same defaults? Yes , probably with 1. + 2, and now (with the patch) 3. using HeapDumpPath the others should use it most likely too. (but our users just asked for 3. that's why the patch). About HeapDumpGzipLevel , I could add this (I think this was already mentioned), but before adding it I wanted to be sure that there the change is supported. ------------- PR Comment: https://git.openjdk.org/jdk/pull/18190#issuecomment-2041981493
