On Mon, Oct 24, 2011 at 7:51 PM, Will Dietz <span dir="ltr"><<a href="mailto:wdietz2@illinois.edu">wdietz2@illinois.edu</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
On Mon, Oct 24, 2011 at 1:27 AM, Nicolas Geoffray<br>
<<a href="mailto:nicolas.geoffray@gmail.com">nicolas.geoffray@gmail.com</a>> wrote:<br>
> Are the patches attached the patches outstanding? I'm a little confused :)<br>
> Could you send one email per patch, and inline the patch int the email? I<br>
> think that will make things easier :)<br>
<br>
Forget what I said, I'll start new threads for any patches that need<br>
your attention :).<br>
<br>
> In any case, here are some comments on the Unsafe implementation:<br>
> +JNIEXPORT JavaObject* JNICALL Java_sun_misc_Unsafe_staticFieldBase(<br>
> +JavaObject* unsafe, JavaObjectField* _field) {<br>
> +  llvm_gcroot(unsafe, 0);<br>
> +  llvm_gcroot(_field, 0);<br>
> +  JavaObject* res = 0;<br>
> +  BEGIN_NATIVE_EXCEPTION(0)<br>
> +<br>
> +  JavaField * field = JavaObjectField::getInternalField(_field);<br>
> +  assert(field);<br>
> +  field->classDef->initialiseClass(JavaThread::get()->getJVM());<br>
> +<br>
> +  res = (JavaObject*)field->classDef->getStaticInstance();<br>
> Unfortunately, this does not work. The static instance of a class is a<br>
> malloc'ed memory, not allocated by the GC. I looked at the comments in<br>
> Unsafe.java, saying that it's fine for the method to return something that<br>
> is not an object, but that is plain.... evil!<br>
> It looks like we have two options:<br>
> 1) Return a dummy Object, that the 'put' and 'get' methods check for. The<br>
> dummy Object is specific to the VM, and contains a reference to the static<br>
> instance.<br>
> 2) Make the static instance a GC-allocated object.<br>
> Both cases are tricky to implement. The first one has the advantage to be<br>
> localized: you don't need to change code in the existing code base. The<br>
> second one does need lots of changes in the existing code base, including in<br>
> the JIT which assumes that a static instance is a non-heap object.<br>
> Therefore, I would go with option 1). Take a look at the VMClassLoader class<br>
> in JnjvmClassLoader.h to see how that works. We fake up a Java Object, that<br>
> will be scanned by the GC. In your case, it looks like in order to stay<br>
> valid, the object must make sure that the class is not garbage collected.<br>
<br>
Okay.  The fake-java-object idea seems like a good idea.<br>
<br>
What are you suggesting with your "in order to stay valid" comment?<br></blockquote><div><br></div><div>The problem is that if the class gets GC'ed, the class loader will deallocate what it allocated, including the static instances. So somehow we need to make sure the class loader stays alive.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<br>
That said, could we instead return 'null' here, and return the<br>
(base+offset) as 'offset' in getStaticFieldOffset?  The documentation<br>
seems to suggest that is "allowed" (even on 64bit), and keeps things<br>
simple.  Thoughts?<br></blockquote><div><br></div><div><br></div><div>That could be a possibility, but unfortunately does not solve the problem of the class loader being GC'ed. So although I like the approach of returning null and using the (base + offset), it will only work if we don't GC classes. Or if we are sure that the comment in the class is right:</div>
<div><br></div><div><div>  673        * This value may be null.  This value may refer to an object</div><div>  674        * which is a "cookie", not guaranteed to be a real Object, and it should</div><div>  675        * not be used in any way except as argument to the get and put routines in</div>
<div>  676        * this class.</div><div>  677        */</div></div><div><br></div><div>(<a href="http://www.docjar.org/html/api/sun/misc/Unsafe.java.html">http://www.docjar.org/html/api/sun/misc/Unsafe.java.html</a>)</div>
<div><br></div><div>then it would be OK. But I think anyone can use Unsafe.java, so that does not hold. I hate this Unsafe class, it's plain stupid :)</div><div><br></div><div>Therefore, our only long term solution is to create this object that makes sure the class loader does not get GC'ed :(</div>
<div><br></div><div>Nicolas</div><div> </div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<font color="#888888"><br>
~Will<br>
</font></blockquote></div><br>