RFR: 8261608: Move common CDS archive building code to archiveBuilder.cpp

[Coleen Phillimore]

On Thu, 11 Feb 2021 23:41:34 GMT, Ioi Lam <iklam at openjdk.org> wrote:

> This is a follow-up to https://git.openjdk.java.net/jdk/pull/2296:
> 
> - Move common code for writing the CDS archive from metaspaceShared.cpp to archiveBuilder.cpp
> 
> - Data structures related to dumping were haphazardly organized in several classes (e.g., `DumpRegions`). We needed various APIs to access them across classes. These should be consolidated in archiveBuilder.cpp and the API should be cleaned up
> 
> - Detailed stats (`DumpAllocStats::print_stats`) were available only for static dump. Refactor the code so they are also printed for dynamic dump

Looks like a good change.

src/hotspot/share/classfile/moduleEntry.cpp line 33:

> 31: #include "logging/log.hpp"
> 32: #include "memory/archiveBuilder.hpp"
> 33: #include "memory/archiveUtils.hpp"

Do these files need archiveUtils.hpp or does archiveBuilder.hpp need it?

src/hotspot/share/memory/archiveBuilder.cpp line 178:

> 176:   _num_instance_klasses = 0;
> 177:   _num_obj_array_klasses = 0;
> 178:   _num_type_array_klasses = 0;

Should these be initializers also?

src/hotspot/share/memory/archiveUtils.hpp line 44:

> 42: class ArchivePtrMarker : AllStatic {
> 43:   static CHeapBitMap*  _ptrmap;
> 44:   static VirtualSpace* _vs;

I think this is a good change.

src/hotspot/share/memory/metaspaceShared.inline.hpp line 34:

> 32: 
> 33: #if INCLUDE_CDS_JAVA_HEAP
> 34: bool MetaspaceShared::is_archive_object(oop p) {

Was this unused? I don't find 'is_archive_object' in the diff.

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

Marked as reviewed by coleenp (Reviewer).

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