RFR: 8250989: Consolidate buffer allocation code for CDS static/dynamic dumping [v2]

[Thomas Stuefe]

On Sat, 6 Feb 2021 05:08:07 GMT, Ioi Lam <iklam at openjdk.org> wrote:

> > Hi Ioi,
> > not a full review, but the metaspace related changes to reservations look fine. See minor nits below.
> > This is unrelated to your patch: in compute_shared_base, you set SharedBaseAddress in case of an error. I think it should either be just a calculator function with no side effects - just returning the corrected value - or always set SharedBaseAddress and _requested_base_address always, returning void. That would be clearer.
> 
> I changed `compute_shared_base()` to have no side effect.
> 
> > One question, SharedBaseAddress vs MetaspaceShared::_requested_base_address : what is the difference? When do you use one, when the other?
> 
> I added comments in metaspaceShared.hpp:
> 
> ```
>   // This is the base address as specified by -XX:SharedBaseAddress during -Xshare:dump.
>   // Both the base/top archives are written using this as their base address.
>   //
>   // During static dump: _requested_base_address == SharedBaseAddress.
>   //
>   // During dynamic dump: _requested_base_address is not always the same as SharedBaseAddress:
>   // - SharedBaseAddress is used for *reading the base archive*. I.e., CompactHashtable uses
>   //   it to convery offsets to pointers to Symbols in the base archive.
>   //   The base archive may be mapped to an OS-selected address due to ASLR. E.g.,
>   //   you may have SharedBaseAddress == 0x00ff123400000000.
>   // - _requested_base_address is used for *writing the output archive*. It's usually
>   //   0x800000000 (unless it was set by -XX:SharedBaseAddress during -Xshare:dump).
>   static char* requested_base_address() {
>     return _requested_base_address;
>   }
> ```
> 
> Thanks!
> Ioi

Much clearer, thanks!

Cheers, Thomas

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

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