On Fri, 11 Sep 2026 17:34:48 GMT, Coleen Phillimore <[email protected]> wrote:

>> Remove the upcall to addClass during class loading.  The comment says it's 
>> only so GC can keep classes alive while the class loader is alive.  We have 
>> other ways to do that. There were some JVMTI tests in the past that failed 
>> without this vector but today seems to be only one test.  Maybe there's some 
>> code that has a dependency on this in heap walking.
>> Tested tier1-6
>> 
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the 
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> Coleen Phillimore has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Use Alan's suggested spec changes.

I've completed the first review pass and posted my comments and questions. Will 
do one more early next week.

test/hotspot/jtreg/serviceability/HeapDump/ClassLoaderFieldsTest.java line 48:

> 46:  * @test
> 47:  * @bug 8391308
> 48:  * @summary Verifies heap dump contains java.lang.Class instance in 
> ClassLoader

Q: Is this summary consistent with the fix? Can it be a little bit more clear?
The fix removes the `ClassLoader` to the loaded classes relationship.

test/hotspot/jtreg/serviceability/jvmti/FollowReferences/ClassLoader/ClassLoaderTest.java
 line 30:

> 28:  * @summary The test verifies heap walking API (FollowReferences) reports 
> classes
> 29:  *          when starting from the class loader.
> 30:  * @run main/othervm/native -agentlib:ClassLoaderTest ClassLoaderTest

Q: Does this need `@requires vm.jvmti` ?

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

PR Review: https://git.openjdk.org/jdk/pull/32519#pullrequestreview-5185744414
PR Review Comment: https://git.openjdk.org/jdk/pull/32519#discussion_r3995555345
PR Review Comment: https://git.openjdk.org/jdk/pull/32519#discussion_r3995532193

Reply via email to