[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