In principle I agree with you, but killing the process on an unhandled thread
exception is an incompatibility with MRI:
irb(main):001:0> Thread.start { raise 'foo' }
=> #<Thread:0x38abfe4 dead>
irb(main):002:0>
-----Original Message-----
From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Tomas Matousek
Sent: Thursday, July 10, 2008 2:21 PM
To: [email protected]
Subject: Re: [Ironruby-core] Code Review: BlockDispatch7
StepString, StepNumeric, StepFixnum check for the block in the loop. Note that
the exception is not thrown if no item is visited by the enumerator. But you're
right there are methods that handle the block incorrectly. We need to write
some tests that pass nil to all library methods that use blocks to cover all
cases.
The assignment "result = item" in Enumerable.Find isn't in fact redundant. It
assigns to the closed-over variable "result".
"return RuntimeFlowControl.BlockBreak(selfBlock, item);" doesn't return from
the Find method, it returns from the lambda defined within the method :)
Yes, I'll revisit the Call methods on the next pass.
I've added rethrow to the Thread.CreateThread since it is imo better to kill
the process rather than silently swallow the exception (I hit this when
something wrong happened in the thread and I didn't know what because the
exception has been swallowed).
Tomas
-----Original Message-----
From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Curt Hagenlocher
Sent: Thursday, July 10, 2008 1:10 PM
To: [email protected]
Subject: Re: [Ironruby-core] Code Review: BlockDispatch7
There doesn't generally appear to be any consistency about comparing the block
parameter to null. For instance, in RangeOps, only StepFixnum checks;
StepString, StepNumeric and StepObject do not. I know that the situation
predates the current changeset, but it might be a good time to go through and
fix it.
The test for a non-empty range in RangeOps.StepFixnum does not correctly handle
ExcludeEnd.
In method Enumerable.Find, there's now an extraneous assignment of "item" to
"result".
In Proc.cs, there are a bunch of newly-added Call methods inside an #if UNUSED.
Are these for future use?
The rethrow inside ThreadOps.CreateThread will take down the process. Is that
what's desired?
I wouldn't swear to understanding all of the changes to the compiler, but
things otherwise look good to me. I particularly like the new pattern for
Block.Yield; the "out" parameter will make it much harder to forget to check
for a jump.
-----Original Message-----
From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Tomas Matousek
Sent: Wednesday, July 09, 2008 7:09 PM
To: IronRuby External Code Reviewers
Cc: [email protected]
Subject: [Ironruby-core] Code Review: BlockDispatch7
tfpt review "/shelveset:BlockDispatch7;REDMOND\tomat"
Improves dispatch to blocks.
Previously, DynamicSites were used to adapt call site's arguments to the
signature of the target block. In block invocation no resolution needs to be
performed, which makes it different from method invocation. The dynamic
behavior is only in the arguments to parameters transformation. In usual case
it is straightforward though. Only if splatting/unsplatting is used (and in
some other special cases) there are various checks that need to be performed to
shuffle the arguments right. Although dynamic sites could help here in some
cases (by caching by a shape of the target block signatures) the usual cases
are rather slowed down by the overhead. In an optimal case w/o any
splatting/unsplatting and without polymorphic sites kicking in at least one
comparison and 2 delegate calls needs to be done.
This change replaces dynamic site dispatch by a virtual dispatch optimized for
0-4 parameters. Each block is associated with a block dispatcher (a subclass of
BlockDispatcher abstract class, previously RubyBlockInfo) that corresponds to
its signature. The specialized dispatchers implement virtual Invoke methods for
0...4 and N parameters w/ and w/o splatting. The call site uses one of those
Invoke methods (based on its arguments) and calls it. The dispatcher holds on a
delegate that points to the block method. The delegate is called by Invoke
methods with transformed arguments. In the optimal case (e.g. 1.times {|x| puts
x}) the cost of block yield is a virtual method dispatch and a delegate call.
Besides no runtime-generated stubs are needed which improves startup time.
Using the dispatchers also enables to move some previously generated code into
RubyOps and therefore decreases the amount of generated code even more.
Blocks are still IDOs to provide good interop. This change also made the rules
much simpler.
TODO:
There is some work to be done to optimize some paths thru dispatchers. Will
need to run some micro-benchmarks for block dispatch to see where we should do
better.
Also, some parts of the code seem to be good candidates for source code
generation, but I haven't opted for that for now since it was easier to write
it by hand (there are many exemptions to the "rules" of the block dispatch, so
even if the code looks like it could be generated at the first glance the
generator would actually get more complicated to handle all such cases). I've
let this in TODO bucket.
Tomas
_______________________________________________
Ironruby-core mailing list
[email protected]
http://rubyforge.org/mailman/listinfo/ironruby-core
_______________________________________________
Ironruby-core mailing list
[email protected]
http://rubyforge.org/mailman/listinfo/ironruby-core
_______________________________________________
Ironruby-core mailing list
[email protected]
http://rubyforge.org/mailman/listinfo/ironruby-core
