RFR(S) 8231257 (CDS) Avoid calling FileMapInfo::write_region twice

[Ioi Lam]

Hi Calvin,

Thanks for the review. I'll fix the problems you pointed out, do more 
testing and then push.

- Ioi

On 9/23/19 12:08 PM, Calvin Cheung wrote:
> Hi Ioi,
>
> Thanks for doing this cleanup.
>
> http://cr.openjdk.java.net/~iklam/jdk14/8231257-call-write-region-once.v01/src/hotspot/share/memory/filemap.cpp.sdiff.html 
>
>
> 1102     header_bytes += strlen(Arguments::GetSharedArchivePath() + 1);
>
> Should the '+ 1' be outside of strlen?
>
>              header_bytes += strlen(Arguments::GetSharedArchivePath()) 
> + 1;
>
> http://cr.openjdk.java.net/~iklam/jdk14/8231257-call-write-region-once.v01/src/hotspot/share/memory/filemap.hpp.sdiff.html 
>
>
> 410   void  seek_to_position(size_t pos);
>
> Could the above be under the private block since it is only used 
> within FileMapInfo?
>
> thanks,
>
> Calvin
>
> On 9/19/19 9:39 AM, Ioi Lam wrote:
>> https://bugs.openjdk.java.net/browse/JDK-8231257
>> http://cr.openjdk.java.net/~iklam/jdk14/8231257-call-write-region-once.v01/ 
>>
>>
>> Please review this clean up in CDS:
>>
>> We call FileMapInfo::write_region twice in a loop --
>>
>> + first time just to store the CRC and offset into in the file header
>> + second time is to actually write the region
>>
>> This is awkward. The whole reason is for writing the file header 
>> before writing the regions.
>>
>> But that's not necessary. We should first write the regions, then 
>> seek back to position 0, and write the header.
>>
>> Thanks
>> - Ioi
>>
>>
>>
>> <https://bugs.openjdk.java.net/browse/JDK-8231257>