[vmkit-commits] [PATCH] Unsafe implementation

[Nicolas Geoffray]

On Mon, Oct 24, 2011 at 7:51 PM, Will Dietz <wdietz2 at illinois.edu> wrote:

> On Mon, Oct 24, 2011 at 1:27 AM, Nicolas Geoffray
> <nicolas.geoffray at gmail.com> wrote:
> > Are the patches attached the patches outstanding? I'm a little confused
> :)
> > Could you send one email per patch, and inline the patch int the email? I
> > think that will make things easier :)
>
> Forget what I said, I'll start new threads for any patches that need
> your attention :).
>
> > In any case, here are some comments on the Unsafe implementation:
> > +JNIEXPORT JavaObject* JNICALL Java_sun_misc_Unsafe_staticFieldBase(
> > +JavaObject* unsafe, JavaObjectField* _field) {
> > +  llvm_gcroot(unsafe, 0);
> > +  llvm_gcroot(_field, 0);
> > +  JavaObject* res = 0;
> > +  BEGIN_NATIVE_EXCEPTION(0)
> > +
> > +  JavaField * field = JavaObjectField::getInternalField(_field);
> > +  assert(field);
> > +  field->classDef->initialiseClass(JavaThread::get()->getJVM());
> > +
> > +  res = (JavaObject*)field->classDef->getStaticInstance();
> > Unfortunately, this does not work. The static instance of a class is a
> > malloc'ed memory, not allocated by the GC. I looked at the comments in
> > Unsafe.java, saying that it's fine for the method to return something
> that
> > is not an object, but that is plain.... evil!
> > It looks like we have two options:
> > 1) Return a dummy Object, that the 'put' and 'get' methods check for. The
> > dummy Object is specific to the VM, and contains a reference to the
> static
> > instance.
> > 2) Make the static instance a GC-allocated object.
> > Both cases are tricky to implement. The first one has the advantage to be
> > localized: you don't need to change code in the existing code base. The
> > second one does need lots of changes in the existing code base, including
> in
> > the JIT which assumes that a static instance is a non-heap object.
> > Therefore, I would go with option 1). Take a look at the VMClassLoader
> class
> > in JnjvmClassLoader.h to see how that works. We fake up a Java Object,
> that
> > will be scanned by the GC. In your case, it looks like in order to stay
> > valid, the object must make sure that the class is not garbage collected.
>
> Okay.  The fake-java-object idea seems like a good idea.
>
> What are you suggesting with your "in order to stay valid" comment?
>

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.


>
> That said, could we instead return 'null' here, and return the
> (base+offset) as 'offset' in getStaticFieldOffset?  The documentation
> seems to suggest that is "allowed" (even on 64bit), and keeps things
> simple.  Thoughts?
>


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:

  673        * This value may be null.  This value may refer to an object
  674        * which is a "cookie", not guaranteed to be a real Object, and
it should
  675        * not be used in any way except as argument to the get and put
routines in
  676        * this class.
  677        */

(http://www.docjar.org/html/api/sun/misc/Unsafe.java.html)

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 :)

Therefore, our only long term solution is to create this object that makes
sure the class loader does not get GC'ed :(

Nicolas



> ~Will
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/vmkit-commits/attachments/20111024/56073c88/attachment.html>