RFR: 8353727: HeapDumpPath doesn't expand %p [v2]

[Leonid Mesnik]

On Tue, 8 Apr 2025 11:43:20 GMT, Kevin Walls <kevinw at openjdk.org> wrote:

>> This is a long-standing oversight: HeapDumpPath does not recognise %p for pid expansion.
>> The default filename uses a pid (e.g. java_pid1676937.hprof) but HeapDumpPath does not.
>> It has always done a manual "root plus pid plus extension" on the default filename only, and
>> should move to using Argument::copy_expand_pid() like we do with other such filenames.
>> 
>> 
>> We also assumed the default filename is not a directory (which is very very likely, but doesn't have to be true).
>
> Kevin Walls has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - length checking update
>  - Chris feedback

Marked as reviewed by lmesnik (Reviewer).

test/hotspot/jtreg/runtime/ErrorHandling/TestHeapDumpOnOutOfMemoryError.java line 100:

> 98:         output.shouldContain("Dumping heap to " + type + ".hprof");
> 99:         File dump = new File(heapdumpFilename);
> 100:         Asserts.assertTrue(dump.exists() && dump.isFile(), "Could not find dump file " + dump.getAbsolutePath());

I. think you could just update the test to use
heapdumpFilename = type + ".%p.hprof";
we don't need test twice, it is quite expensive.

test/hotspot/jtreg/runtime/ErrorHandling/TestHeapDumpOnOutOfMemoryError.java line 115:

> 113:         output.stdoutShouldNotBeEmpty();
> 114:         String actualHeapdumpFilename = type + "." + output.pid() + ".hprof";
> 115:         output.shouldContain("Dumping heap to " + actualHeapdumpFilename);

This better to be something like expectedlHeapdumpFilename and 
"Expected heap dump file".

Not very important, but make log cleaner.

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

PR Review: https://git.openjdk.org/jdk/pull/24482#pullrequestreview-2751644078
PR Review Comment: https://git.openjdk.org/jdk/pull/24482#discussion_r2034187020
PR Review Comment: https://git.openjdk.org/jdk/pull/24482#discussion_r2034189152