On Fri, 2 Apr 2021 19:43:31 GMT, Chris Plummer <cjplum...@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

Reply via email to