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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: BlockDispatch7.diff
Type: application/octet-stream
Size: 318183 bytes
Desc: BlockDispatch7.diff
URL:
<http://rubyforge.org/pipermail/ironruby-core/attachments/20080709/61659c78/attachment-0001.obj>
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: ironruby-core-bounces at rubyforge.org [mailto:ironruby-core-bounces at
rubyforge.org] On Behalf Of Tomas Matousek
Sent: Wednesday, July 09, 2008 7:09 PM
To: IronRuby External Code Reviewers
Cc: ironruby-core at rubyforge.org
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
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: ironruby-core-bounces at rubyforge.org [mailto:ironruby-core-bounces at
rubyforge.org] On Behalf Of Curt Hagenlocher
Sent: Thursday, July 10, 2008 1:10 PM
To: ironruby-core at rubyforge.org
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: ironruby-core-bounces at rubyforge.org [mailto:ironruby-core-bounces at
rubyforge.org] On Behalf Of Tomas Matousek
Sent: Wednesday, July 09, 2008 7:09 PM
To: IronRuby External Code Reviewers
Cc: ironruby-core at rubyforge.org
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
Ironruby-core at rubyforge.org
http://rubyforge.org/mailman/listinfo/ironruby-core
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: ironruby-core-bounces at rubyforge.org [mailto:ironruby-core-bounces at
rubyforge.org] On Behalf Of Tomas Matousek
Sent: Thursday, July 10, 2008 2:21 PM
To: ironruby-core at rubyforge.org
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: ironruby-core-bounces at rubyforge.org [mailto:ironruby-core-bounces at
rubyforge.org] On Behalf Of Curt Hagenlocher
Sent: Thursday, July 10, 2008 1:10 PM
To: ironruby-core at rubyforge.org
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: ironruby-core-bounces at rubyforge.org [mailto:ironruby-core-bounces at
rubyforge.org] On Behalf Of Tomas Matousek
Sent: Wednesday, July 09, 2008 7:09 PM
To: IronRuby External Code Reviewers
Cc: ironruby-core at rubyforge.org
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
Ironruby-core at rubyforge.org
http://rubyforge.org/mailman/listinfo/ironruby-core
_______________________________________________
Ironruby-core mailing list
Ironruby-core at rubyforge.org
http://rubyforge.org/mailman/listinfo/ironruby-core