RFR: 8262386: resourcehogs/serviceability/sa/TestHeapDumpForLargeArray.java timed out [v7]

[Lin Zang]

On Fri, 2 Apr 2021 19:43:31 GMT, Chris Plummer <cjplummer at openjdk.org> wrote:

>> 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 eight additional commits since the last revision:
>> 
>>  - 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
>>  - code refine
>>  - 8262386: resourcehogs/serviceability/sa/TestHeapDumpForLargeArray.java timed out
>
> If your rewrite has the side affect of fixing this issue, what I would suggest is a new CR and PR that just address the rewrite, especially since this PR has a lot of history that I think you are saying will pretty much become obsolete, and therefore is just noise to anyone wanting to review your changes. In the new PR you can note that the changes will also address 8262386, and you can include an `/issue 8262386` comment so it will be closed out with new PR.

Hi Chris(@plummercj ), 
     Do you think this PR could be the proper fix of 8262386?   
     After investigation, I think the rewrite may be more complicated than I thought:

-  The compressed heap dump, when the heap size is not larger than `2GB(Integer.MAX_VALUE)`, could be dumped directly with the dump size calculated by DataOutputStream.size(),  same as the uncompressed heap dump.  In this way, segmented heap dump could be used only for large heap dump.
- When heap size is larger than `2GB`,  segmented heap dump must be enabled to avoid the overflow of the `size` slot in segment, and also DataOutputStream.size() could not be used because of overflow. In this case, the `unbuffered mode` (or `write through mode`) should be used to avoid caching memory for compressed dump, especially for large objects. 

And the benefit of this rewriting is to reduce memory consumption.  E.g. compressed heap dump does not require the segmental heap dump when heap is not large, and segmented heap dump could save memory by using `unbuffered mode`. But it doesn't have stronge relationship of the fix of 8262386.

IMHO,  this PR could be the fix of current implementation, and the fix code will be reused in the rewriting-solution. So maybe we could fix this issue first and then tracking the rewriting with a seperate JBS issue and PR? 
What do you think?  

BRs,
Lin

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

PR: https://git.openjdk.java.net/jdk/pull/2803