On Fri, 17 Apr 2026 21:35:06 GMT, Ashutosh Mehra <[email protected]> wrote:
> hmm, I got a crash with my patch. I will debug that some time later.
My local testing `appcds/aotCode` on x64 passed with your patch. But I will
wait your next comment before integrating it.
> src/hotspot/share/cds/cds_globals.hpp line 178:
>
>> 176: "Test failure of adapter linking when loading from AOT
>> cache.") \
>> 177:
>> \
>> 178: product(bool, AOTCodeCPUFeatureCheck, true, DIAGNOSTIC,
>> \
>
> Is there any use-case for disabling cpu feature check? Otherwise this flag
> can be removed, as it is not being used anywhere.
removed
> src/hotspot/share/code/aotCodeCache.cpp line 282:
>
>> 280: }
>> 281:
>> 282: bool AOTCodeCache::allow_const_field(ciConstant& value) {
>
> IMO it seems this API is better suited in `ciEnv`.
Replaced it with `!env->is_aot_compile()` in call sites. Originally
`allow_const_field` had a lot more checks.
> src/hotspot/share/code/aotCodeCache.cpp line 321:
>
>> 319: is_using = is_caching_enabled();
>> 320: }
>> 321: if (ClassInitBarrierMode > 0 && !(is_dumping && AOTCodeCaching)) {
>
> Is this required given that `ClassInitBarrierMode` gets set to 0 if
> `is_dumping_code()` is false in `init2()`?
removed from here
> src/hotspot/share/code/aotCodeCache.cpp line 1096:
>
>> 1094: assert(_load_entries != nullptr, "sanity");
>> 1095: {
>> 1096: uint name_offset = entry->offset() + entry->name_offset();
>
> This block of code is repeated again below:
>
> uint name_offset = entry->offset() + entry->name_offset();
> const char* name = _load_buffer + name_offset;;
> uint level = entry->comp_level();
> uint comp_id = entry->comp_id();
> bool for_preload = entry->for_preload();
> bool clinit_brs = entry->has_clinit_barriers();
>
> I think it is worth moving it out of `ifdef ASSERT` so that it doesn't have
> to be repeated.
removed this block
> src/hotspot/share/code/aotCodeCache.cpp line 1143:
>
>> 1141: // We can still use normal AOT code if preload code is
>> 1142: // invalidated - normal AOT code has less restrictions.
>> 1143: Method* method = entry->method();
>
> This is unused.
removed
> set_c1_stubs_complete and set_c2_stubs_complete can happen concurrently.
No, because the same lock `CompileThread_lock` is used to schedule
initialization of both compilers - see
`AbstractCompiler::should_perform_init()`.
But the order could be different during assembly and production which will
cause issue. That is why I had different buffers in original changes: _C1_blobs
and _C2_blobs.
> src/hotspot/share/code/aotCodeCache.cpp line 4020:
>
>> 4018: #undef _extrs_max
>> 4019: #undef _stubs_max
>> 4020: #undef _shared_blobs_max
>
> `_shared_blobs_max`, `_C1_blobs_max`, `_C2_blobs_max`, `_blobs_max`,
> `_shared_blobs_base`, `_C1_blobs_base`, `_C2_blobs_base`, `_blobs_end` are
> not defined. These undef-s can be removed.
removed
> src/hotspot/share/code/aotCodeCache.cpp line 4322:
>
>> 4320: struct AOTCodeStats {
>> 4321: private:
>> 4322: struct RTStats {
>
> I think we can get rid of this inner structures in `AOTCodeStats` and
> `AOTCodeEntryStats`. In premain I was using `AOTCodeStats` is a common
> interface for both these stats so I introduced two different inner structs.
> But now that there are two different interfaces, I think we don't need the
> inner struts.
removed RTStats and other structure in AOTCodeEntryStats.
> src/hotspot/share/code/aotCodeCache.hpp line 109:
>
>> 107: uint _name_size;
>> 108: uint _code_offset; // Start of code in cache
>> 109: uint _code_size; // Total size of all code sections
>
> this is unused; can be removed.
Removing `_code_size` caused cache corruption or memory stomp - when load AOT
stubs we hit assert that stub ID is invalid. Need more time to investigate.
`sizeof(AOTCodeEntry)` is `44` after this removal. May be something expect it
should be 8 bytes aligned?
> src/hotspot/share/code/aotCodeCache.hpp line 447:
>
>> 445: #define AOTCODECACHE_DECLARE_FUN(type, name, func) type _saved_ ## name;
>> 446:
>> 447: struct AOTCodeSection {
>
> This struct is also not used anywhere; can be removed.
removed
-------------
PR Comment: https://git.openjdk.org/jdk/pull/30778#issuecomment-4272173376
PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r3141980459
PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r3142282610
PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r3141981180
PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r3141981758
PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r3141981991
PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r3141993445
PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r3141982246
PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r3141982911
PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r3141986149
PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r3141983147
