On Wed, 29 May 2024 02:03:42 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:
>> The fix updates HeapMerger to use writer buffer (no need to copy memory,
>> also writer buffer is 1MB instead of 4KB).
>> Additionally fixed small issue in FileWriter (looks like `ssize_t` instead
>> of `size_t` is a typo, the argument should be unsigned)
>>
>> Testing: all HeapDump-related tests on Oracle supported platforms
>
> src/hotspot/share/services/heapDumper.cpp line 2137:
>
>> 2135: while ((cnt = segment_fs.read(_writer->buffer(), 1,
>> _writer->buffer_size())) != 0) {
>> 2136: _writer->set_position(cnt);
>> 2137: _writer->flush();
>
> Why flush on each iteration instead of just once after you are done with the
> loop?
Standard way to use AbstractDumpWriter is to use
`AbstractDumpWriter::write_XXX` methods (they all call `write_raw(const void*
s, size_t len)` which copies data from the provided memory to
`_writer->buffer()`, and flushes when the buffer is full)
This code uses writer internals - on each iteration we read up to 1MB from
`segment_fs` directly to `_writer->buffer()`, so we need to flush it before
performing next read, otherwise data in the buffer will be overridden.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18850#discussion_r1619412072
