Re: [cp-patches] [generics] RFC : implementation of jjava.lang.instrument package

[Mark Wielaard]

Hi Nicolas,

On Mon, 2005-11-28 at 19:08 +0100, Nicolas Geoffray wrote:
> Here's two files, InstrumentationImpl and VMInstrumentationImpl.
> I put all native vm specific methods in VMInstrumentationImpl, but
> if you think it's not necessary, it's ok with me to have all these methods
> in InstrumentationImpl.

This looks pretty good. Thanks!
Some comments and questions:

InstrumentationImpl should implement Instrumentation (then a couple of
methods need to be made public).

I believe the Vector of transformaers is handled correctly even when
manipulated by multiple threads. But I like using an ArrayList and
Iterator and do explicit synchronization (but in this case that is
clearly just a personal preference, so feel free to ignore if you
disagree that it is clearer.)

How atomic do we want applyRedefineClasses() to be? Instead of looping
through all classes to redefine and calling applyRedefineClass() on each
one, it might be a good idea to have an applyRedefineClasses(Object[])
in VMInstrumentationImpl to redefine them all at the same time after
preparing them all. Also if something went wrong with the transformation
of any of them we might want to have a cancelRedefineClass() to let the
VM know we won't be applying any prepared classes.

Why does the private callTransformers() implementation take a
ProtectionDomain? Isn't that classBeingRedefined.getProtectionDomain()?

About the calltransformers FIXME. What else can/should we do here?

Normally we like to provide some default/noop implementation for the VM
reference classes. Is it possible to provide the necessary hooks in
ClassLoader/VMClassLoader to call the transformers just before
defineClass() is called? If we do this wouldn't it be slightly easier to
have all these classes in vm/reference/java/lang to make package access
easier?

An example how this is all supposed to work together when a VM
implements this VM interface would be nice. Also to make clear whether
the premain() method is an instance of static method, what command line
extensions/flags are expected, etc.

Cheers,

Mark