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 <kk...@kohsuke.org> on 2005/08/14 02:50:39 UTC

[javaflow] instrumenting 'NEW' op

I've been playing with javaflow and noticed the following.

Suppose I have the following code:

	File root = new File("data");

This compiles into the following bytecode:

     // local variable 0 is "this"
     // local variable 1 is "root"

     0  new java/io/File
     3  dup
     4  ldc "data"
     6  invokespecial java/io/File/<init>(Ljava/lang/String;)V
     9  astore_1

After the instrumentation, this code expands to:

     118  ldc "data"
     120  aload 2
     122  swap
     123  invokevirtual o/a/c/j/b/StackRecorder/pushObject...
     126  new java/io/File
     129  dup
     130  aload 2
     132  invokevirtual o/a/c/j/b/StackRecorder/popObject...
     135  checkcast java/lang/String
     138  invokespecial java/io/File/<init>(Ljava/lang/String;)V
     141  astore_1

I don't think I understand the instrumentation logic completely, but 
from a cursory look, the idea is to evaluate constructor parameters 
before the 'new' op (and StackRecorder is used as a temporary place to 
store evaluated objects.)

This is actually causing an NPE if I run the above code outside the 
continuation environment (because StackRecorder is null.)

So here goes my questions:

Why is this necessary? I'd imagine it's related to restoring/capturing 
stack frames when Continuation.suspend is invoked inside a constructor, 
but it's not clear to me why we need to handle 'new's/'invokespecial's 
differently from, say, 'invokevirtual'.

Also, I noticed that the BcelClassTransformer isn't actually generating 
the stack capture/restore code for invokespecial. Is that a TODO? Is 
this related to the following line in the TODO file?

>   o fix unintialized objects for method/constructor calls



-- 
Kohsuke Kawaguchi

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [javaflow] instrumenting 'NEW' op

Posted by Kohsuke Kawaguchi <kk...@kohsuke.org>.
Kohsuke Kawaguchi wrote:
> Torsten Curdt wrote:
>>>
>>> I don't think I understand the instrumentation logic completely,  
>>> but from a cursory look, the idea is to evaluate constructor  
>>> parameters before the 'new' op (and StackRecorder is used as a  
>>> temporary place to store evaluated objects.)
>> 
>> That's true ...Stephan introduced that
>> to get rid of uninitialized objects on
>> the stack.
>> 
>> TBH I would like to find a different
>> way of doing that. Maybe we can save
>> the type of the object on the stack
>> and then create a new uninitialized
>> object on the restore.
> 
> I read the JVM spec and I understand the interaction between the 
> verifier and the uninitialized objects better. So I think I now 
> understand the reasoning behind this. When you got a Java code like this:
> 
> String v = new String(computeString());
> 
> String computeString() {
>    Continuation.suspend();
>    return "abc";
> }
> 
> then the stack capturing gets problematic because the stack contains 
> uninitialized object. The transformer was trying to avoid this issue by 
> performing evaluations of parameters before the 'new' instruction.
> 
> Like you said, I think it's nicer to change this so that the capturing 
> would simply discard the uninitialized object and the restoration 
> creates a new instance. We can infer the type of the object that needs 
> to be 'new'ed from the byte code, so that information doesn't have to be 
> saved in the StackRecorder.
> 
> I'm interested in fixing this, if that's OK with you.

I think I realized the problem with this approach. The code below 
wouldn't pass the verifier even though it's safe.


    ldc 0
    ifeq x

    new java/lang/String
    goto y

x:
    new java/lang/String

y:
    invokespecial java/lang/String/<init>()V

When the stack operand type is compared for two incoming control flows 
to "y:", those two uninitialized String types are considered different.

I also checked the BRAKES project and they do the transformation similar 
to the current transformation of javaflow (evaluate arguments before 
'new') The only difference is that they use local variables to store 
evaluated results.

I have to say ... the bytecode stuff is dense!

-- 
Kohsuke Kawaguchi

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [javaflow] instrumenting 'NEW' op

Posted by Kohsuke Kawaguchi <ko...@sun.com>.
Torsten Curdt wrote:
>>
>> I don't think I understand the instrumentation logic completely,  
>> but from a cursory look, the idea is to evaluate constructor  
>> parameters before the 'new' op (and StackRecorder is used as a  
>> temporary place to store evaluated objects.)
> 
> That's true ...Stephan introduced that
> to get rid of uninitialized objects on
> the stack.
> 
> TBH I would like to find a different
> way of doing that. Maybe we can save
> the type of the object on the stack
> and then create a new uninitialized
> object on the restore.

I read the JVM spec and I understand the interaction between the 
verifier and the uninitialized objects better. So I think I now 
understand the reasoning behind this. When you got a Java code like this:

String v = new String(computeString());

String computeString() {
   Continuation.suspend();
   return "abc";
}

then the stack capturing gets problematic because the stack contains 
uninitialized object. The transformer was trying to avoid this issue by 
performing evaluations of parameters before the 'new' instruction.

Like you said, I think it's nicer to change this so that the capturing 
would simply discard the uninitialized object and the restoration 
creates a new instance. We can infer the type of the object that needs 
to be 'new'ed from the byte code, so that information doesn't have to be 
saved in the StackRecorder.

I'm interested in fixing this, if that's OK with you.


I also learned that the byte code verifier ensures that the constructor 
can be only invoked on an uninitialized object. I guess this means that 
we can't suspend if the call stack includes a constructor. This seems 
like a rather serious limitation.

Did this limitation apply to the BRAKES project? Or did they find a way 
to work around this? like maybe by copying constructor into an ordinary 
method?


-- 
Kohsuke Kawaguchi
Sun Microsystems                   kohsuke.kawaguchi@sun.com

Re: [javaflow] instrumenting 'NEW' op

Posted by Torsten Curdt <tc...@apache.org>.
>
> I don't think I understand the instrumentation logic completely,  
> but from a cursory look, the idea is to evaluate constructor  
> parameters before the 'new' op (and StackRecorder is used as a  
> temporary place to store evaluated objects.)

That's true ...Stephan introduced that
to get rid of uninitialized objects on
the stack.

TBH I would like to find a different
way of doing that. Maybe we can save
the type of the object on the stack
and then create a new uninitialized
object on the restore.

>
> This is actually causing an NPE if I run the above code outside the  
> continuation environment (because StackRecorder is null.)

Well ...usually you should not run
this code outside the continuation
environment. Maybe we should try to
force that. So far that was the most
common problem people had getting
started with javaflow.

> Why is this necessary? I'd imagine it's related to restoring/ 
> capturing stack frames when Continuation.suspend is invoked inside  
> a constructor,

Yepp :)

> but it's not clear to me why we need to handle  
> 'new's/'invokespecial's differently from, say, 'invokevirtual'.

Hmmmm... not sure if we need to.
Usually I would say we don't need
to treat it differently, too.

> Also, I noticed that the BcelClassTransformer isn't actually  
> generating the stack capture/restore code for invokespecial. Is  
> that a TODO? Is this related to the following line in the TODO file?
>
>
>>   o fix unintialized objects for method/constructor calls

Yes, that's about that the "new" operator.

cheers
--
Torsten