On Thu, 26 Aug 2021 04:27:36 GMT, Lin Zang <lz...@openjdk.org> wrote:
>> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/utilities/HeapHprofBinWriter.java
>> line 1367:
>>
>>> 1365: @Override
>>> 1366: public synchronized void write(int b) throws IOException {
>>> 1367: if (segmentMode && !unbufferedMode) {
>>
>> It is not clear why the condition` !unbufferedMode` is additionally checked
>> here and several places below. The `SegmentedOutputStream` constructor
>> always sets `false` to the`unbufferedMode` field at L1344. Is it possible
>> that `segmentMode` can be also unbuffered?
>
> The `unbufferedMode` is set to true at L1498, and it is use when writing
> array data. The purpose is to write the data directly to the file instead of
> buffering them first, it can avoid memory copy and also avoid timeout when
> dumping large array (because the buffer must contains the whole segment
> before it is written to file, to avoid data pollution for compressed dump,
> and if the buffer is not enough, a memory copy is conducted)
Okay, thanks.
>> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/utilities/HeapHprofBinWriter.java
>> line 1580:
>>
>>> 1578: private boolean allowSegmented;
>>> 1579: // Write data directly to underlying stream. Don't use
>>> internal buffer.
>>> 1580: private boolean unbufferedMode;
>>
>> I would suggest replace `unbufferedMode` with `bufferedMode` field. It will
>> simplify logic a little bit.
>> Then you will need to rename the method
>> `fillSegmentSizeAndEnableUnbufferedMode` to be
>> `fillSegmentSizeAndDisableBufferedMode`.
>> There are several places where `!unbufferedMode` will be replaced with
>> `bufferedMode` .
>
> Good idea, I will made the change. Thanks.
Lin, I'm okay with your fix in general and will approve it after check of your
change for the above suggestion.
-------------
PR: https://git.openjdk.java.net/jdk/pull/2803
