Re: [cp-patches] [generics] RFC : implementation of jjava.lang.instrument package
Hi Christian, Christian Thalinger wrote: On Fri, 2005-12-02 at 12:22 +0100, Nicolas Geoffray wrote: gnu/java/lang/instrument. Would you like InstrumentationImpl to be in vm/reference/java/lang/InsutrumentationImpl? This sounds weird. We could also make the callTransformers method public. I think it's very helpful for vm implementors that all vm interface stuff is _only_ located in vm/reference. Just my 2 cents... I couldn't agree more. What surprised me is not the vm/reference/* emplacement Mark suggested, but that VMInstrumentationImpl and Instrumentation had to go in the java.lang package, whereas I was more thinking to gnu.java.lang package. But Mark is right when he says it will be easier with security and package private method issues to have the files in the java.lang package. Cheers, Nicolas ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
Re: [cp-patches] [generics] RFC : implementation of jjava.lang.instrument package
On Fri, 2005-12-02 at 12:22 +0100, Nicolas Geoffray wrote: gnu/java/lang/instrument. Would you like InstrumentationImpl to be in vm/reference/java/lang/InsutrumentationImpl? This sounds weird. We could also make the callTransformers method public. I think it's very helpful for vm implementors that all vm interface stuff is _only_ located in vm/reference. Just my 2 cents... TWISTI ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
Re: [cp-patches] [generics] RFC : implementation of jjava.lang.instrument package
Hi Nicolas, On Fri, 2005-12-02 at 17:52 +0100, Nicolas Geoffray wrote: Last RFC, and I'm commiting this in 2005-12-02 Nicolas Geoffray [EMAIL PROTECTED] * java/lang/instrument: New directory. * java/lang/instrument/ClassDefinition.java: New file. * java/lang/instrument/ClassFileTransformer.java: New file. * java/lang/instrument/IllegalClassFormatException.java: New file. * java/lang/instrument/Instrumentation.java: New file. * java/lang/instrument/UnmodifiableClassException.java: New file. * vm/reference/java/lang/InstrumentationImpl.java: New file. * vm/reference/java/lang/VMInstrumentationImpl.java: New file. Looks very nice! Thanks. One change request: - Please put only VMInstrumentationImpl into vm/reference/java/lang and put InstrumentationImpl (which is shared and for which we don't expect people to change it per VM) into java/lang/ directly. Two nitpicks: - We check for null in InstrumentationImpl.getObjectSize() so the VMInstrumentationImpl.getObjectSize() documentation should say that. - There should be a space after if, while and synchronized according to our coding style guides. When you commit this can you please send a little message explaining the new instrumentation addition and how runtime hackers are supposed to provide support for it to the main classpath mailinglist? That way we hopefully get more feedback and make sure runtime hackers aren't completely surprised when this pops up in the next (generics) release. Thanks, Mark ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
Re: [cp-patches] [generics] RFC : implementation of jjava.lang.instrument package
Hi Mark,
Mark Wielaard wrote:
My idea was even that a VM might only want to have one redefine
prepare/apply cycle running at the same time. Then it can easily cleanup
anything not used. But this might be optimizing for an unusual case. We
can probably assume that transformers will actually transform correctly.
You're deleting all shared code I came up with :) I switched
to your idea because it reduces the number of native methods.
Aha. Sorry I missed the dual nature of the
ClassFileTransFormer.transform() method. I see you documented this in
your original patch. In that case I really do think we should move these
to vm/reference/java/lang and make this method package private.
Which method? callTransformers? It's a method from the
InstrumentationImpl class, therefore it's in
gnu/java/lang/instrument. Would you like InstrumentationImpl
to be in vm/reference/java/lang/InsutrumentationImpl? This sounds
weird. We could also make the callTransformers method public.
Right, I see we need to break the VM interface for this :{
Probably best not to do that for the first release (0.20) but lets
suggest how we would like to change the interface for 0.21 so runtime
implementers can prepare and scream about it. On the other hand this is
the generics branch only, not very many VMs depend on it yet. But not
very urgent I guess before we have this working with at least one VM.
I'm working on it (the jnjvm can now execute with the generics branch,
yeah :)). OK for making suggestions, where should it be written?
Just make a new paragraph for Instrumentation. Then mention that it
currently only is on the generics branch since it uses some new language
features.
OK then.
Cheers,
Nicolas
___
Classpath-patches mailing list
Classpath-patches@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath-patches
Re: [cp-patches] [generics] RFC : implementation of jjava.lang.instrument package
Hi Nicolas, On Fri, 2005-12-02 at 12:22 +0100, Nicolas Geoffray wrote: Aha. Sorry I missed the dual nature of the ClassFileTransFormer.transform() method. I see you documented this in your original patch. In that case I really do think we should move these to vm/reference/java/lang and make this method package private. Which method? callTransformers? It's a method from the InstrumentationImpl class, therefore it's in gnu/java/lang/instrument. Would you like InstrumentationImpl to be in vm/reference/java/lang/InsutrumentationImpl? This sounds weird. We could also make the callTransformers method public. Yes, but it makes it easier to share code with ClassLoader and VMClassLoader if we do this. Then callTransformers doesn't have to be public, but can just be package private. Then we don't have to worry about whether or not someone can get insecure access to a IntrumentationImpl (although I admit that is not very likely). It just seems more natural to me to have all implementation classes in the same package because they seem very related. I'm working on it (the jnjvm can now execute with the generics branch, yeah :)). OK for making suggestions, where should it be written? In the VM Integration Guide of course and in the NEWS file. And when we have a first implementation of all this checked in it might make sense to explicitly ask on the main classpath list to get the attention of those runtime implementors that don't follow the patches list that closely. Cheers, Mark ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
Re: [cp-patches] [generics] RFC : implementation of jjava.lang.instrument package
Mark Wielaard wrote: Yes, but it makes it easier to share code with ClassLoader and VMClassLoader if we do this. Then callTransformers doesn't have to be public, but can just be package private. Then we don't have to worry about whether or not someone can get insecure access to a IntrumentationImpl (although I admit that is not very likely). It just seems more natural to me to have all implementation classes in the same package because they seem very related. Allright then, VMInstrumentationImpl and InstrumentationImpl are in vm/reference/java/lang Last RFC, and I'm commiting this in 2005-12-02 Nicolas Geoffray [EMAIL PROTECTED] * java/lang/instrument: New directory. * java/lang/instrument/ClassDefinition.java: New file. * java/lang/instrument/ClassFileTransformer.java: New file. * java/lang/instrument/IllegalClassFormatException.java: New file. * java/lang/instrument/Instrumentation.java: New file. * java/lang/instrument/UnmodifiableClassException.java: New file. * vm/reference/java/lang/InstrumentationImpl.java: New file. * vm/reference/java/lang/VMInstrumentationImpl.java: New file. instrument.tar.gz Description: GNU Zip compressed data ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
Re: [cp-patches] [generics] RFC : implementation of jjava.lang.instrument package
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 ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
Re: [cp-patches] [generics] RFC : implementation of jjava.lang.instrument package
Hi Nicolas,
On Thu, 2005-12-01 at 17:51 +0100, Nicolas Geoffray wrote:
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.
I had in mind that the VM wouldn't need a cancel method. The
prepareRedefineClass gives a vm internal representation
of a class, and if redefinition is canceled this representation would
be garbage collected and forgotten by the vm. But maybe some vm
would like to be noticed. I added the method since it might be
usefull.
My idea was even that a VM might only want to have one redefine
prepare/apply cycle running at the same time. Then it can easily cleanup
anything not used. But this might be optimizing for an unusual case. We
can probably assume that transformers will actually transform correctly.
Why does the private callTransformers() implementation take a
ProtectionDomain? Isn't that
classBeingRedefined.getProtectionDomain()?
Yes, but the classBeingRedefined might be null, if the method is
called
by the vm from the native ClassLoader.defineClass method. Therefore,
the vm gives as arguments the protectionDomain it has.
Aha. Sorry I missed the dual nature of the
ClassFileTransFormer.transform() method. I see you documented this in
your original patch. In that case I really do think we should move these
to vm/reference/java/lang and make this method package private.
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?
It's possible, but it could break all existing vms lying on the vm
reference classes.
The VMClassLoader would need an extra defineClass method and an extra
field
for an InstrumentationImpl object. The extra defineClass method would
check if the
InstrumentationImpl is null and if not, call the
InstrumentationImpl.callTransformers
method.
Right, I see we need to break the VM interface for this :{
Probably best not to do that for the first release (0.20) but lets
suggest how we would like to change the interface for 0.21 so runtime
implementers can prepare and scream about it. On the other hand this is
the generics branch only, not very many VMs depend on it yet. But not
very urgent I guess before we have this working with at least one VM.
Yeah, documentation to write! :). I will create a documentation file
for the java.lang.instrument package,
and change the vm integration guide (should I create a new paragraph
for generics?)
Just make a new paragraph for Instrumentation. Then mention that it
currently only is on the generics branch since it uses some new language
features.
Cheers,
Mark
___
Classpath-patches mailing list
Classpath-patches@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath-patches
Re: [cp-patches] [generics] RFC : implementation of jjava.lang.instrument package
Hi Nicolas, On Sat, 2005-11-26 at 19:11 +0100, Nicolas Geoffray wrote: Yes, that name is OK. I don't know how much we can actually share between runtimes though. The interface is pretty minimal already, so a split between VMInstrumentationImpl and InstrumentationImpl that calls the VM class seems overkill. The InstrumentationImpl.getObjectSize is clearly a native for vm. And we also need a native redefineClass(Class, byte[]). If we don't provide a VMInstrumentationImpl class, would these method be native in InstrumentationImpl? (I thought vm implementors only had to care about vm/reference/* files?) Ideally yes. I was just thinking that if the interface between InstrumentationImpl and VMInstrumentationImpl is really thin and there is not much code that can be shared maybe we just have one class that the VM should implement. I haven't thought very deep about the actual implementation yet or how easy it is to write generic code that can be easily shared. Because it's the vm that parses arguments, it knowns if the application does instrumentation. Therefore, if it supports redefining classes, it can set a flag or change definition of VMClassLoader.defineClass. The vm then creates an InstrumentationImpl object and calls premain. The VMClassLoader.defineClass will be changed to call a private InstrumentationImpl.defineClass method. This method will then call each transformer. Does that sound correct to you? This way I don't modify ClassLoader.defineClass. Yes, this makes perfect sense. Thanks, Mark ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
