This is the mail archive of the java-patches@gcc.gnu.org mailing list for the Java project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RE: PATCH: Reduce allocation overhead, fix IA64 GC descriptors


Thanks.

> -----Original Message-----
> From: Tom Tromey [mailto:tromey@redhat.com]
...
> Hans> 3) Fix the dynamic test for _Jv_AllocObject so that it works
> Hans> correctly on IA64.  The current code registers all objects for
> Hans> finalization on IA64, which has horrible performance
> Hans> implications.  (I was hoping to remove the dynamic test
> Hans> completely.  But that would have CNI and pobably JNI
> Hans> implications.  So I put it back in, but it's now infrequently
> Hans> executed.)
> 
> We could have the compiler generate two calls for each allocation: one
> to allocate the object and then another one to register the finalizer.
> Then the dynamic test would only apply to CNI/JNI allocations.
> 
> That would make the generated code larger.  Another choice would be to
> add a third allocation entry point: allocate when we know a finalizer
> is required.
That could be done.  I'm not sure it's worth optimizing this case.  If you
need to register a finalizer, the allocation will be expensive anyway, so
the test doesn't add much.  I do want to get the test out of the "fast"
path, which this patch should do.

> 
> Hans> 	gcc/java/class.c: Fix java vtable layout for
> Hans> 		TARGET_VTABLE_USES_DESCRIPTORS
> Hans> [ ... ]
> 
> First, could you write a detailed ChangeLog entry?
How's this:

	gcc/java/class.c(get_dispatch_table),
	gcc/java/expr.c(build_invokevirtual): Fix java vtable layout for
		TARGET_VTABLE_USES_DESCRIPTORS.  Add only one additional
		descriptor to the vtable for java.  The first word of the
		vtable is the class pointer and the second is the GC
descriptor
		in all cases.
	libjava/java/lang/Object.h, libjava/include/jvm.h: Adjust for
		revised vtable layout on IA64.  With 
		TARGET_VTABLE_USES_DESCRIPTORS, there is only one extra
		descriptor.

	gcc/java/decl.c (java_init_decl_processing),gcc/java/java-tree.h:
		Add alloc_no_finalizer_node to allow allocation of objects
		known to not require finalization.
	gcc/java/expr.c(expand_java_NEW), gcc/java/parse.y(patch_invoke):
		Generate calls for finalizer-free allocation.
	libjava/prims.cc, libjava/gcj/javaprims.h: Add
		_Jv_AllocObjectNoInitNoFinalizer, _Jv_AllocObjectNoFinalizer
		to support fionalizer-free allocation.

	libjava/prims.cc: Some old cleanups.  The collector now handles test
		for out of memory.
> 
> Hans> +#define FINALIZE_INDEX 0
> 
> There's got to be another way to do this.
I'm open to suggestions.  We already assume in the runtime that
java.lang.Object.finalize() is the first entry in the vtable.  Thus I don't
think this makes anything more brittle than it already is.  And checking for
a method override for slot 0 seems to be the right test.  But I don't know
the front end terribly well, and there may be completely different and
better ways to test for the presence of a finalizer.  My impression is that
this one is at least not horribly slow, unlike some other alternatives I
started looking at.

