RFR: 8236926: Concurrently uncommit memory in G1 [v7]

[Stefan Johansson]

On Mon, 16 Nov 2020 19:39:13 GMT, Stefan Johansson <sjohanss at openjdk.org> wrote:

>> Please review this change that implements concurrent uncommit for G1.
>> 
>> **Summary**
>> G1 currently check if the heap can be shrunk at the end of the Remark pause and at the end of a Full GC. The uncommit work (handing back the memory to the OS) is quite expensive and this change moves it out of the pause. The actual uncommitting is now handled by the G1 service thread and the new task `G1 Uncommit Region Task`. The new task will uncommit memory in chunks of regions to avoid starving out other tasks.
>> 
>> The calculations of how much to shrink the heap and when is not changed, but during the pause only quick preparation work is done. Splitting the uncommit work into two parts comes with some additional meta-data cost. Previously we had a single bitmap to mark if a region was committed or not, now we need two bitmaps. One bitmap to keep track of the regions available for use (active) and one bitmap for the regions ready to be uncommitted (inactive). The union of those two bitmaps are the regions currently committed. When expanding the heap we prefer to re-activate regions from the inactive bitmap if there are any, instead of committing new regions, since this is cheaper (avoiding calls to the OS). 
>> 
>> Splitting the work also comes with some additional synchronization. Both the uncommit task and a mutator thread doing a humongous allocation might want to alter the inactive map at the same time. To prevent this a new lock `Uncommit_lock` is added.
>> 
>> One thing to note is that there is still one case left where we do the uncommit directly and this is during CDS initialization. 
>> 
>> **Logging**
>> To track the concurrent uncommit in logs a few additional messages have been added. There are no new `info` messages, but for `gc+heap` there are two new `debug` messages and one `trace`:
>> [7,468s][debug][gc,heap        ] GC(32) Regions ready for uncommit: 1873
>> ...
>> [7,509s][trace][gc,heap        ] Concurrent Uncommit: 256M, 32 regions, 11,173ms
>> [7,522s][trace][gc,heap        ] Concurrent Uncommit: 256M, 32 regions, 12,599ms
>> ...
>> [9,691s][debug][gc,heap        ] Concurrent Uncommit Summary: 4864M, 608 regions, 405,827ms
>> 
>> The `trace` is printed for each invocation of the task while the `debug` message is only printed when there is no more uncommit work available. As you can see in the above example, it's not certain that all regions made ready for uncommit are actually uncommitted. The reason for this is that the heap had to grow again during the concurrent uncommit, and regions were re-activated.
>> 
>> On `gc+heap+region` there are new logs to see how ranges of regions transition between different states:
>> [6,337s][debug][gc,heap,region ] Uncommit regions [12768, 13024)
>> [6,424s][debug][gc,heap,region ] Uncommit regions [13024, 13280)
>> [6,438s][debug][gc,heap,region ] Uncommit regions [13280, 13536)
>> [6,510s][debug][gc,heap,region ] Uncommit regions [13536, 13792)
>> [6,573s][debug][gc,heap,region ] GC(79) Reactivate regions [13792, 15651)
>> [6,574s][debug][gc,heap,region ] GC(79) Activate regions [76, 96)
>> [6,579s][debug][gc,heap,region ] GC(79) Activate regions [97, 1099)
>> 
>> **Testing**
>> Two new tests have been added, one gtest and one jtreg test. These are intended to test the basic functionality, but most testing is gained by just running applications that resize the heap. This is quite common in our testing, so the code will be exercised a lot.
>> 
>> I've run multiple runs of mach5 testing tier 1-5 as well as local testing. I've also done a performance run and as expected there are not significant changes.
>
> Stefan Johansson has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 15 commits:
> 
>  - Merge branch 'master' into 8236926-ccu
>  - Zoom feedback
>  - Albert review 2
>  - Albert review
>  - Merge branch 'master' into 8236926-ccu
>  - Lock for small mapper and use BitMap parallel operations.
>  - Self review
>  - Simplified task
>  - Improved logging
>  - Test improvement
>  - ... and 5 more: https://git.openjdk.java.net/jdk/compare/3675653c...c354b1d8

Thanks for the review Thomas. Addressed most of your concerns, but some things I left as is.

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

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