I've been working on a chapter for "Ruby Best Practices"[0] about the
dynamic nature of Ruby, and I initially thought Jim Weirich's
BlankSlate would be an excellent example (even though 1.9 has
BasicObject).
However, when I look at the reveal class method, I found what seems to
be a fairly major limitation that I am having a hard time glossing
over in my prose.
The issue is that when you reveal a previously hidden method, the
UnboundMethod that was stored gets bound to whatever the first
instance is that calls the revealed method, because Jim's code caches
the bound_method via a closure. The best 'fix' I got was to drop the
caching and rebind the method to self on every single method call, but
I wasn't able to get away with that without saying "This is probably a
performance nightmare". Does anyone have a better suggested fix? The
original code for reveal, pulled from the builder gem, is below.
### Code pulled from blankslate.rb in the builder gem.
# Redefine a previously hidden method so that it may be called on a blank
# slate object.
def reveal(name)
bound_method = nil
unbound_method = find_hidden_method(name)
fail "Don't know how to reveal method '#{name}'" unless unbound_method
define_method(name) do |*args|
bound_method ||= unbound_method.bind(self)
bound_method.call(*args)
end
end
end
I've been working on a chapter for "Ruby Best Practices"[0] about the
dynamic nature of Ruby, and I initially thought Jim Weirich's
BlankSlate would be an excellent example (even though 1.9 has
BasicObject).
However, when I look at the reveal class method, I found what seems to
be a fairly major limitation that I am having a hard time glossing
over in my prose.
The issue is that when you reveal a previously hidden method, the
UnboundMethod that was stored gets bound to whatever the first
instance is that calls the revealed method, because Jim's code caches
the bound_method via a closure. The best 'fix' I got was to drop the
caching and rebind the method to self on every single method call, but
I wasn't able to get away with that without saying "This is probably a
performance nightmare". Does anyone have a better suggested fix? The
original code for reveal, pulled from the builder gem, is below.
From what you write caching a bound method seems a bad idea because it
is instance specific. Caching of an unbound method would seem much
more reasonable (unless, that is, I am missing something).
### Code pulled from blankslate.rb in the builder gem.
# Redefine a previously hidden method so that it may be called on a blank
# slate object.
def reveal(name)
bound_method = nil
unbound_method = find_hidden_method(name)
fail "Don't know how to reveal method '#{name}'" unless unbound_method
define_method(name) do |*args|
bound_method ||= unbound_method.bind(self)
bound_method.call(*args)
end
end
end
I do not really see caching that extends the instance on which #reveal
was invoked. There is actually one closure per instance (more
correctly: one closure per call of reveal) and so no global cache.
Where exactly is your problem?
Cheers
robert
···
2009/1/25 Gregory Brown <gregory.t.brown@gmail.com>:
--
remember.guy do |as, often| as.you_can - without end
I do not really see caching that extends the instance on which #reveal
was invoked. There is actually one closure per instance (more
correctly: one closure per call of reveal) and so no global cache.
Where exactly is your problem?
I think you missed that the local variable was at the class level.
I've been working on a chapter for "Ruby Best Practices"[0] about the
dynamic nature of Ruby, and I initially thought Jim Weirich's
BlankSlate would be an excellent example (even though 1.9 has
BasicObject).
However, when I look at the reveal class method, I found what seems to
be a fairly major limitation that I am having a hard time glossing
over in my prose.
The issue is that when you reveal a previously hidden method, the
UnboundMethod that was stored gets bound to whatever the first
instance is that calls the revealed method, because Jim's code caches
the bound_method via a closure. The best 'fix' I got was to drop the
caching and rebind the method to self on every single method call, but
I wasn't able to get away with that without saying "This is probably a
performance nightmare". Does anyone have a better suggested fix? The
original code for reveal, pulled from the builder gem, is below.
From what you write caching a bound method seems a bad idea because it
is instance specific. Caching of an unbound method would seem much
more reasonable (unless, that is, I am missing something).
### Code pulled from blankslate.rb in the builder gem.
# Redefine a previously hidden method so that it may be called on a blank
# slate object.
def reveal(name)
bound_method = nil
unbound_method = find_hidden_method(name)
fail "Don't know how to reveal method '#{name}'" unless unbound_method
define_method(name) do |*args|
bound_method ||= unbound_method.bind(self)
bound_method.call(*args)
end
end
end
I do not really see caching that extends the instance on which #reveal
was invoked. There is actually one closure per instance (more
correctly: one closure per call of reveal) and so no global cache.
Where exactly is your problem?
Let's say you do BasicSlate.reveal(:meth). Then you do slate.meth. At
that point, bound_method in the closure is the meth method, bound to
the object slate. When you then call other_slate.meth, the ||=
short-circuits, and bound_method is *still* the unbound method bound
to slate (not to other_slat).
So every time you call meth, you're calling it on the same instance.
David
···
On Mon, 26 Jan 2009, Robert Klemme wrote:
2009/1/25 Gregory Brown <gregory.t.brown@gmail.com>:
I do not really see caching that extends the instance on which #reveal
was invoked. There is actually one closure per instance (more
correctly: one closure per call of reveal) and so no global cache.
Where exactly is your problem?
I think you missed that the local variable was at the class level.
Right, you stated so in the text and then I got confused by "unbound_method.bind(self)".
But in that case my remark about binding at class level remains true: there is no point in binding to a particular instance at class level and caching that. And your code seems to indicate that you agree with me there.
Sorry -- I posted my reply to your earlier post without having noticed
this post.
David
···
On Tue, 27 Jan 2009, Robert Klemme wrote:
On 26.01.2009 17:18, Gregory Brown wrote:
On Mon, Jan 26, 2009 at 8:55 AM, Robert Klemme >> <shortcutter@googlemail.com> wrote:
I do not really see caching that extends the instance on which #reveal
was invoked. There is actually one closure per instance (more
correctly: one closure per call of reveal) and so no global cache.
Where exactly is your problem?
I think you missed that the local variable was at the class level.
Right, you stated so in the text and then I got confused by "unbound_method.bind(self)".
I do not really see caching that extends the instance on which #reveal
was invoked. There is actually one closure per instance (more
correctly: one closure per call of reveal) and so no global cache.
Where exactly is your problem?
I think you missed that the local variable was at the class level.
Right, you stated so in the text and then I got confused by
"unbound_method.bind(self)".
But in that case my remark about binding at class level remains true: there
is no point in binding to a particular instance at class level and caching
that. And your code seems to indicate that you agree with me there.
Cool. This is what I'm going to go with in my chapter. I emailed Jim
to see if I can get a definitive 'yes, this is a) a bug or b) some
special casing for builder's needs or c) some special sauce that you
missed'. My guess is it's a), but stranger things have happened
-greg
···
On Mon, Jan 26, 2009 at 4:07 PM, Robert Klemme <shortcutter@googlemail.com> wrote:
On 26.01.2009 17:18, Gregory Brown wrote:
On Mon, Jan 26, 2009 at 8:55 AM, Robert Klemme >> <shortcutter@googlemail.com> wrote:
I do not really see caching that extends the instance on which #reveal
was invoked. There is actually one closure per instance (more
correctly: one closure per call of reveal) and so no global cache.
Where exactly is your problem?
I think you missed that the local variable was at the class level.
Right, you stated so in the text and then I got confused by
"unbound_method.bind(self)".
But in that case my remark about binding at class level remains true: there
is no point in binding to a particular instance at class level and caching
that. And your code seems to indicate that you agree with me there.