[aarch64-port-dev ] Question about ccs reservation, CDS and aarch64 specifics

[Thomas Stüfe]

Hi Ioi,

On Thu, Apr 16, 2020 at 7:49 PM Ioi Lam <ioi.lam at oracle.com> wrote:

> (I suppose you mean "compressed class space" by "ccs" :-)
>
>
Yes, I think I stole this from Stefan Karlsson :)


> <snip>
>


> I am not even sure if case (C) can happen at all.
>
> I admit that I've been guilty of making the interface even more complicated
> with JDK-8231610 <https://bugs.openjdk.java.net/browse/JDK-8231610>
> (Relocate the CDS archive if it cannot be mapped to the
> requested address). Looks now is a good time to clean up.
>
>
The coding has been complicated to begin with, and then it usually only
gets worse since no-one has time for a revamp :( A clean up would be very
helpful.

One reason I look at this coding now, beside the aarch64 problem, was that
I try to disentangle CDS from Metaspace, especially the alignment policy.
Remember, I tried to tackle this last summer? but it keeps biting me. For
such a small problem this is weirdly complicated.


> One thing that can be cleaned up is the call to
> Metaspace::allocate_metaspace_compressed_klass_ptrs:
>
> (a) when CDS is enabled:
>
>     Metaspace::global_initialize()
>     -> MetaspaceShared::initialize_runtime_shared_and_meta_spaces()
>        -> ... MetaspaceShared::map_archives()
>          -> ... reserve the space, eventually calling
> Metaspace::reserve_space
>          -> call Metaspace::allocate_metaspace_compressed_klass_ptrs()
>
> (b) when CDS is disabled
>
>     Metaspace::global_initialize()
>     -> allocate_metaspace_compressed_klass_ptrs
>        -> (if cds is not enabled) Metaspace::reserve_space()
>
>
> In case (b), we should first reserve the space, and then call into
> allocate_metaspace_compressed_klass_ptrs. This will simplify the arguments
> of allocate_metaspace_compressed_klass_ptrs, and will also limit the
> variations
> of calls to Metaspace::reserve_space(). I think this will make it possible
> to
> drop the use_requested_addr argument and rely simply on (requested_addr !=
> NULL)
>
>
So, in all cases we'd pre-reserve the ReservedSpace and hand it down to
Metaspace::allocate_metaspace_compressed_klass_ptrs()?

This would melt down Metaspace::allocate_metaspace_compressed_klass_ptrs()
to just "initialize compressed class space from a pre-arranged
ReservedSpace, and set up base + shift".

We could probably rename that thing
to Metaspace::set_up_compressed_klass_space(ReservedSpace* rs, cds_base);

We even could move set_narrow_klass_base_and_shift() out of
Metaspace::set_up_compressed_klass_space, then it becomes a series of three
simple operations:
1) obtain a ReservedSpace however you see fit
2) register it with Metaspace as address space for ccs,
3) set_narrow_klass_base_and_shift. We would not have to hand down cds_base
to Metaspace, only for it to be used as base address
in set_narrow_klass_base_and_shift.

One question which came to me today was:

In AppCDS, DynamicArchiveBuilder::do_it() calls Metaspace::reserve_space().
Is that really needed, does a DumpRegion have anything to do with ccs?
Don't they just need some space to dump into? Hope that question is not
dumb.

Thanks, Thomas


> Thanks
> - Ioi
>
>
> Does that make sense? In other words, if the whole point of
> Metaspace::reserve_preferred_space() is "OS knows better, let it try
> to find a good address", would it not make sense to just try a low
> address as part of the try-addresses-loop?
>
> We certainly don't want to have to use a dedicated heapbase register
> or a shift. Just give us a multiple of 4*G and we're happy.
>
>
>
>