Question about ccs reservation, CDS and aarch64 specifics
ioi.lam at oracle.com
Thu Apr 16 18:28:50 UTC 2020
On 4/16/20 11:14 AM, Thomas Stüfe wrote:
> Hi Ioi,
> On Thu, Apr 16, 2020 at 7:49 PM Ioi Lam <ioi.lam at oracle.com
> <mailto:ioi.lam at oracle.com>> wrote:
> (I suppose you mean "compressed class space" by "ccs" :-)
> Yes, I think I stole this from Stefan Karlsson :)
> 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
> 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
> (a) when CDS is enabled:
> -> MetaspaceShared::initialize_runtime_shared_and_meta_spaces()
> -> ... MetaspaceShared::map_archives()
> -> ... reserve the space, eventually calling
> -> call Metaspace::allocate_metaspace_compressed_klass_ptrs()
> (b) when CDS is disabled
> -> 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
> 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.
Yes, that seems the right thing to do. That will hopefully make the
aarch64 initialization code a little simpler as well.
> 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.
Do you mean:
That's not necessary. When I wrote the code I thought
Metaspace::reserve_space was a general function for reserving spaces :-)
but as you said, this function is probably intended only for
initializing the CCS.
> Thanks, Thomas
> - 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.
More information about the hotspot-runtime-dev