commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Kohsuke Kawaguchi ...@kohsuke.org>
Subject Re: [javaflow] MethodLookup
Date Fri, 29 Jul 2005 23:45:30 GMT
(Please check the other e-mail I sent, which is hopefully much less 
controversial.)

Torsten Curdt wrote:
>>> See the example in my blog ...which is actually
>>> taken from the Cocoon integration. (I think in
>>> there is also the link to the class) Would be
>>> great if you could also have a look into that
>>> class.
>>>
>>
>> I looked at Cocoon code you had in [1]. I didn't particularly see  
>> anything that made me rethink (IOW, I don't see anything that  
>> absolutely requires MethodLookup nor ContinuationContext instead of  
>> Runnable.)
> 
> Had a look into your implementation
> ..you have the "run" method delegating to
> the different methods.

The Invoker class was needed to work around the problem/feature in the 
current Continuation API that the first object to be invoked needs to 
have a default constructor. The Fiber class can be extended by users and 
therefore it may not have the default constructor, so I can't just call 
Fiber.run immediately from Continuation.execute().

Fiber.run delegates to another Runnable because that's what 
java.lang.Thread does.


> If you want to get rid of the of the method
> lookup you do need to pass in the Runnable
> also to the continueWith ...not sure if I
> like that.

No, I don't think you have to. That Runnable object is a part of 
Continuation already. The continueWith method can look it up from there. 
Specifically, it already retains a reference to that object in Stack. We 
just need to make it explicitly accessible (instead of fiddling with 
references in stacks to get to it.)


> I had similar idea in the beginning:
> 
>   startWith(Method, ContinuationContext)
>   continueWith(Method, Continuation, ContinuationContext)
> 
> What I did not like: Passing in of the
> Continuation already implies the Method.

I agree. That's why I originally suggested to use Runnable in this 
level, to get rid of Method altogether.

> Beforehand the method was saved inside
> the Continuation object. But this does
> not work when it comes down to serialization.

By calling Runnable.run() method, you no longer have to save the Method 
object in Continuation. That means no MethodLookup is necessary, no 
method name is necessary.

> BTW: IIRC I needed to mark the static as
> transient for the XStream serialization
> ..but not totally sure.

I used XStream in other projects, and I believe it didn't persist static 
fields. But I will put that back. Maybe we are using different versions.

>> But at the same time, I saw that you have some existing investments  
>> with the current javaflow Continuation API and it's very  
>> understandable that you don't want it to change.
> 
> Well ...it's still marked alpha so as long
> as it makes sense to me I am fine changing
> it.
> 
> But TBH I currently would like to focus on
> the current bytecode rewriting stuff first
> before changing the API that much.

OK.



>> See the attachments for the exact code. All in all, I didn't think  
>> this re-write is ugly, but that is always a subjective issue.
> 
> I don't understand this forking and cloning
> stuff - you don't need that. A continuation
> should be re-entrant and always create a new
> one as a result.

Unlike Continuation, Fiber is a mutable object. It updates itself as you 
continue execution. That's why sometimes you want a "fork" to create an 
identical copy.

Fiber.fork doesn't deep-copy Continuation, as Continuation itself 
doesn't need to be copied. It's just Fiber object that needs to be 
copied, hence the use of clone().



>> Anyway, I'm no longer too keen about convincing you to change the  
>> API of the Continuation class. If you like my rewrite, that's good,  
>> but if not, that's also fine with me. The current Continuation API  
>> makes the Fiber class implementation uglier, but that's not too big  
>> a problem.
>>
>> Instead, I'm hoping that you allow me to commit this Fiber class to  
>> javaflow, as this is what I primarily want to use in my code.
> 
> If you don't mind I would prefer if you keep
> that code in your codebase for now. Let's get
> back to that once we have the rewriting based
> on ASM in place. I would like to focus on the
> hard part first ;)
 >
 > Would that be ok for you?

Hmm...

Even if Fiber doesn't change any of the existing Continuation class, do 
you still feel uncomfortable with it?

As an user, it doesn't really matter to me what byte-code manipulation 
library javaflow is using, because it's completely behind the scene. The 
current BCEL one seems to be working, so it's not very high priority for 
me to rewrite javaflow by using ASM. Besides, I doubt if I can be of any 
help when it comes to porting javaflow from BCEL to ASM. That part is 
the guts of javaflow and it takes you to change it.

For me, the javaflow API surface is of a higher priority, because in the 
end my goal is to use it. I suspect that a thread-like API connects to 
users more easily, as there's less learning involved. I'm hoping that 
this contributes to an improved user experience. For this reason, I hope 
it's beneficial to the javaflow project.

Once again, I'm not trying to convince you to change the existing 
Continuation class in any substantial way. I'm only asking you to put 
this Fiber class side-by-side with the current Continuation class.

Please please can I have this class? I promise I won't talk about the 
Continuation class signature change any more :-)

If not, I guess I just need to accumulate more small improvements to 
make you feel comfortable with me. It's disappointing for me for now, 
but that's certainly fair, I guess.

-- 
Kohsuke Kawaguchi

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


Mime
View raw message