RFR: 8253909: Implement detailed map file for CDS

[Thomas Stuefe]

On Thu, 1 Oct 2020 20:06:32 GMT, Ioi Lam <iklam at openjdk.org> wrote:

> For analyzing the contents of a CDS archive, we need a "map file" that describes the archive in different levels of
> detail. This can be done using unified logging. E.g.,
> java -Xshare:dump -Xlog:cds+map=trace:file=cds.map:none:filesize=0
> 
> For example, we can use the map file for troubleshooting [JDK-8253495](https://bugs.openjdk.java.net/browse/JDK-8253495)
> (runtime/cds/DeterministicDump.java broken). We can diff two different cds.map files to see where the non-deterministic
> contents come from.
> See attachments in [JDK-8253909](https://bugs.openjdk.java.net/browse/JDK-8253909) for example map files at the
> info/debug/trace levels.

Not a full review, just some drive-by comments.

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

> 721:   // Dump all the data [base...top). Pretend that the base address
> 722:   // will be mapped to runtime_base at run-time.
> 723:   static void write_data(address base, address top, address runtime_base) {

Consider printing the data with os::print_hex_dump() instead. Would slim down this coding.

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

> 767:       log_info(cds, map)("Log level = info");
> 768:       log_info(cds, map)("Run with -Xlog:cds+map=debug for more detailed information");
> 769:       log_info(cds, map)("Run with -Xlog:cds+map=trace for even more detailed information");

Matter of taste, but I do not think usage information belongs into a log.

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

> 757:              GrowableArray<MemRegion> *closed_heap_regions,
> 758:              GrowableArray<MemRegion> *open_heap_regions,
> 759:              char* bitmap, size_t bitmap_size_in_bytes) {

Consider, instead of directly writing to UL, writing to an outputStream*. That would make the code more versatile and
reusable. You can then use LogStream to print to UL with the same result.

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

> 607: // picked by the OS. At runtime, we try to map at a fixed location (SharedBaseAddress). For
> 608: // consistency, we log everything using runtime addresses.
> 609: class ArchiveBuilder::CDSMapLogger {

AllStatic?

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

> 773:     address header_end = header + mapinfo->header()->header_size();
> 774:     write_region("header", header, header_end, 0);
> 775:     write_data(header, header_end, 0);

Consider writing out the header members human-readable, that would be more useful than a hexdump. I'd also print them
already at Info level, since often the header info is the only thing interesting.

src/hotspot/share/memory/filemap.hpp line 461:

> 459:   void  write_region(int region, char* base, size_t size,
> 460:                      bool read_only, bool allow_exec);
> 461:   char* write_bitmap_region(const CHeapBitMap* ptrmap,

Can you add a comment here and in MetaspaceShared::write_core_archive_regions() that the returned bitmap is c-heap
allocated and needs to be freed?

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

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