> 
> Hans> +static void jvmpi_notify_alloc(jclass klass, jint 
> size, jobject obj)
> 
> Need a newline after the `void'.
> 
> Hans> +  jvmpi_notify_alloc(klass, size, obj);
> 
> Need a space before the `('.
> There are a few of these.
Fixed in my tree for the next version of the patch.  Thanks.

> 
> Hans> +# ifndef __ia64__
> Hans> +    if (klass->vtable->get_finalizer()
> Hans> +        != java::lang::Object::class$.vtable->get_finalizer())
> Hans> +      _Jv_RegisterFinalizer (obj, _Jv_FinalizeObject);
> Hans> +# else
> Hans> +    // On IA64, the vtable contains (pc,gp) pairs, not 
> function pointers.
> Hans> +    // Get_finalizer just returns a pointer to the 
> vtable entry, which will
> Hans> +    // never match the one in object.  Hence we 
> actually compare the pc
> Hans> +    // fields.
> Hans> +    if (((void **)klass->vtable->get_finalizer())[0]
> Hans> +        != ((void **)
> Hans> +	     
> java::lang::Object::class$.vtable->get_finalizer())[0])
> Hans> +      _Jv_RegisterFinalizer (obj, _Jv_FinalizeObject);
> Hans> +# endif
> 
> The intent was for get_finalizer() to return the actual finalizer, not
> the address of the finalizer's slot.  This method exists only to let
> us do this test.  It has no other use.  So in theory you shouldn't
> have to do this.  I'm guessing that the get_finalizer() code in jvm.h
> inadvertently had its meaning changed by rth's patch.
To clarify (at the risk of reciting the obvious):  On IA64, a function
descriptor is a struct containing the pc (the address of the first
instruction), and gp (the required global offset table pointer when running
the function, set up by the caller).  Function pointers are pointers to such
a struct.  Vtables now contain the structs corresponding to the methods
directly instead of pointers to them.  (Except for the vtable part, I think
this is very similar to the PowerPC convention for function pointers, for
example.)

It sounds like get_finalizer() should just return the pc component of the
descriptor on IA64, since that uniquely identifies the function.  That
simplifies the code, and leaves performance unchanged.  It's a little
counter-intuitive since, at least on IA64, the result can't be called.  But
it does solve this problem.  If we agree, I can go ahead and make that
change.  (It could also return the struct as a whole, but that adds cost,
and is only slightly less confusing.)

> 
> Looking through the code, it looks like there are several places that
> assume that the vtable element is a function pointer.  For instance
> look at this code in resolve.cc:
> 
> 	    vtable->set_method(index, (void*)&_Jv_abstractMethodError);
> 
I think this is fine as it is.  RTH alread fixed set_method to copy the
structure instead of the pointer.

> There is code in the other direction in interpret.cc:
> 
> 	    fun = (void (*)()) table->get_method(rmeth->vtable_index);
> 
He also fixed get_method to return a pointer to the vtable entry, which is
then a valid function pointer, so this should also be fine as it is.

> How can we make this work?  What is the meaning of the `pc' and `gp'
> fields?  I have a partial patch, appended, but it doesn't fix problems
> like the above :-(.  If you can tell me how to fix it, I will do so.
> 
I'm not sure I would do it this way.  You correctly point out another
problem in _Jv_MarkObj and _Jv_MarkArray.  I had missed those.  But I think
if we change get_finalizer() to just return the PC on IA64, this magically
gets fixed, though perhaps too magically.  I don't think there's any need to
compare gp values to check function equality; a given instruction sequence
will need a fixed global offset table pointer.

I think you really want to check the (first word of) the finalizer entry in
the vtable, not the class pointer.  For a proper initialized object, that's
guaranteed to be nonzero.  For a free list object of sufficient size, that's
guaranteed to be zero.  The same isn't true for the class slot.

If you agree, I will generate a revised patch.

Hans

> Tom
> 
> 
> Index: ChangeLog
> from  Tom Tromey  <tromey@redhat.com>
> 
> 	* prims.cc (_Jv_AllocObject): Use _Jv_VTable::methods_equal.
> 	* boehm.cc (_Jv_MarkObj): Look at `clas', not 
> finalizer, to see if
> 	object is cleared.
> 	(_Jv_MarkArray): Likewise.
> 	* include/jvm.h (_Jv_VTable::get_method) [__ia64__]: Return PC
> 	element.
> 	(_Jv_VTable::get_method, _Jv_VTable::set_method): No longer
> 	conditional.
> 	(_Jv_VTable::methods_equal): New method.
> 
> Index: boehm.cc
> ===================================================================
> RCS file: /cvs/gcc/gcc/libjava/boehm.cc,v
> retrieving revision 1.31
> diff -u -r1.31 boehm.cc
> --- boehm.cc 2001/11/12 10:42:45 1.31
> +++ boehm.cc 2001/11/21 22:05:06
> @@ -89,7 +89,7 @@
>    // This assumes Java objects have size at least 3 words,
>    // including the header.   But this should remain true, since this
>    // should only be used with debugging allocation or with 
> large objects.
> -  if (__builtin_expect (! dt || !(dt -> get_finalizer()), false))
> +  if (__builtin_expect (! dt || !(dt->clas), false))
>      return mark_stack_ptr;
>    jclass klass = dt->clas;
>    ptr_t p;
> @@ -295,7 +295,7 @@
>    // Assumes size >= 3 words.  That's currently true since 
> arrays have
>    // a vtable, sync pointer, and size.  If the sync pointer 
> goes away,
>    // we may need to round up the size.
> -  if (__builtin_expect (! dt || !(dt -> get_finalizer()), false))
> +  if (__builtin_expect (! dt || !(dt->clas), false))
>      return mark_stack_ptr;
>    jclass klass = dt->clas;
>    ptr_t p;
> Index: prims.cc
> ===================================================================
> RCS file: /cvs/gcc/gcc/libjava/prims.cc,v
> retrieving revision 1.62
> diff -u -r1.62 prims.cc
> --- prims.cc 2001/10/23 05:42:02 1.62
> +++ prims.cc 2001/11/21 22:05:08
> @@ -353,8 +353,8 @@
>    // implementation would look for Object.finalize in Object's method
>    // table at startup, and then use that information to find the
>    // appropriate index in the method vector.
> -  if (klass->vtable->get_finalizer()
> -      != java::lang::Object::class$.vtable->get_finalizer())
> +  if (_Jv_VTable::methods_equal (klass->vtable->get_finalizer(),
> +				 
> java::lang::Object::class$.vtable->get_finalizer()))
>      _Jv_RegisterFinalizer (obj, _Jv_FinalizeObject);
>  
>  #ifdef ENABLE_JVMPI
> Index: include/jvm.h
> ===================================================================
> RCS file: /cvs/gcc/gcc/libjava/include/jvm.h,v
> retrieving revision 1.45
> diff -u -r1.45 jvm.h
> --- include/jvm.h 2001/10/23 05:42:03 1.45
> +++ include/jvm.h 2001/11/21 22:05:09
> @@ -45,16 +45,20 @@
>    // adding new data members.
>    vtable_elt method[1];
>  
> +  vtable_elt get_method (int i) { return method[i]; }
> +  void set_method (int i, vtable_elt elt) { method[i] = elt; }
> +
> +  static void methods_equal (vtable_elt e1, vtable_elt e2)
> +  {
>  #ifdef __ia64__
> -  void *get_method(int i) { return &method[i]; }
> -  void set_method(int i, void *fptr) { method[i] = 
> *(vtable_elt *)fptr; }
> +    return e1.pc == e2.pc && e1.gp == e2.gp;
>  #else
> -  void *get_method(int i) { return method[i]; }
> -  void set_method(int i, void *fptr) { method[i] = fptr; }
> +    return e1 == e2;
>  #endif
> +  }
>  
> -  void *get_finalizer() { return get_method(0); }
> -  static size_t vtable_elt_size() { return sizeof(vtable_elt); }
> +  vtable_elt get_finalizer() { return get_method(0); }
> +  static size_t vtable_elt_size() { return sizeof (vtable_elt); }
>    static _Jv_VTable *new_vtable (int count);
>  };
>  
> 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]