[vmkit-commits] [PATCH] Unsafe implementation

[Will Dietz]

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?

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?

~Will