RFR 8038212: Method::is_valid_method() check has performance regression impact for stackwalking

[Mikael Gerdin]

On Tuesday 13 May 2014 14.16.30 Coleen Phillimore wrote:
> Hi again,
> 
> I have moved the metaspace vm internal test to the end of execution, and
> also test if klasses are in metaspace, which works fine.  Thank you for
> the suggestion!  Here is a new webrev:

I'm not too happy about moving the test to the before_exit handler.

Also, how can using the run_unit_test-macro (defined in jni.cpp) even work in 
java.cpp?

About methods and classes being available I think we may have preloaded some 
classes in Threads::create_vm, right?

/Mikael


> 
> http://cr.openjdk.java.net/~coleenp/8038212_2/
> 
> Thanks,
> Coleen
> 
> On 5/13/14, 9:51 AM, Coleen Phillimore wrote:
> > Hi Mikael,
> > 
> > On 5/13/14, 6:33 AM, Mikael Gerdin wrote:
> >> Hi Coleen,
> >> 
> >> On Monday 12 May 2014 20.19.15 Coleen Phillimore wrote:
> >>> Summary: Only prune metaspace virtual spaces at safepoint so walking
> >>> them is safe outside a safepoint.
> >>> 
> >>> Walking class loader data graph for contains is really slow for
> >>> applications like nashorn that has a lot of class loader data.
> >>> 
> >>> Tested with nsk.quick.testlist, jtreg tests, and jck tests. Also ran
> >>> 
> >>> open webrev at http://cr.openjdk.java.net/~coleenp/8038212/
> >>> bug link https://bugs.openjdk.java.net/browse/JDK-8038212
> >> 
> >> Thanks for taking care of this fix.
> >> I think the idea of deferring CLDG::purge to the next safepoint
> >> cleanup is
> >> sound.
> > 
> > Oh great!  I'm glad you agree.
> > 
> >> I like the removal of CLDG::contains since it makes more sense to ask
> >> Metaspace straight up.
> >> 
> >> I have a small question about the test you added:
> >> 
> >> 3798   static void test_contains() {
> >> 3799     // Test that all the methods in the CLDG are contained
> >> 3800     ClassLoaderDataGraph::methods_do(assert_contains);
> >> 3801   }
> >> 
> >> Have we always allocated methods at this point in the VM startup?
> > 
> > I thought this ExecuteVMInternalTests (or whatever the option is
> > called) is at shutdown?  I'll check.
> > 
> >> Should we also test CLDG::classes_do to make sure it works for the
> >> compressed
> >> class space?
> > 
> > I had that same thought this morning.  Yes, I'll add it.
> > 
> > Coleen
> > 
> >> /Mikael
> >> 
> >>> Marcus L. tested it with nashorn tests.
> >>> 
> >>> Thanks,
> >>> Coleen