[vmkit-commits] [PATCH] Touchup ClassLoader class-caching policies.
Nicolas Geoffray
nicolas.geoffray at gmail.com
Tue Nov 29 11:57:21 PST 2011
Looks good!
On Tue, Nov 29, 2011 at 4:23 PM, Will Dietz <wdietz2 at illinois.edu> wrote:
> Inlined below.
>
> Refactor code that ensures a class is in a given classloader's
> classmap into new method "ensureCached".
>
> Additionally, drop some short-circuit logic that looks directly to
> bootstrap loader and in a few places call ensureCached where we didn't
> before. Idea is to ensure that if a class is loaded through a
> particular classloader that it should be cached there.
>
> Also drop the redundant code in loadClassFromJavaString while we're at it
> :).
>
> FWIW new behavior causes both runtimes to pass mauve's classloader
> tests (neither did before), but of course this change should make
> sense beyond that also.
>
> ~Will
>
> >From d1d307c9e14a9a01d11f5dd382327ace5c9a8cd0 Mon Sep 17 00:00:00 2001
> From: Will Dietz <w at wdtz.org>
> Date: Tue, 29 Nov 2011 09:11:27 -0600
> Subject: [PATCH 2/2] Touchup ClassLoader class-caching policies.
>
> Essentially, if a class is loaded by a call to a particular classloader it
> should be cached there.
> ---
> lib/j3/ClassLib/OpenJDK/OpenJDK.inc | 3 +-
> lib/j3/VMCore/JnjvmClassLoader.cpp | 66
> ++++++++++------------------------
> lib/j3/VMCore/JnjvmClassLoader.h | 4 ++
> 3 files changed, 25 insertions(+), 48 deletions(-)
>
> diff --git a/lib/j3/ClassLib/OpenJDK/OpenJDK.inc
> b/lib/j3/ClassLib/OpenJDK/OpenJDK.inc
> index 1b20596..f5e0c3c 100644
> --- a/lib/j3/ClassLib/OpenJDK/OpenJDK.inc
> +++ b/lib/j3/ClassLib/OpenJDK/OpenJDK.inc
> @@ -1312,8 +1312,7 @@ JVM_FindLoadedClass(JNIEnv *env, jobject
> _loader, jstring name) {
> CommonClass * Cl = 0;
>
> JCL = JnjvmClassLoader::getJnjvmLoaderFromJavaObject(loader, vm);
> - if (JCL)
> - Cl = JCL->lookupClassFromJavaString(str);
> + Cl = JCL->lookupClassFromJavaString(str);
> jclass res = Cl ? (jclass)Cl->getClassDelegateePtr(vm) : 0;
> RETURN_FROM_JNI(res);
>
> diff --git a/lib/j3/VMCore/JnjvmClassLoader.cpp
> b/lib/j3/VMCore/JnjvmClassLoader.cpp
> index 2a77030..f8e5980 100644
> --- a/lib/j3/VMCore/JnjvmClassLoader.cpp
> +++ b/lib/j3/VMCore/JnjvmClassLoader.cpp
> @@ -344,6 +344,12 @@ UserClass* JnjvmClassLoader::loadName(const UTF8*
> name, bool doResolve,
> vm->noClassDefFoundError(name);
> }
>
> + ensureCached(cl);
> +
> + return cl;
> +}
> +
> +void JnjvmClassLoader::ensureCached(UserCommonClass* cl) {
> if (cl && cl->classLoader != this) {
> classes->lock.lock();
> ClassMap::iterator End = classes->map.end();
> @@ -352,8 +358,6 @@ UserClass* JnjvmClassLoader::loadName(const UTF8*
> name, bool doResolve,
> classes->map.insert(std::make_pair(cl->name, cl));
> classes->lock.unlock();
> }
> -
> - return cl;
> }
>
>
> @@ -414,6 +418,7 @@ UserCommonClass*
> JnjvmClassLoader::lookupClassOrArray(const UTF8* name) {
>
> if (this != bootstrapLoader) {
> temp = bootstrapLoader->lookupClassOrArray(name);
> + ensureCached(temp);
> if (temp) return temp;
> }
>
> @@ -450,6 +455,7 @@ UserCommonClass*
> JnjvmClassLoader::loadClassFromUserUTF8(const UTF8* name,
> if (prim) return constructArray(name);
> if (componentName) {
> UserCommonClass* temp = loadName(componentName, doResolve,
> doThrow, NULL);
> + ensureCached(temp);
> if (temp) return constructArray(name);
> }
> } else {
> @@ -464,7 +470,6 @@ UserCommonClass*
> JnjvmClassLoader::loadClassFromAsciiz(const char* asciiz,
> bool doThrow) {
> const UTF8* name = hashUTF8->lookupAsciiz(asciiz);
> vmkit::ThreadAllocator threadAllocator;
> - UserCommonClass* result = NULL;
> if (!name) name = bootstrapLoader->hashUTF8->lookupAsciiz(asciiz);
> if (!name) {
> uint32 size = strlen(asciiz);
> @@ -478,16 +483,6 @@ UserCommonClass*
> JnjvmClassLoader::loadClassFromAsciiz(const char* asciiz,
> name = temp;
> }
>
> - result = lookupClass(name);
> - if ((result == NULL) && (this != bootstrapLoader)) {
> - result = bootstrapLoader->lookupClassOrArray(name);
> - if (result != NULL) {
> - if (result->isClass() && doResolve) {
> - result->asClass()->resolveClass();
> - }
> - return result;
> - }
> - }
>
> return loadClassFromUserUTF8(name, doResolve, doThrow, NULL);
> }
> @@ -496,36 +491,22 @@ UserCommonClass*
> JnjvmClassLoader::loadClassFromAsciiz(const char* asciiz,
> UserCommonClass*
> JnjvmClassLoader::loadClassFromJavaString(JavaString* str, bool doResolve,
> bool doThrow) {
> -
> +
> llvm_gcroot(str, 0);
> vmkit::ThreadAllocator allocator;
> UTF8* name = (UTF8*)allocator.Allocate(sizeof(UTF8) + str->count *
> sizeof(uint16));
> -
> +
> name->size = str->count;
> - if (ArrayUInt16::getElement(JavaString::getValue(str), str->offset)
> != I_TAB) {
> - for (sint32 i = 0; i < str->count; ++i) {
> - uint16 cur = ArrayUInt16::getElement(JavaString::getValue(str),
> str->offset + i);
> - if (cur == '.') name->elements[i] = '/';
> - else if (cur == '/') {
> - return 0;
> - }
> - else name->elements[i] = cur;
> - }
> - } else {
> - for (sint32 i = 0; i < str->count; ++i) {
> - uint16 cur = ArrayUInt16::getElement(JavaString::getValue(str),
> str->offset + i);
> - if (cur == '.') {
> - name->elements[i] = '/';
> - } else if (cur == '/') {
> - return 0;
> - } else {
> - name->elements[i] = cur;
> - }
> + for (sint32 i = 0; i < str->count; ++i) {
> + uint16 cur = ArrayUInt16::getElement(JavaString::getValue(str),
> str->offset + i);
> + if (cur == '.') name->elements[i] = '/';
> + else if (cur == '/') {
> + return 0;
> }
> + else name->elements[i] = cur;
> }
> -
> - UserCommonClass* cls = loadClassFromUserUTF8(name, doResolve, doThrow,
> str);
> - return cls;
> +
> + return loadClassFromUserUTF8(name, doResolve, doThrow, str);
> }
>
> UserCommonClass* JnjvmClassLoader::lookupClassFromJavaString(JavaString*
> str) {
> @@ -585,15 +566,8 @@ UserClassArray*
> JnjvmClassLoader::constructArray(const UTF8* name) {
> assert(cl && "no base class for an array");
> JnjvmClassLoader* ld = cl->classLoader;
> res = ld->constructArray(name, cl);
> -
> - if (res && res->classLoader != this) {
> - classes->lock.lock();
> - ClassMap::iterator End = classes->map.end();
> - ClassMap::iterator I = classes->map.find(res->name);
> - if (I == End)
> - classes->map.insert(std::make_pair(res->name, res));
> - classes->lock.unlock();
> - }
> +
> + ensureCached(res);
> return res;
> }
>
> diff --git a/lib/j3/VMCore/JnjvmClassLoader.h
> b/lib/j3/VMCore/JnjvmClassLoader.h
> index c0f8a4f..b3bce46 100644
> --- a/lib/j3/VMCore/JnjvmClassLoader.h
> +++ b/lib/j3/VMCore/JnjvmClassLoader.h
> @@ -84,6 +84,10 @@ private:
> ///
> const UTF8* lookupComponentName(const UTF8* name, UTF8* holder, bool&
> prim);
>
> +
> + /// ensureCached - Make sure the specified class is in our classes map
> + ///
> + void ensureCached(UserCommonClass* cl);
> protected:
>
> JnjvmClassLoader(vmkit::BumpPtrAllocator& Alloc) : allocator(Alloc) {}
> --
> 1.7.5.1
> _______________________________________________
> vmkit-commits mailing list
> vmkit-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/vmkit-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/vmkit-commits/attachments/20111129/6b19a957/attachment.html>
More information about the vmkit-commits
mailing list