Re: [cp-patches] Some VMStackWalker cleanups/simplifications

[Archie Cobbs]

Mark Wielaard wrote:
> > > Right. It isn't needed for the Method.invoke() case, but the default
> > > implementation got this wrong so this fix was also included. The Class
> > > parameter is needed for those cases where the call stack depth is not
> > > precisely know, either because the call was made through some other
> > > method or class, the call was inlined by the compiler or runtime or
> > > because the runtime actually implements these methods by calling other
> > > classes written in java again (kaffe and jikesrvm do this for 
> > > example).
> >
> > There is nothing in the existing interface that prevents these VMs from
> > working. I strongly object to making these incredibly lame changes, just
> > to make it a little easier for them.
>
> I think that is a little strong. I think it makes the interface a lot
> easier, robust and hopefully more efficient to implement. But if no
> runtime hacker likes it this way I will drop it. This was just done to
> make it easier for some runtimes like jamvm, sablevm and kaffe that are
> currently broken with the current default implementation or cannot adopt
> the current interface easily.

I'd like to better understand the issue with those VMs. It seems to
me that VMStackBrowser and VMStackWalker can both be implemented in
terms of the other (one's an iterator, one returns an array). So how
can a VM be able to implement one API but not the other?

Or is this just a matter of convenience, as one way is closer to
the way those particular VM's already do it? If that's the case then
we're getting into the murky waters of VM-specific "convenience code".

I agree passing the Class parameter is a bit ugly in any case.

Finally, it seems that instead of this patch implementing
VMStackBrowser in terms of VMStackWalker, it should be done the other
way around, so that VMStackBrowser is the native code, and VMStackWalker
is implemented in terms of VMStackBrowser. Then the API doesn't encourage
the inefficient creation of the entire Class[] array every time, which is
sort of the whole point of this patch, right? Of course, that's probably
inconvenient for the other VMs , who may protest...

-Archie

__________________________________________________________________________
Archie Cobbs      *        CTO, Awarix        *      http://www.awarix.com