On Wed, 29 Nov 2023 20:14:07 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:
>> Alex Menkov has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - feedback
>> - prepare_parallel_dum
>
> src/hotspot/share/services/heapDumper.cpp line 1886:
>
>> 1884: };
>> 1885:
>> 1886: // Support class using when iterating over the heap.
>
> Suggestion:
>
> // Support class used when iterating over the heap.
Fixed
> src/hotspot/share/services/heapDumper.cpp line 2134:
>
>> 2132: for (int i = 0; i < _dump_seq; i++) {
>> 2133: ResourceMark rm;
>> 2134: const char* path = get_writer_path(_path, i);
>
> It's not clear to me why you've chosen to move from a JVM_MAXPATHLEN stack
> buffer to a dynamically allocated one.
It's better to have a single place to generate path of the segment file, so the
fix introduced DumpMerger::get_writer_path()
And theoretically the path can be longer than JVM_MAXPATHLEN
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16665#discussion_r1410021293
PR Review Comment: https://git.openjdk.org/jdk/pull/16665#discussion_r1410020829
