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

[Kevin Walls]

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

Updated.
Additionally, the total_length check at line 2760 is wrong now.  But it is also redundant, we  use copy_expand_pid to do our length checks on expansion.  Use max_digit_chars to reduce buffer length in those copy_expand_pid calls, to leave room for possible later sequence numbers (this is very conservative).

On longer path lengths, worth noting that using MAXPATHLEN (4k) is higher than outputStream::print_cr allows.
This means we can get through all of  HeapDumper::dump_heap(bool oome) and call  HeapDumper::dump() which uses:
 2606     out->print_cr("Dumping heap to %s ...", path);
..and will show a VM warning like "outputStream::do_vsnprintf output truncated"

Again, very very long HeapDumpPaths are not efficient. 8-)

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

PR Comment: https://git.openjdk.org/jdk/pull/24482#issuecomment-2786165819