RFR: Save/load i2c2i adapters [v2]

[Vladimir Kozlov]

On Thu, 24 Oct 2024 15:59:03 GMT, Ashutosh Mehra <asmehra at openjdk.org> wrote:

>> This is an attempt to save and load i2c2i adapters along with the adapter handler table.
>> There are mainly two parts to this change:
>> 1. Storing of adapter code in the SCCache or AOT code cache.
>> 2. Storing of adapter handler table in the AOT cache.
>> 
>> Adapter handler table is a map from AdapterFingerPrint to AdapterHnadlerEntry. To store them in AOT cache, AdapterFingerPrint and AdapterHandlerEntry are updated to MetaspaceObj. Both these entities are discovered and added to the cache while processing the Method. When storing the adapter handler table, only the entries that have already been archived are considered. This allows pruning of AdapterHnadlerEntry that may be only reachable through a Method that is not eligible to be archived. 
>> 
>> An AdapterHandlerEntry has pointer to the adapter code. Because the AdapterHandlerEntry and the adapter code are stored in separate archives, this link between the AdapterHandlerEntry and the adapter code needs to be removed (see AdapterHandlerEntry::remove_unshareable_info()).
>> During the production run, as the methods in the AOT cache are adopted, the AdapterHandlerEntry is linked back to the adapter code (see AdapterHandlerEntry::restore_unshareable_info).
>> 
>> All this code is guarded by -XX:[+-]ArchiveAdapters option which defaults to false, but is set to true in CDSConfig during the assembly phase.
>> 
>> Other changes worth mentioning:
>> 1. Changes to the SCCache infrastructure to make it possible to store and load adapter code. (Thanks to @adinn)
>> 2. Updating AdapterFingerPrint hashing algorithm to avoid collisions. If there is any collision, then it will prevent finding the adapter code in the SCCache. (Again courtesy of @adinn)
>> 
>> Thanks to @adinn for providing many of these changes.
>> 
>> Performance:
>> -Xlog:init shows time taken for linking of Methods and making adapters. An example output is:
>> 
>> ClassLoader:
>>   clinit:                             150us / 4612 events
>>   link methods:                     28980us / 176893 events
>>   method adapters:                  15378us / 697 events
>> 
>> Save/load of adapters seem to have improved these stats.
>> 
>> | Quarkus | -ArchiveAdapters | +ArchiveAdapters |
>> |---|---|---|
>> | link methods | 12214us / 58913 events | 2700us / 58913 events |
>> | method adapters | 7793us / 607 events | 4402us / 38 events |
>> 
>> | Spring-petclinic | -ArchiveAdapters | +ArchiveAdapters |
>> |---|---|---|
>> | link methods | 28980us / 176893 events | 7485us / 176893 events |...
>
> Ashutosh Mehra has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Acquire Compile_lock when writing exception blobs to the SCCache
>   
>   Signed-off-by: Ashutosh Mehra <asmehra at redhat.com>

Very nice changes.  Few comments.

src/hotspot/share/code/SCCache.cpp line 595:

> 593:   SCCache* cache = SCCache::cache();
> 594:   if (cache != nullptr && cache->_table != nullptr) {
> 595:     cache->_table->init_extrs();

This is repetitive checks pattern. I suggest to replace it with check of call to:

  SCAddressTable* SCCache::addr_table() const {
    return is_on() && (cache()->_table != nullptr) ? cache()->_table : nullptr;
  }

src/hotspot/share/code/SCCache.cpp line 1175:

> 1173:           if (entries_address[i].comp_level() == CompLevel_none) {
> 1174:             shared_blobs_count++;
> 1175:           } else if (entries_address[i].comp_level() == CompLevel_simple) {

we also caching C1 code with simple profiling: `CompLevel_limited_profile`

src/hotspot/share/code/SCCache.cpp line 2488:

> 2486:   // TODO - maybe move this up to selected callers so we only lock
> 2487:   // when saving a c1 or opto blob
> 2488:   MutexLocker ml(Compile_lock);

Only one compiler thread should generate compiler's runtime blobs.
See `AbstractCompiler::should_perform_init()`.

src/hotspot/share/oops/method.cpp line 1271:

> 1269:   // TODO: how to identify code cache full situation now that the adapter() can be
> 1270:   // non-null if AOT cache is in use
> 1271: #if 0

Can you check next (opposite to check in the following assert)?:

 if (adapter() != nullptr && !adapter()->is_linked()) {

The assumption is that we have enough CodeCache when we loading adapters from APT cache. Otherwise we should bailout (did you test such case?).

Is `is_linked()` is specific for adapters from AOT cache?

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

PR Review: https://git.openjdk.org/leyden/pull/25#pullrequestreview-2419510084
PR Review Comment: https://git.openjdk.org/leyden/pull/25#discussion_r1831764076
PR Review Comment: https://git.openjdk.org/leyden/pull/25#discussion_r1831774205
PR Review Comment: https://git.openjdk.org/leyden/pull/25#discussion_r1831781219
PR Review Comment: https://git.openjdk.org/leyden/pull/25#discussion_r1831806413