RFR: 8350209: Preserve adapters in AOT cache [v12]
Vladimir Kozlov
kvn at openjdk.org
Thu May 1 15:46:51 UTC 2025
On Thu, 1 May 2025 15:10:09 GMT, Ashutosh Mehra <asmehra at openjdk.org> wrote:
>> Maybe add an assert here?
>>
>>
>> bool success = AOTCodeCache::store_code_blob(...);
>> assert(success || !AOTCodeCache::is_dumping_adapters(), "");
>
>> assert(success || !AOTCodeCache::is_dumping_adapters(), "");
>
> This condtion `!AOTCodeCache::is_dumping_adapters()` in the assert is not very intuitive. I think what we need to assert is future stores in the aot code cache are disabled. So having a method like `AOTCodeCache::is_store_disabled()` would better communicate the intent here. But I don't mind keeping this condition for this initial PR. I will just suggest to add a better assert message like:
>
> ```assert(success || !AOTCodeCache::is_dumping_adapters(), "storing of adapter must be disabled");```
>
> And I think we should also be setting `_adapter_caching` to false in `report_load_failure` and `report_store_failure` to be consistent, otherwise we may end up in a situation where `AOTAdapterCaching` is false but `_adapter_caching` is true. In fact, I feel we should only be setting `_adapter_caching` and not `AOTAdapterCaching` in `report_load/store_failure` because `_adapter_caching` is the flag used to gate store/load operations.
Thank you, @ashu-mehra. You have good points. I will work on them.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24740#discussion_r2070428589
More information about the build-dev
mailing list