ruby-core@ruby-lang.org archive (unofficial mirror)
 help / color / mirror / Atom feed
* [ruby-core:100597] [Ruby master Feature#17288] Optimize __send__ call with a literal method name
@ 2020-10-27  8:32 muraken
  2020-10-27 18:35 ` [ruby-core:100610] " eregontp
                   ` (11 more replies)
  0 siblings, 12 replies; 13+ messages in thread
From: muraken @ 2020-10-27  8:32 UTC (permalink / raw)
  To: ruby-core

Issue #17288 has been reported by mrkn (Kenta Murata).

----------------------------------------
Feature #17288: Optimize __send__ call with a literal method name
https://bugs.ruby-lang.org/issues/17288

* Author: mrkn (Kenta Murata)
* Status: Open
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
----------------------------------------
I made a patch to optimize a `__send__` call with a literal method name.  This optimization replaces a `__send__` method call with a `send` instruction.  The patch is available in [this pull-request](https://github.com/ruby/ruby/pull/3707).

By this change, the redefined `__send__` method is no longer called when it is called by a literal method name.  I guess it is no problem because the following warning message is displayed for a long time.

    $ ruby -e 'def __send__; end'
    -e:1: warning: redefining `__send__' may cause serious problems

This change makes the optimized case x5~x6 faster.  The benchmark result is below:

```
$ make benchmark COMPARE_RUBY="../../ruby/build-o3/ruby" ITEM=vm_send.yml
(snip)
# Iteration per second (i/s)

|             |compare-ruby|built-ruby|
|:------------|-----------:|---------:|
|vm_send      |     18.536M|  113.778M|
|             |           -|     6.14x|
|vm_send_var  |     18.085M|   16.595M|
|             |       1.09x|         -|
```



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [ruby-core:100610] [Ruby master Feature#17288] Optimize __send__ call with a literal method name
  2020-10-27  8:32 [ruby-core:100597] [Ruby master Feature#17288] Optimize __send__ call with a literal method name muraken
@ 2020-10-27 18:35 ` eregontp
  2020-10-28  8:25 ` [ruby-core:100616] " shyouhei
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: eregontp @ 2020-10-27 18:35 UTC (permalink / raw)
  To: ruby-core

Issue #17288 has been updated by Eregon (Benoit Daloze).


I think `obj.send(:foo)` should be optimized too, to not create a an arbitrary performance difference between `send` and `__send__`.
`send` is also used ~15x used more frequently than `__send__` in gems, so that would be even more valuable.

Of course, optimizing `send` means checking that `obj.method(:send)` corresponds to `Kernel#send` (somewhat similar to `+`).

@matz suggested an operator for `__send__` in https://twitter.com/yukihiro_matz/status/1320496276476063744, that might be a cleaner solution.
OTOH, it would not benefit existing code and not be adopted for a few years.

FWIW in TruffleRuby I've been thinking to optimize all 3 sends (`__send__,` send, public_send), since they are hidden from backtraces, etc, and optimizing them makes it simpler to remove them from backtraces, etc. It would also reduce the overhead in interpreter for `send/__send__(:private_method)` calls (in JITed code, those have no overhead). I would still check the correct method in all cases, changing semantics should IMHO be the last resort.

----------------------------------------
Feature #17288: Optimize __send__ call with a literal method name
https://bugs.ruby-lang.org/issues/17288#change-88241

* Author: mrkn (Kenta Murata)
* Status: Open
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
----------------------------------------
I made a patch to optimize a `__send__` call with a literal method name.  This optimization replaces a `__send__` method call with a `send` instruction.  The patch is available in [this pull-request](https://github.com/ruby/ruby/pull/3707).

By this change, the redefined `__send__` method is no longer called when it is called by a literal method name.  I guess it is no problem because the following warning message is displayed for a long time.

    $ ruby -e 'def __send__; end'
    -e:1: warning: redefining `__send__' may cause serious problems

This change makes the optimized case x5~x6 faster.  The benchmark result is below:

```
$ make benchmark COMPARE_RUBY="../../ruby/build-o3/ruby" ITEM=vm_send.yml
(snip)
# Iteration per second (i/s)

|             |compare-ruby|built-ruby|
|:------------|-----------:|---------:|
|vm_send      |     18.536M|  113.778M|
|             |           -|     6.14x|
|vm_send_var  |     18.085M|   16.595M|
|             |       1.09x|         -|
```



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [ruby-core:100616] [Ruby master Feature#17288] Optimize __send__ call with a literal method name
  2020-10-27  8:32 [ruby-core:100597] [Ruby master Feature#17288] Optimize __send__ call with a literal method name muraken
  2020-10-27 18:35 ` [ruby-core:100610] " eregontp
@ 2020-10-28  8:25 ` shyouhei
  2020-10-28  8:34 ` [ruby-core:100617] " zverok.offline
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: shyouhei @ 2020-10-28  8:25 UTC (permalink / raw)
  To: ruby-core

Issue #17288 has been updated by shyouhei (Shyouhei Urabe).


Hello, I'm against this optimisation.  `obj.__send__(:method)` should just be written as `obj.method`.

Not against the ability to write `obj.__send__(:method)`, but `obj.method` must be the preferable way and thus must be the fastest thing.

This patch adds complexity just to encourage people to follow the wrong way.  This is -1 to me.


----------------------------------------
Feature #17288: Optimize __send__ call with a literal method name
https://bugs.ruby-lang.org/issues/17288#change-88248

* Author: mrkn (Kenta Murata)
* Status: Open
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
----------------------------------------
I made a patch to optimize a `__send__` call with a literal method name.  This optimization replaces a `__send__` method call with a `send` instruction.  The patch is available in [this pull-request](https://github.com/ruby/ruby/pull/3707).

By this change, the redefined `__send__` method is no longer called when it is called by a literal method name.  I guess it is no problem because the following warning message is displayed for a long time.

    $ ruby -e 'def __send__; end'
    -e:1: warning: redefining `__send__' may cause serious problems

This change makes the optimized case x5~x6 faster.  The benchmark result is below:

```
$ make benchmark COMPARE_RUBY="../../ruby/build-o3/ruby" ITEM=vm_send.yml
(snip)
# Iteration per second (i/s)

|             |compare-ruby|built-ruby|
|:------------|-----------:|---------:|
|vm_send      |     18.536M|  113.778M|
|             |           -|     6.14x|
|vm_send_var  |     18.085M|   16.595M|
|             |       1.09x|         -|
```



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [ruby-core:100617] [Ruby master Feature#17288] Optimize __send__ call with a literal method name
  2020-10-27  8:32 [ruby-core:100597] [Ruby master Feature#17288] Optimize __send__ call with a literal method name muraken
  2020-10-27 18:35 ` [ruby-core:100610] " eregontp
  2020-10-28  8:25 ` [ruby-core:100616] " shyouhei
@ 2020-10-28  8:34 ` zverok.offline
  2020-10-28  9:58 ` [ruby-core:100618] " shyouhei
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: zverok.offline @ 2020-10-28  8:34 UTC (permalink / raw)
  To: ruby-core

Issue #17288 has been updated by zverok (Victor Shepelev).


@shyouhei what about private methods?

----------------------------------------
Feature #17288: Optimize __send__ call with a literal method name
https://bugs.ruby-lang.org/issues/17288#change-88249

* Author: mrkn (Kenta Murata)
* Status: Open
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
----------------------------------------
I made a patch to optimize a `__send__` call with a literal method name.  This optimization replaces a `__send__` method call with a `send` instruction.  The patch is available in [this pull-request](https://github.com/ruby/ruby/pull/3707).

By this change, the redefined `__send__` method is no longer called when it is called by a literal method name.  I guess it is no problem because the following warning message is displayed for a long time.

    $ ruby -e 'def __send__; end'
    -e:1: warning: redefining `__send__' may cause serious problems

This change makes the optimized case x5~x6 faster.  The benchmark result is below:

```
$ make benchmark COMPARE_RUBY="../../ruby/build-o3/ruby" ITEM=vm_send.yml
(snip)
# Iteration per second (i/s)

|             |compare-ruby|built-ruby|
|:------------|-----------:|---------:|
|vm_send      |     18.536M|  113.778M|
|             |           -|     6.14x|
|vm_send_var  |     18.085M|   16.595M|
|             |       1.09x|         -|
```



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [ruby-core:100618] [Ruby master Feature#17288] Optimize __send__ call with a literal method name
  2020-10-27  8:32 [ruby-core:100597] [Ruby master Feature#17288] Optimize __send__ call with a literal method name muraken
                   ` (2 preceding siblings ...)
  2020-10-28  8:34 ` [ruby-core:100617] " zverok.offline
@ 2020-10-28  9:58 ` shyouhei
  2020-10-28 10:09 ` [ruby-core:100619] " eregontp
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: shyouhei @ 2020-10-28  9:58 UTC (permalink / raw)
  To: ruby-core

Issue #17288 has been updated by shyouhei (Shyouhei Urabe).


zverok (Victor Shepelev) wrote in #note-3:
> @shyouhei what about private methods?

Private methods shall not be called at the first place. Period. That is breaking encapsulation.

Again I’m not against the ability to do such things. But we must not encourage people.

----------------------------------------
Feature #17288: Optimize __send__ call with a literal method name
https://bugs.ruby-lang.org/issues/17288#change-88250

* Author: mrkn (Kenta Murata)
* Status: Open
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
----------------------------------------
I made a patch to optimize a `__send__` call with a literal method name.  This optimization replaces a `__send__` method call with a `send` instruction.  The patch is available in [this pull-request](https://github.com/ruby/ruby/pull/3707).

By this change, the redefined `__send__` method is no longer called when it is called by a literal method name.  I guess it is no problem because the following warning message is displayed for a long time.

    $ ruby -e 'def __send__; end'
    -e:1: warning: redefining `__send__' may cause serious problems

This change makes the optimized case x5~x6 faster.  The benchmark result is below:

```
$ make benchmark COMPARE_RUBY="../../ruby/build-o3/ruby" ITEM=vm_send.yml
(snip)
# Iteration per second (i/s)

|             |compare-ruby|built-ruby|
|:------------|-----------:|---------:|
|vm_send      |     18.536M|  113.778M|
|             |           -|     6.14x|
|vm_send_var  |     18.085M|   16.595M|
|             |       1.09x|         -|
```



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [ruby-core:100619] [Ruby master Feature#17288] Optimize __send__ call with a literal method name
  2020-10-27  8:32 [ruby-core:100597] [Ruby master Feature#17288] Optimize __send__ call with a literal method name muraken
                   ` (3 preceding siblings ...)
  2020-10-28  9:58 ` [ruby-core:100618] " shyouhei
@ 2020-10-28 10:09 ` eregontp
  2020-10-28 12:32 ` [ruby-core:100621] " shyouhei
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: eregontp @ 2020-10-28 10:09 UTC (permalink / raw)
  To: ruby-core

Issue #17288 has been updated by Eregon (Benoit Daloze).


Here are the first 1000 .send() usages in gems:
https://gist.github.com/eregon/21c8f14c478089c1a9295c21661583a9
420 of them use a literal Symbol for the first argument.

> Private methods shall not be called at the first place. Period.

It's not as simple, there are many cases where it's reasonable to call private methods.
For instance things like `Module#{include,prepend,alias_method,define_method}` used to be private, and `Module#remove_const` still is.
Some gems call their own private methods in tests, which seems fair enough.

shyouhei (Shyouhei Urabe) wrote in #note-2:
> Not against the ability to write `obj.__send__(:method)`, but `obj.method` must be the preferable way and thus must be the fastest thing.

I would think nobody prefers `obj.__send__(:some_method)` to `obj.some_method` if `some_method` is public, so it seems a non-issue to me.
And anyway `obj.some_method` would always be as fast or faster than `obj.__send__(:some_method)`, never slower (that would be a performance bug).

----------------------------------------
Feature #17288: Optimize __send__ call with a literal method name
https://bugs.ruby-lang.org/issues/17288#change-88251

* Author: mrkn (Kenta Murata)
* Status: Open
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
----------------------------------------
I made a patch to optimize a `__send__` call with a literal method name.  This optimization replaces a `__send__` method call with a `send` instruction.  The patch is available in [this pull-request](https://github.com/ruby/ruby/pull/3707).

By this change, the redefined `__send__` method is no longer called when it is called by a literal method name.  I guess it is no problem because the following warning message is displayed for a long time.

    $ ruby -e 'def __send__; end'
    -e:1: warning: redefining `__send__' may cause serious problems

This change makes the optimized case x5~x6 faster.  The benchmark result is below:

```
$ make benchmark COMPARE_RUBY="../../ruby/build-o3/ruby" ITEM=vm_send.yml
(snip)
# Iteration per second (i/s)

|             |compare-ruby|built-ruby|
|:------------|-----------:|---------:|
|vm_send      |     18.536M|  113.778M|
|             |           -|     6.14x|
|vm_send_var  |     18.085M|   16.595M|
|             |       1.09x|         -|
```



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [ruby-core:100621] [Ruby master Feature#17288] Optimize __send__ call with a literal method name
  2020-10-27  8:32 [ruby-core:100597] [Ruby master Feature#17288] Optimize __send__ call with a literal method name muraken
                   ` (4 preceding siblings ...)
  2020-10-28 10:09 ` [ruby-core:100619] " eregontp
@ 2020-10-28 12:32 ` shyouhei
  2020-10-28 20:48 ` [ruby-core:100624] " marcandre-ruby-core
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: shyouhei @ 2020-10-28 12:32 UTC (permalink / raw)
  To: ruby-core

Issue #17288 has been updated by shyouhei (Shyouhei Urabe).


Eregon (Benoit Daloze) wrote in #note-5:
> Here are the first 1000 .send() usages in gems:
> https://gist.github.com/eregon/21c8f14c478089c1a9295c21661583a9
> 420 of them use a literal Symbol for the first argument.

So? I don’t think we should follow that.  If people misunderstand what an OOPL is, we would better not confirm that.

> > Private methods shall not be called at the first place. Period.
> 
> It's not as simple, there are many cases where it's reasonable to call private methods.
> For instance things like `Module#{include,prepend,alias_method,define_method}` used to be private, and `Module#remove_const` still is.

They are/were private for reasons.  Private methods can be made public later, but that must have been done with really careful considerations by the author. Not by callee people.

> Some gems call their own private methods in tests, which seems fair enough.

Testing private methods! That itself has a bunch of discussions.

But even if we put those topics aside, do we want to optimise such tests? I feel that is very low-priority.

> shyouhei (Shyouhei Urabe) wrote in #note-2:
> > Not against the ability to write `obj.__send__(:method)`, but `obj.method` must be the preferable way and thus must be the fastest thing.
> 
> I would think nobody prefers `obj.__send__(:some_method)` to `obj.some_method` if `some_method` is public, so it seems a non-issue to me.
> And anyway `obj.some_method` would always be as fast or faster than `obj.__send__(:some_method)`, never slower (that would be a performance bug).

OK. So the point is wether we want people to call a private method or not. I’m still against that. Encapsulation is a very basic OO principle that Ruby employs. I want that be honoured.

The proposed patch is sending a wrong signal.

----------------------------------------
Feature #17288: Optimize __send__ call with a literal method name
https://bugs.ruby-lang.org/issues/17288#change-88254

* Author: mrkn (Kenta Murata)
* Status: Open
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
----------------------------------------
I made a patch to optimize a `__send__` call with a literal method name.  This optimization replaces a `__send__` method call with a `send` instruction.  The patch is available in [this pull-request](https://github.com/ruby/ruby/pull/3707).

By this change, the redefined `__send__` method is no longer called when it is called by a literal method name.  I guess it is no problem because the following warning message is displayed for a long time.

    $ ruby -e 'def __send__; end'
    -e:1: warning: redefining `__send__' may cause serious problems

This change makes the optimized case x5~x6 faster.  The benchmark result is below:

```
$ make benchmark COMPARE_RUBY="../../ruby/build-o3/ruby" ITEM=vm_send.yml
(snip)
# Iteration per second (i/s)

|             |compare-ruby|built-ruby|
|:------------|-----------:|---------:|
|vm_send      |     18.536M|  113.778M|
|             |           -|     6.14x|
|vm_send_var  |     18.085M|   16.595M|
|             |       1.09x|         -|
```



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [ruby-core:100624] [Ruby master Feature#17288] Optimize __send__ call with a literal method name
  2020-10-27  8:32 [ruby-core:100597] [Ruby master Feature#17288] Optimize __send__ call with a literal method name muraken
                   ` (5 preceding siblings ...)
  2020-10-28 12:32 ` [ruby-core:100621] " shyouhei
@ 2020-10-28 20:48 ` marcandre-ruby-core
  2020-10-28 20:59 ` [ruby-core:100625] " shevegen
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: marcandre-ruby-core @ 2020-10-28 20:48 UTC (permalink / raw)
  To: ruby-core

Issue #17288 has been updated by marcandre (Marc-Andre Lafortune).


shyouhei (Shyouhei Urabe) wrote in #note-4:
> Private methods shall not be called at the first place. Period. That is breaking encapsulation.

I wish that was the case, but Ruby access is *not expressive enough* for this to be the case.

```ruby
class Foo
  class << self
    private def special # General users should not call or rely on `special`
    end
  end

  def foo # even from inside our own class...
    self.class.special # this won't work without `send`
  end

  class Bar
    # Bar is a helper class, written by us
    def foo 
      Foo.special # we want to call special, we need to use `send`
    end
  end
end
```
Note: using `protected` changes nothing in this example

In so many gems, you have to either mark method as `private` and use `send`, or else use `# @api private` but that's just a comment.

----------------------------------------
Feature #17288: Optimize __send__ call with a literal method name
https://bugs.ruby-lang.org/issues/17288#change-88257

* Author: mrkn (Kenta Murata)
* Status: Open
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
----------------------------------------
I made a patch to optimize a `__send__` call with a literal method name.  This optimization replaces a `__send__` method call with a `send` instruction.  The patch is available in [this pull-request](https://github.com/ruby/ruby/pull/3707).

By this change, the redefined `__send__` method is no longer called when it is called by a literal method name.  I guess it is no problem because the following warning message is displayed for a long time.

    $ ruby -e 'def __send__; end'
    -e:1: warning: redefining `__send__' may cause serious problems

This change makes the optimized case x5~x6 faster.  The benchmark result is below:

```
$ make benchmark COMPARE_RUBY="../../ruby/build-o3/ruby" ITEM=vm_send.yml
(snip)
# Iteration per second (i/s)

|             |compare-ruby|built-ruby|
|:------------|-----------:|---------:|
|vm_send      |     18.536M|  113.778M|
|             |           -|     6.14x|
|vm_send_var  |     18.085M|   16.595M|
|             |       1.09x|         -|
```



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [ruby-core:100625] [Ruby master Feature#17288] Optimize __send__ call with a literal method name
  2020-10-27  8:32 [ruby-core:100597] [Ruby master Feature#17288] Optimize __send__ call with a literal method name muraken
                   ` (6 preceding siblings ...)
  2020-10-28 20:48 ` [ruby-core:100624] " marcandre-ruby-core
@ 2020-10-28 20:59 ` shevegen
  2020-10-29  0:11 ` [ruby-core:100628] " shyouhei
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: shevegen @ 2020-10-28 20:59 UTC (permalink / raw)
  To: ruby-core

Issue #17288 has been updated by shevegen (Robert A. Heiler).


Just a few things:

We need to also remember **.instance_variable_get()** and **.instance_variable_set()**.

Ruby does not quite use a similar "restriction-style" OOP like, say, java. It will
depend a lot on the style and preferences of the ruby user. Personally I much prefer
the .send() approach, so zverok's comment about what to do with private stuff, I
say .send() it all the way! Gimme all the goodies; don't restrict me. :D (I tend
to use "private" rarely, and mostly as cue for documentation, rather than as a
"I want to restrict this").

On the topic of .send(), .public_send() and .__send__(), I have a few gems that use
.send(). I like .send(). I do not use .public_send() or .__send__(), so even a new
operator for .__send__() would not affect me since I would not need it, most likely.

Last but not least, at first I thought .send() will be deprecated, e. g. I misread
this change here by nobu:

https://git.ruby-lang.org/ruby.git/commit/?id=3198e7abd70bd2af977f2bb6c967e9df8f91adb0

Perhaps this one is also related to the issue here? I think some ruby users may 
wonder what to use, e. g. .send() or .public_send() or .__send__(). This should also
be kept in mind. Note that I have no strong opinion on the issue here at all, my 
only concern would be whether .send() would be changed, but I think I misread that
when I first saw the git-change.

----------------------------------------
Feature #17288: Optimize __send__ call with a literal method name
https://bugs.ruby-lang.org/issues/17288#change-88258

* Author: mrkn (Kenta Murata)
* Status: Open
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
----------------------------------------
I made a patch to optimize a `__send__` call with a literal method name.  This optimization replaces a `__send__` method call with a `send` instruction.  The patch is available in [this pull-request](https://github.com/ruby/ruby/pull/3707).

By this change, the redefined `__send__` method is no longer called when it is called by a literal method name.  I guess it is no problem because the following warning message is displayed for a long time.

    $ ruby -e 'def __send__; end'
    -e:1: warning: redefining `__send__' may cause serious problems

This change makes the optimized case x5~x6 faster.  The benchmark result is below:

```
$ make benchmark COMPARE_RUBY="../../ruby/build-o3/ruby" ITEM=vm_send.yml
(snip)
# Iteration per second (i/s)

|             |compare-ruby|built-ruby|
|:------------|-----------:|---------:|
|vm_send      |     18.536M|  113.778M|
|             |           -|     6.14x|
|vm_send_var  |     18.085M|   16.595M|
|             |       1.09x|         -|
```



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [ruby-core:100628] [Ruby master Feature#17288] Optimize __send__ call with a literal method name
  2020-10-27  8:32 [ruby-core:100597] [Ruby master Feature#17288] Optimize __send__ call with a literal method name muraken
                   ` (7 preceding siblings ...)
  2020-10-28 20:59 ` [ruby-core:100625] " shevegen
@ 2020-10-29  0:11 ` shyouhei
  2020-10-29  4:06 ` [ruby-core:100632] " marcandre-ruby-core
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: shyouhei @ 2020-10-29  0:11 UTC (permalink / raw)
  To: ruby-core

Issue #17288 has been updated by shyouhei (Shyouhei Urabe).


@marcandre Here you are:

```ruby
class Foo
  using Module.new {
    refine Foo.singleton_class do
      def special
      end
    end
  }

  def foo
    self.class.special
  end

  class Bar
    def foo
      Foo.special
    end
  end
end
```

----------------------------------------
Feature #17288: Optimize __send__ call with a literal method name
https://bugs.ruby-lang.org/issues/17288#change-88263

* Author: mrkn (Kenta Murata)
* Status: Open
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
----------------------------------------
I made a patch to optimize a `__send__` call with a literal method name.  This optimization replaces a `__send__` method call with a `send` instruction.  The patch is available in [this pull-request](https://github.com/ruby/ruby/pull/3707).

By this change, the redefined `__send__` method is no longer called when it is called by a literal method name.  I guess it is no problem because the following warning message is displayed for a long time.

    $ ruby -e 'def __send__; end'
    -e:1: warning: redefining `__send__' may cause serious problems

This change makes the optimized case x5~x6 faster.  The benchmark result is below:

```
$ make benchmark COMPARE_RUBY="../../ruby/build-o3/ruby" ITEM=vm_send.yml
(snip)
# Iteration per second (i/s)

|             |compare-ruby|built-ruby|
|:------------|-----------:|---------:|
|vm_send      |     18.536M|  113.778M|
|             |           -|     6.14x|
|vm_send_var  |     18.085M|   16.595M|
|             |       1.09x|         -|
```



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [ruby-core:100632] [Ruby master Feature#17288] Optimize __send__ call with a literal method name
  2020-10-27  8:32 [ruby-core:100597] [Ruby master Feature#17288] Optimize __send__ call with a literal method name muraken
                   ` (8 preceding siblings ...)
  2020-10-29  0:11 ` [ruby-core:100628] " shyouhei
@ 2020-10-29  4:06 ` marcandre-ruby-core
  2020-10-29 20:12 ` [ruby-core:100660] " eregontp
  2020-10-30  0:17 ` [ruby-core:100663] " shyouhei
  11 siblings, 0 replies; 13+ messages in thread
From: marcandre-ruby-core @ 2020-10-29  4:06 UTC (permalink / raw)
  To: ruby-core

Issue #17288 has been updated by marcandre (Marc-Andre Lafortune).


shyouhei (Shyouhei Urabe) wrote in #note-9:
> @marcandre Here you are:
>
> [snip brilliant code]

0) I was wrong, I retract what I said.

1) My mind is blown. I have always thought of refinements as a way to safely monkey-patch other people's classes, in particular builtin classes. Never as a way to structure access to our own classes. This can also be very fine-grained if desired. This is brilliant @shyouhei!

2) Goto 1

I made a quick POC with RuboCop to isolate methods that I've always wanted to isolate and the only issues was mocking internal methods:

```
Failure/Error: allow(cop).to receive(:complete_investigation).and_return(cop_report)
#<Fake::FakeCop:0x00007fed7e1ce530 ...> does not implement: complete_investigation
```

I'm fine with this, I already try to avoid stub and mocks anyways (10 tests out of 14515 :-) and I'm sure I can find better ways around that.

I also wanted to check any performance impact. I couldn't see any (running tests or running RuboCop). Are there any known circumstances where performance would be affected?

Are there gems using this technique?
Blog posts discussing this?

----------------------------------------
Feature #17288: Optimize __send__ call with a literal method name
https://bugs.ruby-lang.org/issues/17288#change-88267

* Author: mrkn (Kenta Murata)
* Status: Open
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
----------------------------------------
I made a patch to optimize a `__send__` call with a literal method name.  This optimization replaces a `__send__` method call with a `send` instruction.  The patch is available in [this pull-request](https://github.com/ruby/ruby/pull/3707).

By this change, the redefined `__send__` method is no longer called when it is called by a literal method name.  I guess it is no problem because the following warning message is displayed for a long time.

    $ ruby -e 'def __send__; end'
    -e:1: warning: redefining `__send__' may cause serious problems

This change makes the optimized case x5~x6 faster.  The benchmark result is below:

```
$ make benchmark COMPARE_RUBY="../../ruby/build-o3/ruby" ITEM=vm_send.yml
(snip)
# Iteration per second (i/s)

|             |compare-ruby|built-ruby|
|:------------|-----------:|---------:|
|vm_send      |     18.536M|  113.778M|
|             |           -|     6.14x|
|vm_send_var  |     18.085M|   16.595M|
|             |       1.09x|         -|
```



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [ruby-core:100660] [Ruby master Feature#17288] Optimize __send__ call with a literal method name
  2020-10-27  8:32 [ruby-core:100597] [Ruby master Feature#17288] Optimize __send__ call with a literal method name muraken
                   ` (9 preceding siblings ...)
  2020-10-29  4:06 ` [ruby-core:100632] " marcandre-ruby-core
@ 2020-10-29 20:12 ` eregontp
  2020-10-30  0:17 ` [ruby-core:100663] " shyouhei
  11 siblings, 0 replies; 13+ messages in thread
From: eregontp @ 2020-10-29 20:12 UTC (permalink / raw)
  To: ruby-core

Issue #17288 has been updated by Eregon (Benoit Daloze).


How about a private module instead?
```ruby
class Foo
  module Helpers
    def self.special # General users should not call or rely on `special`
      :special
    end
  end
  private_constant :Helpers

  def foo # even from inside our own class...
    Helpers.special # this won't work without `send`
  end

  class Bar
    # Bar is a helper class, written by us
    def foo 
      Helpers.special # we want to call special, we need to use `send`
    end
  end
end

p Foo.new.foo # => :special
p Foo::Bar.new.foo # => :special

p Foo::Helpers.special # =>  private constant Foo::Helpers referenced (NameError)
```

Refinements seems rather heavy to me for this case, notably it creates extra modules, and makes initial lookups slower (once cached it shouldn't matter much).
For an uncached call (e.g. `refine Object` and many different receivers at some call site), I think the overhead would be noticeable.

Also if the refinements need to be used in multiple files, the module passed to `using` needs to be named, and stored in a private constant.
If done so, there seem little point to `using PrivateHelpers; ...; self.class.foo` vs `PrivateHelpers.foo`, except maybe for instance methods added on existing classes.
But then one could simply use a private method on `Foo` to begin with.

I'm probably biased against refinements because the semantics around refinements + super or eval are fairly messy.

----------------------------------------
Feature #17288: Optimize __send__ call with a literal method name
https://bugs.ruby-lang.org/issues/17288#change-88300

* Author: mrkn (Kenta Murata)
* Status: Open
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
----------------------------------------
I made a patch to optimize a `__send__` call with a literal method name.  This optimization replaces a `__send__` method call with a `send` instruction.  The patch is available in [this pull-request](https://github.com/ruby/ruby/pull/3707).

By this change, the redefined `__send__` method is no longer called when it is called by a literal method name.  I guess it is no problem because the following warning message is displayed for a long time.

    $ ruby -e 'def __send__; end'
    -e:1: warning: redefining `__send__' may cause serious problems

This change makes the optimized case x5~x6 faster.  The benchmark result is below:

```
$ make benchmark COMPARE_RUBY="../../ruby/build-o3/ruby" ITEM=vm_send.yml
(snip)
# Iteration per second (i/s)

|             |compare-ruby|built-ruby|
|:------------|-----------:|---------:|
|vm_send      |     18.536M|  113.778M|
|             |           -|     6.14x|
|vm_send_var  |     18.085M|   16.595M|
|             |       1.09x|         -|
```



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [ruby-core:100663] [Ruby master Feature#17288] Optimize __send__ call with a literal method name
  2020-10-27  8:32 [ruby-core:100597] [Ruby master Feature#17288] Optimize __send__ call with a literal method name muraken
                   ` (10 preceding siblings ...)
  2020-10-29 20:12 ` [ruby-core:100660] " eregontp
@ 2020-10-30  0:17 ` shyouhei
  11 siblings, 0 replies; 13+ messages in thread
From: shyouhei @ 2020-10-30  0:17 UTC (permalink / raw)
  To: ruby-core

Issue #17288 has been updated by shyouhei (Shyouhei Urabe).


JFYI I learned the use of "immediate" refinement from rails.
https://github.com/rails/rails/pull/27363

----------------------------------------
Feature #17288: Optimize __send__ call with a literal method name
https://bugs.ruby-lang.org/issues/17288#change-88303

* Author: mrkn (Kenta Murata)
* Status: Open
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
----------------------------------------
I made a patch to optimize a `__send__` call with a literal method name.  This optimization replaces a `__send__` method call with a `send` instruction.  The patch is available in [this pull-request](https://github.com/ruby/ruby/pull/3707).

By this change, the redefined `__send__` method is no longer called when it is called by a literal method name.  I guess it is no problem because the following warning message is displayed for a long time.

    $ ruby -e 'def __send__; end'
    -e:1: warning: redefining `__send__' may cause serious problems

This change makes the optimized case x5~x6 faster.  The benchmark result is below:

```
$ make benchmark COMPARE_RUBY="../../ruby/build-o3/ruby" ITEM=vm_send.yml
(snip)
# Iteration per second (i/s)

|             |compare-ruby|built-ruby|
|:------------|-----------:|---------:|
|vm_send      |     18.536M|  113.778M|
|             |           -|     6.14x|
|vm_send_var  |     18.085M|   16.595M|
|             |       1.09x|         -|
```



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-10-30  0:17 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-27  8:32 [ruby-core:100597] [Ruby master Feature#17288] Optimize __send__ call with a literal method name muraken
2020-10-27 18:35 ` [ruby-core:100610] " eregontp
2020-10-28  8:25 ` [ruby-core:100616] " shyouhei
2020-10-28  8:34 ` [ruby-core:100617] " zverok.offline
2020-10-28  9:58 ` [ruby-core:100618] " shyouhei
2020-10-28 10:09 ` [ruby-core:100619] " eregontp
2020-10-28 12:32 ` [ruby-core:100621] " shyouhei
2020-10-28 20:48 ` [ruby-core:100624] " marcandre-ruby-core
2020-10-28 20:59 ` [ruby-core:100625] " shevegen
2020-10-29  0:11 ` [ruby-core:100628] " shyouhei
2020-10-29  4:06 ` [ruby-core:100632] " marcandre-ruby-core
2020-10-29 20:12 ` [ruby-core:100660] " eregontp
2020-10-30  0:17 ` [ruby-core:100663] " shyouhei

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).