Question about ccs reservation, CDS and aarch64 specifics

Thomas Stüfe thomas.stuefe at gmail.com
Sat Apr 18 07:15:21 UTC 2020


Hi Ioi,

I am working on a small patch and have some more questions.

- First, a simple one, in
DynamicArchiveBuilder::reserve_space_and_init_buffer_to_target_delta(), the
space does not have anything to do with metaspace, as you wrote, so the
alignment could be anything, right?

- Out of curiousity, when you pack the different regions (DumpRegion::pack)
you align the end to page size. Why? Why could the next region not simply
follow immediately? I looked if any code needs a region to be page aligned,
but may have missed it.

- void MetaspaceShared::initialize_dumptime_shared_and_meta_spaces() :

I assume this code has to work for all three cases right
1) lp32.
2) lp64 with and without UseCompressedClassPointers?
3) lp64 without UseCompressedClassPointers?

If yes, does the setting for UseCompressedClassPointers have to be the same
at run time?


In this layout:
  // On 64-bit VM, the heap and class space layout will be the same as if
  // you're running in -Xshare:on mode:
  //
  //                              +-- SharedBaseAddress (default =
0x800000000)
  //                              v
  // +-..---------+---------+ ... +----+----+----+--------------------+
  // |    Heap    | Archive |     | MC | RW | RO |    class space     |
  // +-..---------+---------+ ... +----+----+----+--------------------+
  // |<--   MaxHeapSize  -->|     |<-- UnscaledClassSpaceMax = 4GB -->|
  //

Why does the class space has to follow mc+rw+ro? Could it come before?

Actually, does it have to be in the same space at all, or could it live
somewhere completely different?

Thanks!

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

>
>
> 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> 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.
>
>
> 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:
>
> DynamicArchiveBuilder::reserve_space_and_init_buffer_to_target_delta()
> -> MetaspaceShared::reserve_shared_space
>     -> Metaspace::reserve_space
>
> 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
> - Ioi
>
> 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.
>>
>>
>>
>>
>


More information about the hotspot-runtime-dev mailing list