[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-runtime-dev
Subject:    Re: RFR: 8262046: Clean up parallel class loading code and comments [v2]
From:       Ioi Lam <iklam () openjdk ! java ! net>
Date:       2021-03-31 23:59:22
Message-ID: Oyj24g5u_CPjnZNO2LNKDYr4-BLoIuq-mzfrMiw9b4Q=.bb5cfc0e-08c4-495d-87d1-c0169009c697 () github ! com
[Download RAW message or body]

On Wed, 31 Mar 2021 16:32:35 GMT, Coleen Phillimore <coleenp@openjdk.org> wrote:

> > src/hotspot/share/classfile/systemDictionary.cpp line 383:
> > 
> > > 381: // the super class directly, then resume steps 4-6.
> > > 382: //
> > > 383: //
> > 
> > resolve_super_or_fail had this comment that I found difficult to understand and \
> > work through, which I did at one point, so I wanted to replace it with one that \
> > seemed less so.  It occurs to me that this is probably unhelpful also.   Maybe \
> > the whole comment from lines 351 to 381 should be replaced with a sentence like: 
> > //resolve_super_or_fail adds a LOAD_SUPER placeholder to the placeholder table \
> > before calling // resolve_instance_class_or_null.  ClassCircularityError is \
> > detected when a LOAD_SUPER or LOAD_INSTANCE // placeholder for the same thread, \
> > class, classloader is found. 
> > I'm unlikely to work through the new comment example again myself.
> 
> Since nobody wants to read this example in this comment and neither do I, I'm \
> removing it.  Debugging this using -Xlog:class+load+placeholders=debug will show \
> the stacking of placeholders that detect CCE.

I guess it's OK to remove this comment, since most of us who have looked at this code \
in the past few years haven't found this comment to be helpful. Maybe you can add a \
comment about `-Xlog:class+load+placeholders=debug` instead?

If we do want to improve the comment, maybe we can replacing Base->Super with \
something like Car->Vehicle. That way it's clear which is the supertype.

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

PR: https://git.openjdk.java.net/jdk/pull/3200


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic