(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: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
