Forum: IronRuby Code Review: AdaptiveRules4

tfpt review "/shelveset:AdaptiveRules4;REDMOND\tomat"


DLR: Fixes bugs and implements new features in interpreter.

-          Static field assignment.

-          LessThan

-          ConvertUnary - should re-box numeric values.

-          Force compilation whenever ref/out parameters are 
encountered.

-          Invocation expression with delegate target.

-          Unbox - no-op.

-          TypeEqual and TypeIs for sealed types.

Ruby:

-          Implements adaptively compiled rules:

-          RubyMetaBinder (subclass of all Ruby binders) implements 
BindDelegate so that:

o   It creates AST for the rule calling Bind and wraps the result into a 
lambda exactly like call site binder does.

o   If this lambda is interpretable (doesn't contain loops or other 
constructs that force compilation) it creates an InterpretedDispatcher 
that  hooks the lambda's "Compiled" event so that it gets called back as 
soon as compiled delegate is available. Until that happens it dispatches 
rule invocations to the interpreter.

o   If the lambda is forced-compiled it uses the resulting delegate 
right away.

o   The dispatcher is a generic class generated for Func and Action 
delegates with 0..15 generic parameters.


-          Also Adds specs for "include?" used on ClrNames.



Tomas

Inline.

From: Dino Viehland
Sent: Thursday, June 18, 2009 12:55 PM
To: Tomas Matousek; IronRuby External Code Reviewers; Rowan Code 
Reviewers
Cc: ironruby-core@rubyforge.org
Subject: RE: Code Review: AdaptiveRules4

On the rethrow in NewInstruction you should do 
ExceptionHelpers.UpdateForRethrow (otherwise the stack trace for the 
inner exception will be lost).

OK.

I would suggest that we should generate the numeric conversions into 
specialized subclasses so we don't need to do the switch at runtime.

I think switch is not as expensive here given that we are 
boxing/unboxing already. The number of classes would be huge (there are 
11 x 4 x 2 = 88 cases). So I would prefer to keep the switch.

On CompileSetItemArray is it the case that this is really not 
implemented?  Can you actually write something in the TODO: and remove 
the code which presumably doesn't work?

Yes it is not implemented. I'll do that.

Can you throw NotImplemented in CompileConvertToType instead of just 
returning when we don't support the conversion?

TODO here doesn't mean that the conversion is not supported. For example 
conversions to a super-class or implemented interfaces are no-op. A 
conversion to a non-implemented interface or an unrelated class, etc. 
should fail. So this needs to be fixed, but I can't just throw an 
exception since in most cases it is ok. Will add better comment.

What was wrong w/ StaticFieldAccessInstruction?  Why do we want to have 
to push and pop an extra value when accessing a static field?

I though static field access is not so frequent operation to be 
optimized, so we can save an instruction class. I can roll it back if 
you think we should optimize that case.

Otherwise the outer layer changes look good.

From: Tomas Matousek
Sent: Thursday, June 18, 2009 11:33 AM
To: IronRuby External Code Reviewers; Rowan Code Reviewers
Cc: ironruby-core@rubyforge.org
Subject: Code Review: AdaptiveRules4


tfpt review "/shelveset:AdaptiveRules4;REDMOND\tomat"


DLR: Fixes bugs and implements new features in interpreter.

-          Static field assignment.

-          LessThan

-          ConvertUnary - should re-box numeric values.

-          Force compilation whenever ref/out parameters are 
encountered.

-          Invocation expression with delegate target.

-          Unbox - no-op.

-          TypeEqual and TypeIs for sealed types.

Ruby:

-          Implements adaptively compiled rules:

-          RubyMetaBinder (subclass of all Ruby binders) implements 
BindDelegate so that:

o   It creates AST for the rule calling Bind and wraps the result into a 
lambda exactly like call site binder does.

o   If this lambda is interpretable (doesn't contain loops or other 
constructs that force compilation) it creates an InterpretedDispatcher 
that  hooks the lambda's "Compiled" event so that it gets called back as 
soon as compiled delegate is available. Until that happens it dispatches 
rule invocations to the interpreter.

o   If the lambda is forced-compiled it uses the resulting delegate 
right away.

o   The dispatcher is a generic class generated for Func and Action 
delegates with 0..15 generic parameters.


-          Also Adds specs for "include?" used on ClrNames.



Tomas