RFR: 8204681: Option to include timestamp in hprof filename

[Kevin Walls]

On Tue, 13 Aug 2024 15:07:17 GMT, Sonia Zaldana Calles <szaldana at openjdk.org> wrote:

> Hi all, 
> 
> This PR addresses [8204681](https://bugs.openjdk.org/browse/JDK-8204681) enabling support for timestamp expansion in filenames specified in `-XX:HeapDumpPath` using `%t`. 
> 
> As mentioned in this comments for this issue, this is somewhat related to [8334492](https://bugs.openjdk.org/browse/JDK-8334492) where we enabled support for `%p` for filenames specified in jcmd. 
> 
> With this patch, I propose: 
> - Expanding the utility function `Arguments::copy_expand_pid` to `Arguments::copy_expand_arguments` to deal with `%p` expansions for pid and `%t` expansions for timestamps. 
> - Leveraging the above utility function to enable argument expansion for both heap dump filenames and jcmd output commands. 
> - Though the linked JBS issue only relates to heap dumps generated in case of OOM, I think we can edit it to more broadly support filename expansion to support `%t` for jcmd as well. 
> 
> Testing: 
> - [x] Added test cases pass with all platforms (verified with a GHA job). 
> - [x] Tier 1 passes with GHA. 
> 
> Looking forward to hearing your thoughts!
> 
> Thanks, 
> Sonia

Hi,
Yes, agreed with notes above that current time is the timestamp that is useful.
(Multiple files from the same VM are created with the same filename pattern, and file created is different over time.  Good.  And if the pid is the same, timestamp keeps things "more" unique.)
That means we should not need any changes relating to start_time = create_vm_timer...? ( src/hotspot/share/runtime/threads.cpp )

I'd imagined a raw numerical timestamp, to keep things simple, the example in JDK-8204681 was like that.
However src/hotspot/share/utilities/ostream.cpp has make_log_name() which says "%t => YYYY-MM-DD_HH-MM-SS" so maybe we could follow that pattern.  As long as it's clear, and we'll be doing help/man page updates also.

JDK-8204681 was logged originally about -XX:HeapDumpPath but yes all of our output filenames from diagnostic commands would ideally support this.

FYI, in JDK-8338603 I will try and clear up the "FILE" type confusion.  It is not easy for a JMX client to know that implementation-dependent types may exist.

I can come back for a proper review soon.

-------------

PR Review: https://git.openjdk.org/jdk/pull/20568#pullrequestreview-2254201847