Issue #16188 has been updated by jeremyevans0 (Jeremy Evans).


Eregon (Benoit Daloze) wrote:
> jeremyevans0 (Jeremy Evans) wrote:
> > Comparing 2.6 to 2.7 is irrelevant in regards to the discussion of the effect of `ruby2_keywords`.  There are many other changes between 2.6 and 2.7 that have a much larger effect than `ruby2_keywords`.
> 
> Which other changes would affect calls like `req(*arr)`?
> AFAIK, there is no change for that in 2.7 except `ruby2_keywords`, is it?

There are a ton of other changes in 2.7 that could affect performance besides `ruby2_keywords`.

> > If you want to measure  the effect of `ruby2_keywords`, you need to benchmark the master branch against the master branch with the removal of `ruby2_keywords`, as I did in an earlier comment, and as your TruffleRuby benchmark did.
> 
> I'm concerned your diff above still keeps some overhead, e.g., there are still references to the `ruby2_keywords` flag.

I don't think those references are in a case the benchmark would hit, but I could be wrong.  As you mentioned in a later comment, you prepared your own diff for just removing `ruby2_keywords`.

> > Considering the percentage of calls using splats without keyword splats compared to all other calls,
> 
> How much do you think is that percentage? Right now I would expect far more `*args` than `*args, **kwargs`.

There are probably more `*args` than `*args, **kwargs`.  However, that is not what is important. I would guess that method calls with no arguments or only explicit arguments outnumber calls with splats at least 10-1 if not 100-1.  So a 10% performance difference would be 0.1-1% difference in a real world benchmark.

> > it seems unlikely this change will have a significant effect in a real world benchmark on TruffleRuby.
> 
> I think it can actually, every `foo(*args)` call, typically used for delegating arguments to another method is can be up to ~11% slower.
> Some gems use `foo(*args)` quite often.
> Typically, `method_missing` uses it too, and `method_missing` can be performance-sensitive (https://twitter.com/headius/status/1197972352488767489).

Well, we could guess, or you could pick an existing real world benchmark and run it and report the results for:

* CRuby master
* CRuby master with ruby2_keywords removed
* TruffleRuby master
* TruffleRuby master with ruby2_keywords added

I would be surprised if there was a measurable difference.

> https://github.com/ruby/ruby/compare/master...eregon:no-ruby2_keywords is a more extensive removal of `ruby2_keywords`, although not complete yet.
> It's actually a lot of extra logic in `setup_parameters_complex`.

Do all non-`ruby2_keywords` tests still pass with that?  If so, what are the benchmark results with the patch?

> I don't think anyone is seriously considering using `ruby2_keywords` in Ruby 3+ (or at least in Ruby 3.3+).
> So it seems CRuby should improve performance for `*args, **kwargs`, or propose another way to delegate more efficiently (e.g., `...` is one way I think many Rubyists would like).

I disagree. I think that assuming it works better, most Ruby programmers will consider using it.  Now, if explicit keyword argument performance is improved so that `ruby2_keywords` is no longer the faster way, then I could see there no longer be a need for it.  However, that is far from a forgone conclusion.


> It requires extra modifications, yes, but IMHO very easy modifications: marking where positional args should be converted to kwargs for delegation.

Marking methods that do delegation is much easier than modifying the internals of methods, especially in more complex cases such as when the delegating method stores the arguments and they are sent to a target method in a separate method.

> Since users have to think where they need `ruby2_keywords`, I would argue placing `send_keyword_hash` is no harder than adding `ruby2_keywords`, and makes the whole model a lot easier to understand, less magic and fixes performance.

It's definitely more difficult, especially in complex cases.  I don't think it is easier to understand or less magic, it is just more explicit.  As to the performance issues, they are very minor in CRuby and based on your own benchmark fairly minor in TruffleRuby.

> > > * Use another way for delegation in Ruby 2.7 (e.g. the lexical `pass_keywords` or `...`, see https://eregon.me/blog/2019/11/10/the-delegation-challenge-of-ruby27.html )
> > 
> > The lexical `pass_keywords` is not truly lexical, as only the current VM frame was flagged, so the behavior inside blocks in a method was not what the user would expect.  Modifying the implementation to handle lexical VM frames could possibly result in more slowdown, and I'm not sure how to implement it.
> 
> I do mean lexical, including calls inside blocks.
> 
> I would think it's straightforward to implement `pass_keywords`, including blocks, without overhead on unrelated code:
> Modify every call site inside `pass_keywords` methods with `*args`, including those in blocks inside the method, to use a different bytecode.
> In that bytecode's implementation, do the extra logic needed.
> We might also need some prelude logic in the marked method to remember if it was passed kwargs.
> And that's all there is to it, isn't it?

It sounds so simple.  I look forward to your patch implementing it, as I'm sure I could learn much from it.

> > Additionally, there are cases where non-lexical passing is used (e.g. in Rails), and a lexical approach would not handle those cases.
> 
> I showed it can be done with a block: https://github.com/eregon/rails/commit/8b0625ed68

That's not `pass_keywords` handling the case, that's you modifying the code so that pass_keywords can handle it.  Surely you can see how that is a more difficult change to make.

> > `ruby2_keywords` handles that case, and many other real world cases that the lexical approach does not handle.
> 
> Can you give an example the above approach (capturing the call inside the lambda) cannot handle?

No.  Certainly there is a way to always move code into the lexical scope.  It is a more difficult change and it makes the resulting code harder to understand, but it is possible.

----------------------------------------
Misc #16188: What are the performance implications of the new keyword arguments in 2.7 and 3.0?
https://bugs.ruby-lang.org/issues/16188#change-82776

* Author: Eregon (Benoit Daloze)
* Status: Open
* Priority: Normal
* Assignee: jeremyevans0 (Jeremy Evans)
----------------------------------------
In #14183, keyword arguments became further separated from positional arguments.

Contrary to the original design though, keyword and positional arguments are not fully separated for methods not accepting keyword arguments.
Example: `foo(key: :value)` will `def foo(hash)` will pass a positional argument.
This is of course better for compatibility, but I wonder what are the performance implications.

The block argument is completely separate in all versions, so no need to concern ourselves about that.

In Ruby <= 2.6:
* The caller never needs to know about the callee's arguments, it can just take all arguments and pass them as an array.
  The last argument might be used to extract keyword, but this is all done at the callee side.
* Splitting kwargs composed of Symbol and non-Symbol keys can be fairly expensive, but it is a rare occurrence.
  If inlining the callee and kwargs are all passed as a literal Hash at the call site, there shouldn't be any overhead compared to positional arguments once JIT'ed.

In Ruby 2.7:
* The caller needs to pass positional and keyword arguments separately, at least when calling a method accepting kwargs.
  But, if it calls a methods not accepting kwargs, then the "kwargs" (e.g. `foo(key: :value)`) should be treated just like a final Hash positional argument.
* (If we had complete separation, then we could always pass positional and keyword arguments separately, so the caller could once again ignore the callee)

How is the logic implemented in MRI for 2.7?

Specializing the caller for a given callee is a well-known technique.
However, it becomes more difficult if different methods are called from the same callsite (polymorphic call), especially if one accepts kwargs and another does not.
In that case, I think we will see a performance cost to this approach, by having to pass arguments differently based on the method to be called.

What about delegation using `ruby2_keywords`?
Which checks does that add (compared to 2.6) in the merged approach with the Hash flag?



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

Unsubscribe: <mailto:ruby-core-request / ruby-lang.org?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>