[ruby-core:33566] Re: [Ruby 1.9-Feature#4085][Open] Refinements and nested methods

Hello,

On Thu, Dec 2, 2010 at 11:05 PM, Shugo Maeda <[e-mail]> wrote:
> 2010/11/30 Charles Oliver Nutter <[e-mail]>:
>> * "using" not being a keyword requires all calls to check for
>> refinements all the time, globally degrading performance.
>
> This means that you should check a flag or something in StaticScope
> for every method invocation, and you cannot accept the overhead,
> right?

Every little bit matters. In experimental optimizations, JRuby is able
to reduce a dynamic call to two memory indirections + compare + static
dispatch directly to jitted code. When the dispatch path is this fast,
adding multiple additional layers of memory indirection and comparison
to support rarely-changing refinements can definitely show up. I also
have not attempted to implement an optimized version of refinement
dispatch, and I worry that there will be additional
performance-impacting issues.

> What do you think of refine?  Should it also be a keyword?
> How about refinements activation in reopened definitions and
> refinement inheritance?
> Can all they be problems?

refine as keyword: I don't think so, since it, like many other
meta-programming methods, applies its changes only once to the
class/module that surrounds it and the block it has been given. It's
very "magical", but I'm not sure that's enough of a reason to make it
a keyword.

refinement activation in reopened definition: Reopened definitions are
hierarchies of new lexical scopes. If a refinement is active for those
scopes it will only affect them and no other scopes. Obviously you
should not be able to apply a new refinement to scopes that have
already been closed without refinements in place. That is my #1
concern with this proposal...what is static (as in StaticScope in
JRuby) should remain static. I don't believe "refine" or refinements
applied in reopened classes violate that rule.

>> * there are very likely many more complexities than illustrated here
>> that result from the combination of runtime-mutable lexical scoping
>> structures, concurrency, and method caching.
>
> As Yusuke showed in [ruby-core:33535], the current implementation has
> this problem.
> The current implementation checks only the (singleton) class of the
> receiver and the VM version.  It should also check the refinements in
> cref to avoid this problem,
> but it causes more overhead.

More overhead is always bad if it affects performance globally :)

> I might withdraw the proposal of refinement propagation for blocks
> given to instance_eval,
> but what do you think of instance_eval without blocks?
>
>  x.instance_eval("...")

This form is "safe" since the string needs to be parsed and evaluated
(and provided with a new scope) each time. I worry a bit about the
inconsistency of having the "" form propagate refinements but the {}
form not propagating refinements. Perhaps this is a big like the
duality of constant lookup inside instance_eval?

> And, how about to introduce a new method (e.g.,
> Module#refinement_eval) which copies a given block and make the copy
> refinement aware?
> I think blocks are useful to implement DSLs like RSpec.
>
>  it "should ..." do
>     foo = Foo.new
>     foo.should == ...  # Object#should is available only in the block.
>  end

Yes, I agree this is a useful and difficult case to address. One
possible solution would be making the modification of the block
explicit and permanent:

p = proc { bar }
p.refine! BarModifyingExt
# p from now on has BarModifyingExt's refinements applied to it

This still can suffer from concurrency problems, if the "p" proc is
stored somewhere and in use when Proc#refine is called, but I think it
addresses the problem of having blocks flip back and forth between
refined and unrefined. It's not perfect wrt concurrency, but it's
(somewhat) better.

I'm not sure this addresses the problems with performance, though. All
method calls in all blocks everywhere would still need to check if
they've been turned into a Proc and "refined", and that would be a
larger impact than simply checking a global flag.

The bottom line is that anything which makes static parse output now
mutable will always be a concurrency problem, and probably always be a
performance hit. All Ruby implementations currently depend on certain
immutable truths about parser output, and this proposal violates many
of those truths.

To be honest, I think it should be a using directive at the file's
toplevel, so subsequently-parsed blocks know they're going to be
refined. I believe more and more that "using" needs to be a keyword;
having yet another "magic" method that can alter parse/execution
behavior beyond its scope scares me.

require 'rspec'

# RSpec::Refinements would define all methods currently
# defined against Object or Kernel, and the refine here
# would simply apply them to this file.
# TODO: using affects current scope or only child scopes?
# The latter would be best, since the current scope is already
# "active" and should not be modified.
using RSpec::Refinements
# does this scope have refinements active?

# describe is from refinement(?) or a real method
describe 'blah' do
  # this scope has refinements active

  # "it" is either from refinement or a real method
  it 'blah' do
    # this scope has refinements active

    # "should" is from refinement
    x.should == y
  end
end

- Charlie