On Tue, 17 Nov 2020 10:55:19 GMT, Thomas Schatzl <tscha...@openjdk.org> wrote:
>> Hi all, >> >> can I have reviews for this change that changes the way how archive >> regions are managed in general and specifically by the G1 collector, fixing >> the crashes caused by adding the module graph into the archive in >> [JDK-8244778](https://bugs.openjdk.java.net/browse/JDK-8244778)? >> >> Previously before the JDK-8244778 change, archived objects could always be >> assumed as live, and so the G1 collector did so, not caring about the >> archive region's contents at all. With JDK-8244778 however, archived objects >> could die, and keep stale references to objects outside of the archive >> regions, which obviously causes crashes when walking these objects. >> >> With this change, open archive region contents are basically handled as any >> other objects; to support that, all open archive regions are now reachable >> via a single object array root. This hopefully also facilitates >> implementation in other collectors. >> >> This allows us to remove quite a bit of special handling in G1 too; the only >> difference is that open archive regions will generally not be collected >> unless they are completely empty: we do want to profit from the sharing >> across VMs as much as possible. >> >> Testing: tier1-5, one or two 6-8 runs >> >> The appcds changes were done by @iklam. These changes are described in this >> document: >> https://wiki.openjdk.java.net/display/HotSpot/CDS+Archived+Heap+Improvements >> >> Thanks, >> Thomas > > Thomas Schatzl has updated the pull request incrementally with two additional > commits since the last revision: > > - sjohanss review > - Remove code that "activates" dormant objects as now all active objects are > reachable via the root object array Marked as reviewed by kbarrett (Reviewer). src/hotspot/share/memory/heapShared.cpp line 660: > 658: VM_Verify verify_op; > 659: VMThread::execute(&verify_op); > 660: if (!FLAG_IS_DEFAULT(VerifyArchivedFields)) { Comment says "command line", so this should be FLAG_IS_CMDLINE rather than !FLAG_IS_DEFAULT. src/hotspot/share/memory/heapShared.cpp line 662: > 660: if (!FLAG_IS_DEFAULT(VerifyArchivedFields)) { > 661: // If this -XX:+VerifyArchivedFields is specified on the > command-line, do extra > 662: // checks. s/this// ------------- PR: https://git.openjdk.java.net/jdk/pull/1163
