On Tue, 13 Aug 2024 15:07:17 GMT, Sonia Zaldana Calles <szald...@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 test/hotspot/jtreg/runtime/ErrorHandling/TestHeapDumpFilenameExpansion.java line 53: > 51: try { > 52: Object[] oa = new Object[Integer.MAX_VALUE]; > 53: for(int i = 0; i < oa.length; i++) { Suggestion: for (int i = 0; i < oa.length; i++) { test/hotspot/jtreg/runtime/ErrorHandling/TestHeapDumpFilenameExpansion.java line 90: > 88: Pattern pattern = > Pattern.compile("file\\d{4}-\\d{2}-\\d{2}_\\d{2}-\\d{2}-\\d{2}"); > 89: File[] files = new File(".").listFiles(); > 90: if(files != null) { Suggestion: if (files != null) { test/jdk/sun/tools/jcmd/TestJcmdArgumentSubstitution.java line 87: > 85: Pattern pattern = > Pattern.compile("myfile\\d{4}-\\d{2}-\\d{2}_\\d{2}-\\d{2}-\\d{2}"); > 86: File[] files = new File(test_dir).listFiles(); > 87: if(files != null) { Suggestion: if (files != null) { ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/20568#discussion_r1732794735 PR Review Comment: https://git.openjdk.org/jdk/pull/20568#discussion_r1732794999 PR Review Comment: https://git.openjdk.org/jdk/pull/20568#discussion_r1732795548
