You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Kohsuke Kawaguchi <ko...@sun.com> on 2005/12/31 17:34:33 UTC
Re: Javaflow and ASM
Kohsuke Kawaguchi wrote:
>>> So if you don't mind, I'm happy to make the necessary changes.
>>
>> Please go ahead. And if you don't mind please send me the test results
>> you'll get.
>
> Will do.
I added a new property to select the transformer, and tried to use ASM.
I found a few issues.
- ContinuationMethodAdapter.visitMaxs was passing (0,0) to the next
visitor. I'm not too familiar with this, but don't we need to basically
pass through values?
- When moving 'new' instructions, local variables are allocated
excessively. I think it would be better for it to be used little more
conservatively. Namely, the number of additional local variables for
save/restore should be max of all constructor parameter counts, not sum
of them.
- When moving 'new' instructions, The maxStacks field isn't updated
correctly, but I think we needed this.
- While instrumenting the FinallyFlow class, I found that
ContinuationMethodAdapter.visitMethodInsn dies while trying to save a
local variable that has a JSR return address in it. I don't think we can
instrument method invocations in a finally block, so there needs to be
code that checks it (I don't know how BCEL version is doing that, though)
That's it for now.
--
Kohsuke Kawaguchi
Sun Microsystems kohsuke.kawaguchi@sun.com
Re: Javaflow and ASM
Posted by Eugene Kuleshov <eu...@javatx.org>.
Kohsuke Kawaguchi wrote:
>> Actually your above example should be quite simple, especially if
>> we can assume that we know local variable types (e.g. debug
>> information is available):
>
> Well, sure, if you can use debug information, but I don't think we can
> assume that in javaflow, can we.
It depends. But in general you probably right. :-)
>> Though more complicated case would be the case when restoring stack
>> not directly used by the currently restoring method. Probably
>> something similar to what happening for NEW opcodes. And for this we
>> will have to always search for common super class or interface.
>
> OK, sounds like we are in agreement that we sometimes do need to compute
> the base class.
Right. BTW, SimpleVerifier is actually doing this already using
Class methods.
Though there is some tricky scenario that would be difficult to
handle. This is in case when two classes would have more then one
common interface (e.g. both implements Comparable and Externalizable)
and don't have common super class. Eric left a comment in
SimpleVerifier code about this scenario and I am not sure how this can
be resolved.
>>> It shouldn't be too difficult; BCEL does that. As I wrote in the
>>> commit message, all we need is a resolver that can get you the byte
>>> code image of those referenced classes (like ClassX and ClassY.)
>>
>> We have code for this within ASM test cases. It is in
>> MUSTANG_SUPPORT branch in org.objectweb.asm.ClassWriterTest3 class. If
>> you like I can try to put this code into SimpleVerifier subclass
>> already used in javaflow.
>
> That would be great.
Will do that and then depends from the results of your testing I
can look at implementing algorithm based on dataflow only.
regards,
Eugene
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: Javaflow and ASM
Posted by Kohsuke Kawaguchi <ko...@sun.com>.
Eugene Kuleshov wrote:
> Actually your above example should be quite simple, especially if
> we can assume that we know local variable types (e.g. debug
> information is available):
Well, sure, if you can use debug information, but I don't think we can
assume that in javaflow, can we.
> Though more complicated case would be the case when restoring stack
> not directly used by the currently restoring method. Probably
> something similar to what happening for NEW opcodes. And for this we
> will have to always search for common super class or interface.
OK, sounds like we are in agreement that we sometimes do need to compute
the base class.
>> It shouldn't be too difficult; BCEL does that. As I wrote in the commit
>> message, all we need is a resolver that can get you the byte code image
>> of those referenced classes (like ClassX and ClassY.)
>
> We have code for this within ASM test cases. It is in
> MUSTANG_SUPPORT branch in org.objectweb.asm.ClassWriterTest3 class. If
> you like I can try to put this code into SimpleVerifier subclass
> already used in javaflow.
That would be great.
--
Kohsuke Kawaguchi
Sun Microsystems kohsuke.kawaguchi@sun.com
Re: Javaflow and ASM
Posted by Eugene Kuleshov <eu...@javatx.org>.
Kohsuke Kawaguchi wrote:
>> Another possible approach could be to use results of
>> DataflowInterpeter to find an instructions that are consuming given
>> stack value and then cast to the type required by this instruction,
>> but I am not sure if there are some gaps in this approach, e.g. if
>> locals will cause issues...
>
> I don't think it's possible. The same variable maybe assigned to, say,
> java.lang.Comparable and java.io.Serializable. Consider the following code:
>
> SomeMiddleClass x;
> if(...)
> x = new ClassX();
> else
> x = new ClassY();
>
[1]
> someFunctionCall();
>
> if(...) {
[2]
> methodThatTakesSerializable(x);
> } else {
[3]
> methodThatTakesComparable(x);
> }
>
> Given that bytecode doesn't have the type for 'x', correctly restoring
> 'x' at someFunctionCall requires us to really find the common base type
> as described in the JVM spec.
Actually your above example should be quite simple, especially if
we can assume that we know local variable types (e.g. debug
information is available):
-- for [1] we have to cast restored variable x to SomeMiddleClass, for
-- for [2] we can cast method parameter (stack value) to Serializable
-- for [3] we can cast method parameter (stack value) to Comparable
Though more complicated case would be the case when restoring stack
not directly used by the currently restoring method. Probably
something similar to what happening for NEW opcodes. And for this we
will have to always search for common super class or interface.
In any case this approach should allow to eliminate second analysis
step and use only dataflow analyzer to move new opcodes and also to
find types of the stack and locals.
> It shouldn't be too difficult; BCEL does that. As I wrote in the commit
> message, all we need is a resolver that can get you the byte code image
> of those referenced classes (like ClassX and ClassY.)
We have code for this within ASM test cases. It is in
MUSTANG_SUPPORT branch in org.objectweb.asm.ClassWriterTest3 class. If
you like I can try to put this code into SimpleVerifier subclass
already used in javaflow.
regards,
Eugene
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: Javaflow and ASM
Posted by Kohsuke Kawaguchi <ko...@sun.com>.
Eugene Kuleshov wrote:
> Kohsuke,
>
> I've tried to run unit tests for asm transformer on code from SVN
> and noticed that they all failing. I believe the reason is that you've
> changed SimpleVerifier to BasicVerifier in the method analyzer and not
> all of the checkcasts created with Object type. So, I am afraid that
> we'll have to use SimpleVerifier to get the actual types of the stack
> and local values.
OK. I'll roll it back to SimpleVerifier.
.... I guess your patch also fixed this. Thank you.
> Another possible approach could be to use results of
> DataflowInterpeter to find an instructions that are consuming given
> stack value and then cast to the type required by this instruction,
> but I am not sure if there are some gaps in this approach, e.g. if
> locals will cause issues...
I don't think it's possible. The same variable maybe assigned to, say,
java.lang.Comparable and java.io.Serializable. Consider the following code:
SomeMiddleClass x;
if(...)
x = new ClassX();
else
x = new ClassY();
someFunctionCall();
if(...) {
methodThatTakesSerializable(x);
} else {
methodThatTakesComparable(x);
}
Given that bytecode doesn't have the type for 'x', correctly restoring
'x' at someFunctionCall requires us to really find the common base type
as described in the JVM spec.
It shouldn't be too difficult; BCEL does that. As I wrote in the commit
message, all we need is a resolver that can get you the byte code image
of those referenced classes (like ClassX and ClassY.)
--
Kohsuke Kawaguchi
Sun Microsystems kohsuke.kawaguchi@sun.com
Re: Javaflow and ASM
Posted by Eugene Kuleshov <eu...@javatx.org>.
Kohsuke,
I've tried to run unit tests for asm transformer on code from SVN
and noticed that they all failing. I believe the reason is that you've
changed SimpleVerifier to BasicVerifier in the method analyzer and not
all of the checkcasts created with Object type. So, I am afraid that
we'll have to use SimpleVerifier to get the actual types of the stack
and local values.
Another possible approach could be to use results of
DataflowInterpeter to find an instructions that are consuming given
stack value and then cast to the type required by this instruction,
but I am not sure if there are some gaps in this approach, e.g. if
locals will cause issues...
regards,
Eugene
PS: I'll send a patch for local vars allocation later today.
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: Javaflow and ASM
Posted by Eugene Kuleshov <eu...@javatx.org>.
Kohsuke Kawaguchi wrote:
>>> - When moving 'new' instructions, local variables are allocated
>>> excessively. I think it would be better for it to be used little more
>>> conservatively. Namely, the number of additional local variables for
>>> save/restore should be max of all constructor parameter counts, not
>>> sum of them.
>>
>> I spotted this too and tried to fix this but haven't completed yet.
>> It seems that instead of using MethodNode.maxLocals as a
>> stackRecorderVar its value should be memorized and vars for param
>> restoration should be always started from that value + 1
>
> Yes.
Can you carry on this or would prefer a patch from me?
>>> - When moving 'new' instructions, The maxStacks field isn't updated
>>> correctly, but I think we needed this.
>>
>> It will be recalculated automatically when ClassWriter is used with
>> computeMaxs
>
> Hmm. I got "insufficient stack size" error or something like that when I
> didn't do anything. I added maxStack += 10 for a try and that made it
> work, which made me suspect of this issue.
That could be a error from analyzer. If so, we may not get away
without fixing maxstack or will have to run a complete stack computation
algorithm.
>>> - While instrumenting the FinallyFlow class, I found that
>>> ContinuationMethodAdapter.visitMethodInsn dies while trying to save a
>>> local variable that has a JSR return address in it. I don't think we
>>> can instrument method invocations in a finally block, so there needs
>>> to be code that checks it (I don't know how BCEL version is doing
>>> that, though)
>>
>> Another option could be to inline these blocks. I believe that one
>> of the projects, which been using ASM did that already and we can just
>> reuse it. I actually was thinking to package it with the asm-commons
>> or something.
>
> That sounds like a good idea.
That is why I've added Eric to this discussion. So, we can find an
acceptable way to integrate this feature.
> With such code where the same set of byte
> code is copied into multiple places, would it still be possible for a
> user to set a breakpoint and etc correctly?
As of breakpoints, they are working based on line number information
and because code is basically identical we can just copy this info to
both places. BTW Java6 and CLDC preverifier both doing this for finally
blocks bytecode.
It is actually interesting. The same line number can be referenced
from several places in the bytecode and finally blocks are not the only
case for this. If I recall correctly the same also happens for the loop
constructs as well as for constructor and static initializers.
regards,
Eugene
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: javaflow: SimpleVerifier's getClass() method
Posted by Kohsuke Kawaguchi <ko...@sun.com>.
Eugene Kuleshov wrote:
> I mentioned before analyzer is used here to compute stack and
> locals slots for method instructions in order to be able to
> restore/cast to correct types when restoring continuation.
Right.
> Speaking about Ant, can you tell me how can I test this issue?
I guess I should write a test case. I think it's just a matter of
writing a few classes that refer to each other, and then try to
instrument them through the Ant task.
When instrumenting from Ant task, you don't want to load those classes
that you are instrumenting into the Ant process. So any type computation
based on java.lang.Class won't work (it's not just a matter of ClassLoader.)
> Or
> maybe you can subclass SimpleVerifier yourself and overwrite following
> method:
>
> protected Class getClass(final Type t) {
> try {
> if (t.getSort() == Type.ARRAY) {
> return getClass()
> .getClassLoader()
> .loadClass(t.getDescriptor().replace('/', '.'));
> }
> return getClass()
> .getClassLoader()
> .loadClass(t.getClassName());
> } catch (ClassNotFoundException e) {
> throw new RuntimeException(e.toString());
> }
> }
>
> Eric, I actually wonder if we can use
> getClass().getClassLoader().loadClass(...) (or
> Thread.currentThread().contextClassLoader().loadClass(...)) instead of
> Class.forName(...) method? Do you recall if there was some reasons to
> use Class.forName?
--
Kohsuke Kawaguchi
Sun Microsystems kohsuke.kawaguchi@sun.com
javaflow: SimpleVerifier's getClass() method
Posted by Eugene Kuleshov <eu...@javatx.org>.
Kohsuke,
I've noticed your comment in svn for ContinuationMethodAnalyzer class:
--
SimpleVerifier assumes that the types involved in the computation is
loadable through Class.forName(), which isn't true when we are running
inside Ant.
The correct computation would require us to parse referenced types
(through a pluggable resolver.) For now, I'm just replacing it with
BasicVerifier.
Given that the purpose of this part of the code is just to check that
we generated the right code, perhaps we can remove it altogether in
the production system?
i.e.,
if(DEBUG) {
... run a verifier
}
--
I mentioned before analyzer is used here to compute stack and
locals slots for method instructions in order to be able to
restore/cast to correct types when restoring continuation.
Speaking about Ant, can you tell me how can I test this issue? Or
maybe you can subclass SimpleVerifier yourself and overwrite following
method:
protected Class getClass(final Type t) {
try {
if (t.getSort() == Type.ARRAY) {
return getClass()
.getClassLoader()
.loadClass(t.getDescriptor().replace('/', '.'));
}
return getClass()
.getClassLoader()
.loadClass(t.getClassName());
} catch (ClassNotFoundException e) {
throw new RuntimeException(e.toString());
}
}
Eric, I actually wonder if we can use
getClass().getClassLoader().loadClass(...) (or
Thread.currentThread().contextClassLoader().loadClass(...)) instead of
Class.forName(...) method? Do you recall if there was some reasons to
use Class.forName?
regards,
Eugene
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: Javaflow and ASM
Posted by Kohsuke Kawaguchi <ko...@sun.com>.
By the way I thought you might find this amusing:
"Javaflow - the ultimate fresh-brewed coffee dispenser!"
http://www.javaflowllc.com/
--
Kohsuke Kawaguchi
Sun Microsystems kohsuke.kawaguchi@sun.com
Re: Javaflow and ASM
Posted by Kohsuke Kawaguchi <ko...@sun.com>.
Eugene Kuleshov wrote:
>> - ContinuationMethodAdapter.visitMaxs was passing (0,0) to the next
>> visitor. I'm not too familiar with this, but don't we need to basically
>> pass through values?
>
> It is actually ok. ClassWriter is used with computeMaxs flag, so
> visitMaxs() method is basically an indicator and its params are ignored.
> It might be possible to caclulate stack and locals but I decided not
> spend time on this for now.
I see. I'll roll back the changes then.
>> - When moving 'new' instructions, local variables are allocated
>> excessively. I think it would be better for it to be used little more
>> conservatively. Namely, the number of additional local variables for
>> save/restore should be max of all constructor parameter counts, not sum
>> of them.
>
> I spotted this too and tried to fix this but haven't completed yet.
> It seems that instead of using MethodNode.maxLocals as a
> stackRecorderVar its value should be memorized and vars for param
> restoration should be always started from that value + 1
Yes.
>> - When moving 'new' instructions, The maxStacks field isn't updated
>> correctly, but I think we needed this.
>
> It will be recalculated automatically when ClassWriter is used with
> computeMaxs
Hmm. I got "insufficient stack size" error or something like that when I
didn't do anything. I added maxStack += 10 for a try and that made it
work, which made me suspect of this issue.
>> - While instrumenting the FinallyFlow class, I found that
>> ContinuationMethodAdapter.visitMethodInsn dies while trying to save a
>> local variable that has a JSR return address in it. I don't think we can
>> instrument method invocations in a finally block, so there needs to be
>> code that checks it (I don't know how BCEL version is doing that, though)
>
> Another option could be to inline these blocks. I believe that one of
> the projects, which been using ASM did that already and we can just
> reuse it. I actually was thinking to package it with the asm-commons or
> something.
That sounds like a good idea. With such code where the same set of byte
code is copied into multiple places, would it still be possible for a
user to set a breakpoint and etc correctly?
--
Kohsuke Kawaguchi
Sun Microsystems kohsuke.kawaguchi@sun.com
Re: Javaflow and ASM
Posted by Eugene Kuleshov <eu...@javatx.org>.
Kohsuke Kawaguchi wrote:
>>>> So if you don't mind, I'm happy to make the necessary changes.
>>> Please go ahead. And if you don't mind please send me the test
>>>>> results you'll get.
>> Will do.
>
> I added a new property to select the transformer, and tried to use ASM.
>
> I found a few issues.
>
> - ContinuationMethodAdapter.visitMaxs was passing (0,0) to the next
> visitor. I'm not too familiar with this, but don't we need to basically
> pass through values?
It is actually ok. ClassWriter is used with computeMaxs flag, so
visitMaxs() method is basically an indicator and its params are ignored.
It might be possible to caclulate stack and locals but I decided not
spend time on this for now.
> - When moving 'new' instructions, local variables are allocated
> excessively. I think it would be better for it to be used little more
> conservatively. Namely, the number of additional local variables for
> save/restore should be max of all constructor parameter counts, not sum
> of them.
I spotted this too and tried to fix this but haven't completed yet.
It seems that instead of using MethodNode.maxLocals as a
stackRecorderVar its value should be memorized and vars for param
restoration should be always started from that value + 1
> - When moving 'new' instructions, The maxStacks field isn't updated
> correctly, but I think we needed this.
It will be recalculated automatically when ClassWriter is used with
computeMaxs
> - While instrumenting the FinallyFlow class, I found that
> ContinuationMethodAdapter.visitMethodInsn dies while trying to save a
> local variable that has a JSR return address in it. I don't think we can
> instrument method invocations in a finally block, so there needs to be
> code that checks it (I don't know how BCEL version is doing that, though)
Another option could be to inline these blocks. I believe that one of
the projects, which been using ASM did that already and we can just
reuse it. I actually was thinking to package it with the asm-commons or
something.
regards,
Eugene
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org