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

[Ioi Lam]

On Thu, 4 Feb 2021 10:02:51 GMT, Thomas Stuefe <stuefe 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

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

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