Re: PATCH: Constant-time interface dispatch, type checking, inlining...

[Bryce McKinlay <bryce at albatross dot co dot nz>]

Tom Tromey wrote:

> Hey Bryce.  I have some questions & comments about the patch:
>
> * Why introduce the JV_CLASS macro?  Why not just make Object.getClass
>   an inline function?

I tried to do this, but the method depends on the definition of
_Jv_Vtable which is not available from Object.h or Class.h without some
ugly reorginisation of header files (its in jvm.h I think)

> * _Jv_LoookupInterfaceMethod just calls _Jv_LoookupInterfaceMethod0
>   with the same arguments.  Can't we get rid of one of these?

Hopefully ;-). The compiler was complaining that I couldn't redefine a
method with C linkage with C++ linkage (it needs to be a friend of
Class). I suppose there is a way around this, the extra method was just
me being lazy! Any ideas?

> * _Jv_IsAssignableFrom should return jboolean, not int.

I know, but if its declared as jboolean, the method just doesn't work
when compiled with the 2.95.2 compiler and -O2. I spent ages trying to
figure that out. There is something wrong with the optimizers handling of
jboolean I think. With jboolean the return value never gets set even
though I can debug it and watch it actually stop on a "return false"
line. On the current cvs compiler it appears to work ok, but I'd like the
code to work with 2.95.2 for now.

> * In _Jv_IsAssignableFrom, formatting is wrong.  Put "||" at
>   the start of a line, not the end.  Likewise "&&".

> * A real nit: _Jv_CheckCast should return "void *", not "void*".
>   (another formatting cleanup)

> * Functions should probably use jshort and not short.

ok x3.

> * Isn't there a race if _Jv_FindIIndex is called from two different
>   threads before it has initialized?  Or do we know this will be run
>   during initialization (when there is only a single thread)?  If so,
>   we should add a comment to that effect.

Yeah, I think it is pretty safe to assume that it will allways be
initilized single threaded (when java.lang.Thread is loaded it will
initialize for java.lang.Runnable).  I will add a comment to this effect.

> * I notice the JvNew<prim>Array functions no longer have C linkage.
>   Does anybody know why they had C linkage in the first place?
>   (This probably isn't a real problem.)
>
> Anyway, it looks good to me. Other gating factors:
>
> * Getting exceptions thrown at the right time

I thought about that some more. If an interface is changed after code has
been compiled against it, then the method indexes within the compiled
interface arn't going to match with the existing
_Jv_LookupInterfaceMethod calls generated by the compiler. It would be
okay so long as new methods are only added to the end of interfaces, but
perhaps it would be better to just reliably fail when a class is loaded
that doesn't match its interface (like the IBM JDK does) rather than
potentially failing in mysterious ways.

As Per said, we already break Java's binary compatibility rules in
various ways so its not really a big deal if we break here as well. What
do you think?

> * Fixing the interpreter

> * And, of course, the compiler changes
>
> Once these things are cleaned up I think it is good to go in.
> Thanks a lot!  This is really awesome stuff!

Thanks!

regards

  [ bryce ]