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