RFR 8029178: Parallel class loading test anonymous-simple gets SIGSEGV in Metaspace:, :contains (original) (raw)

Mikael Gerdin mikael.gerdin at oracle.com
Fri Jan 3 05:34:28 PST 2014

On Friday 03 January 2014 07.57.40 Coleen Phillmore wrote:

Mikael,

Thanks for reviewing the code. See below. On 1/3/2014 4:31 AM, Mikael Gerdin wrote: > Coleen, > > On Thursday 02 January 2014 12.25.25 Coleen Phillmore wrote: > > Summary: Metaspace::contains cannot look at purged metaspaces while CMS > > > > concurrently deallocates them. > > > > > > > > Removed 2 calls to ismetaspaceobject where the object may be in a > > > > deallocated metaspace. Removed walking virtual space lists for > > > > determining contains because the virtual space list can change > > > > concurrently with the walk. CLDG::contains is slower but no slowdowns > > > > with testing were observed. > > > > > > > > Tested by SQE testbase tests, jtreg tests. Functional testing by > > > > parallel class loading tests and nsk/coverage/arguments/arguments008 > > > > (ie. calls Method::isvalidmethod) > > > > > > > > open webrev at http://cr.openjdk.java.net/~coleenp/8029178/ > > --- old/src/share/vm/code/dependencies.cpp 2014-01-02 > 12:04:06.325014397 -0500 > > +++ new/src/share/vm/code/dependencies.cpp 2014-01-02 12:04:05.877014397 > -0500 @@ -655,8 +655,6 @@ > > } else { > > o = deps->ooprecorder()->metadataat(i); > > } > > - assert(o == NULL || o->ismetaspaceobject(), > - errmsg("Should be metadata " PTRFORMAT, o)); > > return o; > > } > > Why did you remove only this call to ismetaspaceobject and no other? > > This and none of the other callers of ismetaspaceobject seem to be > in any hot path so I agree with your assessment that this should not > be a performance issue. This one is called indirectly by flushdependencies so could be called for unloaded metaspaces. I hadn't removed any to start out.

Ok. Ship it!

/Mikael

> It seems to me that the core of the change is to not check the > unloading list in CLDG::contains and then change all calls to go > through CLDG::contains, the CLD and then the SpaceManagers and Chunks, > that seems like a reasonable approach. > > I like that you put CLDG::contains in PRODUCT in os::printlocation! Yes, this could be improved to ask isvalidmethod or isvalidconstantpool or the other metadata objects and call print, but many metaspace objects don't have vtables, so we'd have to check directly. > The old code incorrectly calls the static Metaspace::contains for each > CLD in CLDG::contains but calls it like metaspaceornull->contains(x) > so it's not strange that it was found to be slow :) Yes, the old code was wrong and so one of them had to go. Thanks, Coleen > /Mikael > > > bug link https://bugs.openjdk.java.net/browse/JDK-8029178 > > > > > > > > Thanks, > > > > Coleen

More information about the hotspot-dev mailing list