On Wed, 12 May 2021 11:11:39 GMT, Lin Zang <lz...@openjdk.org> wrote:
>> 8262386: resourcehogs/serviceability/sa/TestHeapDumpForLargeArray.java timed >> out > > Lin Zang has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains 12 additional commits since > the last revision: > > - Merge branch 'master' into s-fix > - fix typo in comments > - Merge branch 'master' into s-fix > - Merge branch 'master' > - Merge branch 'master' into sf > - rename writeThrough to unbufferedMode and code refine > - fix typo in comments > - Merge branch 'master' into sf > - Revert "reduce memory consumption" > > This reverts commit 70e43ddd453724ce36bf729fa6489c0027957b8e. > - reduce memory consumption > - ... and 2 more: > https://git.openjdk.java.net/jdk/compare/7dae89e1...6fde07bd Overall looks good. I just have one question and a couple minor comment suggestions. Also, can you do the same extra testing that you did for #2261 src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/utilities/HeapHprofBinWriter.java line 1510: > 1508: */ > 1509: private boolean shouldFlush() { > 1510: // unbuffered mode always flush data. "flushes data" src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/utilities/HeapHprofBinWriter.java line 1528: > 1526: } > 1527: writeInteger(0); > 1528: writeInteger(size); You've added `writeInteger(size)` here, but I'm not sure where this used to be done. src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/utilities/HeapHprofBinWriter.java line 1577: > 1575: private boolean segmentMode; > 1576: private boolean allowSegmented; > 1577: // Writes data directly to underlying stream, don't use > internal buffer. It should be "Write data". I noticed you recently changed it to "Writes", but I'm not sure why. Also, make this two sentences rather than using a comma. ------------- Changes requested by cjplummer (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2803
