RFR (tedious) 8230586 Encapsulate fields in filemap.hpp

[Calvin Cheung]

Hi Ioi,

Looks good overall.

Just a few comments:

filemap.hpp
----------
326   FileMapHeader *header() const       { return _header; }

This may not be necessary because all references to header() are from 
the class' member functions.
e.g.
342   int    compute_header_crc()  const { return header()->compute_crc(); }

could be
       int    compute_header_crc()  const { return _header->compute_crc(); }


245   MemRegion heap_reserved()                const { return 
_heap_reserved; }

I couldn't find the _heap_reserved member.

filemap.cpp
----------
Extra blank line at line 180.

You have 2 ways of accessing the _header:
e.g.
196   _header->set_header_size(header_size);

214   header()->populate(this, alignment);

As in the above comment on filemap.hpp, maybe just using _header is fine.

cds.h
----
55   int        _is_heap_region;// used in debug build only.

Needs a blank space after the ';'.

thanks,

Calvin

On 9/4/19 9:20 AM, Ioi Lam wrote:
> https://bugs.openjdk.java.net/browse/JDK-8230586
> http://cr.openjdk.java.net/~iklam/jdk14/8230586-encapsulate-filemap-hpp.v01/ 
>
>
> Many of the member fields in filemap.hpp are declared with public 
> accessibility.
> This is contrary to HotSpot coding style, and also makes the code
> unnecessarily intertwined.
>
> I made the fields private and added accessor functions where necessary.
>
> Also, FileMapInfo::_header should not be accessed directly from other 
> code.
> I added accessor functions similar to FileMapInfo::core_spaces_size()
> to access information stored in the header.
>
> Testing: hs-tier1, hs-tier2
>
> Thanks
> - Ioi