[vmkit-commits] [PATCH] Add support for extra VM-only fields, use this to add fields to j.l.Class in OpenJDK

[Nicolas Geoffray]

Thanks Will for the simplified patch! It almost looks good, with the
exception of the forgotten write barrier. Feel free to commit once that is
fixed.

On Mon, Oct 31, 2011 at 9:44 PM, Will Dietz <wdietz2 at illinois.edu> wrote:

> On Fri, Oct 28, 2011 at 5:31 PM, Nicolas Geoffray
> <nicolas.geoffray at gmail.com> wrote:
> > Hi Will!
> > I think this patch could work, but I would definitely try a more
> 'localized'
> > approach before doing it this way.
> > I think that in the bootstrap process of OpenJDK, you should update the
> > Class::virtualSize field for java.lang.Class to include these two new
> > fields, and that would be it. No need to extend the JavaField array (the
> > Java side of things does not have to know we added new fields).
> > Of course, you need to make sure no java.lang.Class objects are allocated
> > *before* you change the size.
> > Thanks, and let me know if that worked!
> > Nicolas
>
> I was playing nice with the JavaFields not so that anything from Java
> could see the fields (I actually intentionally tried to hide them from
> Java code), but so that the created LLVM types (LLVMInfo) for
> java.lang.Class matched up.  Not knowing better, that seemed like
> something to ensure was correct.
>
> However, what you suggested is much simpler and seems to work equally
> well, thanks for the good suggestion.
>
> Updated patch inlined below :)
>
> ~Will
>
> From f57fed146de2962a5e3c1306515ab9374862647e Mon Sep 17 00:00:00 2001
> From: Will Dietz <w at wdtz.org>
> Date: Mon, 31 Oct 2011 15:23:24 -0500
> Subject: [PATCH] Add extra fields to JavaObjectClass to hold CommonClass
> and
>  protection domain.
>
> ---
>  lib/J3/ClassLib/OpenJDK/ClasspathReflect.h |   13 +++++++------
>  lib/J3/ClassLib/OpenJDK/JavaUpcalls.cpp    |    2 ++
>  2 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/lib/J3/ClassLib/OpenJDK/ClasspathReflect.h
> b/lib/J3/ClassLib/OpenJDK/ClasspathReflect.h
> index 75d35a7..4075a5f 100644
> --- a/lib/J3/ClassLib/OpenJDK/ClasspathReflect.h
> +++ b/lib/J3/ClassLib/OpenJDK/ClasspathReflect.h
> @@ -38,30 +38,31 @@ private:
>   JavaObject* annotations;
>   JavaObject* declaredAnnotations;
>   JavaObject* annotationType;
> +  // Extra fields added by VM
> +  UserCommonClass * internalClass;
> +  JavaObject * pd;
>
>  public:
>
>   static UserCommonClass* getClass(JavaObjectClass* cl) {
>     llvm_gcroot(cl, 0);
> -    UNIMPLEMENTED();
> -    return NULL;
> +    return cl->internalClass;
>   }
>
>   static void setClass(JavaObjectClass* cl, UserCommonClass* vmdata) {
>     llvm_gcroot(cl, 0);
> -    UNIMPLEMENTED();
> +    cl->internalClass = vmdata;
>   }
>
>   static void setProtectionDomain(JavaObjectClass* cl, JavaObject* pd) {
>     llvm_gcroot(cl, 0);
>     llvm_gcroot(pd, 0);
> -    UNIMPLEMENTED();
> +    cl->pd = pd;
>

This should be a write barrier.


>   }
>
>   static JavaObject* getProtectionDomain(JavaObjectClass* cl) {
>     llvm_gcroot(cl, 0);
> -    UNIMPLEMENTED();
> -    return NULL;
> +    return cl->pd;
>   }
>
>   static void staticTracer(JavaObjectClass* obj, word_t closure) {
> diff --git a/lib/J3/ClassLib/OpenJDK/JavaUpcalls.cpp
> b/lib/J3/ClassLib/OpenJDK/JavaUpcalls.cpp
> index a4a48c8..476b6e3 100644
> --- a/lib/J3/ClassLib/OpenJDK/JavaUpcalls.cpp
> +++ b/lib/J3/ClassLib/OpenJDK/JavaUpcalls.cpp
> @@ -468,6 +468,8 @@ void
> Classpath::initialiseClasspath(JnjvmClassLoader* loader) {
>
>   newClass =
>     UPCALL_CLASS(loader, "java/lang/Class");
> +  // Add space for extra fields, see ClasspathReflect.h
> +  newClass->virtualSize += 2*sizeof(void*);
>

Did you actually make sure no java.lang.Class  objects were created before
executing that line? I'm not sure how you could ensure that with an assert.
But you could just trace allocations (printf in Class::doNew) manually and
see if it holds today.


>   newThrowable =
>     UPCALL_CLASS(loader, "java/lang/Throwable");
> --
> 1.7.5.1
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/vmkit-commits/attachments/20111031/66fd5450/attachment.html>