On Mon, 27 Oct 2025 17:54:37 GMT, Vladimir Kozlov <[email protected]> wrote:
>> Erik Österlund has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 22 commits: >> >> - Merge branch 'master' into 8326035_JEP_object_streaming_v6 >> - Missing unlock diagnostic VM options, and remove unintended comment >> - Fix for minimal >> - Change streaming/mapping states to be binary instead of tri states >> - Clean up VMProps >> - Make AOTStreamableObjects diagnostic >> - Test polishing >> - Merge branch 'master' into 8326035_JEP_object_streaming_v6 >> - Test fix >> - move exception marks outside locks >> - ... and 12 more: https://git.openjdk.org/jdk/compare/97e5ac6e...3d771ca2 > > src/hotspot/share/cds/aotThread.hpp line 33: > >> 31: // A hidden from external view JavaThread for materializing archived >> objects >> 32: >> 33: class AOTThread : public JavaThread { > > Short names are good but this one don't provide any information about what it > does. > How about `AOTHeapLoadingThread`? I was sort of thinking that there might be more AOT cache work that could benefit from concurrency, other than object loading. That's why I named it a bit more generically. Does that make sense? Otherwise, I'm open to renaming it. > src/hotspot/share/jfr/support/jfrThreadLocal.cpp line 483: > >> 481: assert(t != nullptr, "invariant"); >> 482: oop threadObj = t->threadObj(); >> 483: return threadObj != nullptr ? AccessThreadTraceId::id(threadObj) : >> static_cast<traceid>(t->monitor_owner_id()); > > Is this related to JEP? > Is it bug fix (return `monitor_owner_id` instead of 0)? I added this because the AOT thread starts earlier than the Thread class is initialized. Therefore, I have to create the thread with a null thread object, and initialize the thread oop after the Thread class is initialized. This threw this JFR code off. So I made it accept the monitor_owner_id which is set up for similar reasons. > src/hotspot/share/memory/metaspace.cpp line 29: > >> 27: #include "cds/aotMetaspace.hpp" >> 28: #include "cds/cdsConfig.hpp" >> 29: #include "cds/heapShared.hpp" > > There are no other changes in this file. Is this include needed? Probably not - I'll remove it. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/27732#discussion_r2468536258 PR Review Comment: https://git.openjdk.org/jdk/pull/27732#discussion_r2468546194 PR Review Comment: https://git.openjdk.org/jdk/pull/27732#discussion_r2468547745
