On Tue, 18 Apr 2023 20:16:06 GMT, Ioi Lam <ik...@openjdk.org> wrote:

>> This PR combines the "open" and "closed" regions of the CDS archive heap 
>> into a single region. This significantly simplifies the implementation, 
>> making it more compatible with non-G1 collectors. There's a net removal of 
>> ~1000 lines in src code and another ~1200 lines of tests.
>> 
>> **Notes for reviewers:**
>> - Most of the code changes in CDS are removing the handling of "open" vs 
>> "closed" objects.
>>   - Reviewers can start with ArchiveHeapWriter::copy_source_objs_to_buffer().
>>   - It might be easier to see the diff with whitespaces off.
>> - There are two major changes in the G1 code
>>   - The archived objects are now stored in the "old" region (see 
>> g1CollectedHeap.cpp in 
>> [58d720e](https://github.com/openjdk/jdk/pull/13284/commits/58d720e294bb36f21cb88cddde724ed2b9e93770))
>>   - The majority of the other changes are removal of the "archive" region 
>> type (see heapRegionType.hpp). For ease of review, such code is isolated in 
>> [a852dfb](https://github.com/openjdk/jdk/pull/13284/commits/a852dfbbf5ff56e035399f7cc3704f29e76697f6)
>> - Testing changes:
>>   - Now the archived java objects can move, along with the "old" regions 
>> that contain them. It's no longer possible to test whether a heap object 
>> came from CDS. As a result, the `WhiteBox.isShared(Object o)` API has been 
>> removed.
>>   - Many tests that uses this API are removed. Most of them were written 
>> during early development of CDS archived objects and are no longer relevant.
>> 
>> **Testing:**
>> - Mach5 tiers 1 ~ 7
>
> Ioi Lam has updated the pull request with a new target base due to a merge or 
> a rebase. The pull request now contains 21 commits:
> 
>  - Merge branch 'master' into 8298048-combine-cds-heap-to-single-region-PUSH
>  - Fixed assert in runtime/cds/appcds/SharedArchiveConsistency.java
>  - Removal of JFR custom closed/open archive region types
>  - Remove g1 full gc skip marking optimization
>  - Some comment updates
>  - Move g1collectedheap archive related regions together in the cpp file
>  - Factor out region/range iteration
>  - Fix comment
>  - Ioi fix
>  - fixed merge
>  - ... and 11 more: https://git.openjdk.org/jdk/compare/0f3828dd...8a35c7ee

GC changes seem good. Only cursorily looked at runtime/CDS changes.

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

Marked as reviewed by tschatzl (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/13284#pullrequestreview-1391786221

Reply via email to