Ioi, Looks good! Thanks to all who have contributed!
A couple of minor comments/questions: 1. jvm.h (hotspot and jdk) All three APIs talk about loader_type, but the code uses loader. 2. Launcher.java To the best of my understanding - the call to findLoadedClass does not require synchronizing on the class loader lock, that is needed to ensure find/define atomicity - so that we do not call defineClass twice on the same class - i.e. in loadClass - it is needed around the findLoadedClass / findClass(defineClass) calls. This call is just a SystemDictionary lookup and does not require the lock to be held. David H and Mandy - does that make sense to you both? thanks, Karen On Oct 28, 2014, at 12:38 AM, Ioi Lam wrote: > > On 10/27/14, 7:04 PM, Mandy Chung wrote: >> >> On 10/27/2014 3:32 PM, Ioi Lam wrote: >>> Hi David, I have update the latest webrev at: >>> >>> http://cr.openjdk.java.net/~iklam/8061651-lookup-index-open-v3/ >>> >> >> The update looks good. Thanks. >> >>> This version also contains the JDK test case that Mandy requested: >>> >>> http://cr.openjdk.java.net/~iklam/8061651-lookup-index-open-v3/jdk/test/sun/misc/URLClassPath/EnableLookupCache.java.html >>> >>> >> >> What I request to add is a test setting the system property >> (-Dsun.cds.enableSharedLookupCache=true) and continue to load class A and B. >> Removing line 44-58 should do it and also no need to set -Dfoo.foo.bar. >> > Do you mean change the test to call > System.setProperty("sun.cds.enableSharedLookupCache", "true")? > > But we know that the property is checked only once, before any app classes > are loaded. So calling System.setProperty in an application class won't test > anything. >> It'd be good if you run this test and turn on the debug traces to make sure >> that the application class loader and ext class loader will start up with >> the lookup cache enabled and make up call to the VM. As it doesn't have the >> app cds archive, it will invalidate the cache right away and continue the >> class lookup with null cache array. > In the latest code, if CDS is not available, lookupCacheEnabled will be set > to false inside the static initializer of URLClassPath: > > private static volatile boolean lookupCacheEnabled > = > "true".equals(VM.getSavedProperty("sun.cds.enableSharedLookupCache")); > > later, when the boot/ext/app loaders call into here: > > synchronized void initLookupCache(ClassLoader loader) { > if ((lookupCacheURLs = getLookupCacheURLs(loader)) != null) { > lookupCacheLoader = loader; > } else { > // This JVM instance does not support lookup cache. > disableAllLookupCaches(); > } > } > > their lookupCacheURLs[] fields will all be set to null. As a result, > getLookupCacheForClassLoader and knownToNotExist0 will never be called. > > I can add a DEBUG_LOOKUP_CACHE trace inside disableAllLookupCaches to print > "lookup cache disabled", and check for that in the test. Is this OK? > > Thanks > - Ioi > > >
