On Fri, 25 Apr 2025 20:54:21 GMT, John R Rose <jr...@openjdk.org> wrote:
>> Aleksey Shipilev has updated the pull request with a new target base due to >> a merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains four additional >> commits since the last revision: >> >> - Minor touchup >> - Merge branch 'master' into JDK-8342283-cds-many-classes >> - Adjust TEST.groups >> - Fix > > src/java.base/share/classes/java/lang/ClassLoader.java line 2590: > >> 2588: * @implNote This is done while the JVM is running in >> single-threaded mode, >> 2589: * and at the very end of Java bytecode execution. We know that no >> more classes >> 2590: * will be loaded and none of the fields modified by this method >> will be used again. > > There is a tiny technical debt here. If future AOT code generation uses a > future aggressive constant folding of object fields (cf. > `TrustFinalNonStaticFields`) and a constant CL reference ends up in optimized > code and there is a constant-folded reference (and.. and…) it is remotely > possible that the old value of the field will get wrongly embedded in AOT > code. > > If we arrange AOT code generation to occur after all of these fixups (in Java > code) are done, then the problem will not occur. It's a delicate set of > invariants. Your expanded comment is a good start at calling them out, but > this is a long string we are pulling on. (I borrowed this comment from the initial @iklam's PR, so I need to credit him as contributor.) Yes, CDS does awkward state manipulations at dump time. Resetting the states of final/`@Stable` objects can run into issues that you described. I think this is one of the reasons why Leyden generates AOT code with `-XX:-FoldStableValues`: https://github.com/openjdk/leyden/blob/885096a8b3194371cde6b96ce5554d89f99618d7/src/hotspot/share/code/aotCodeCache.cpp#L162 I would guess we need to do the same with `TrustFinalNonStaticFields`. The awkward part of current `trust_final_non_static_fields()` code is that it implicitly trusts things in `java/lang`, even with `-TrustFinalNonStaticFields`. That sounds like something we need to rectify for Leyden AOT code. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/24877#discussion_r2063277934
