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


jeremyevans0 (Jeremy Evans) wrote:
> You shouldn't even take your word, as you haven't implemented it yet.  I'll post a benchmark, and I ask you to do the same.

Fair enough, I'll try to benchmark this soon in TruffleRuby then.
If I can have a call with you it would be helpful to discuss the conceptual implementation for that logic and what examples are good to compare (and ideas related to delegation).

> > For me, it ruins a good part of the performance advantages of separating positional and keyword arguments, because once again there is automatic conversion and "keywords split out from the last Array argument".
> 
> Separating positional and keyword arguments was not done for performance reasons, it was done to avoid the issues that not separating them caused.

I know, but I think separating arguments can be an opportunity for better performance performance and cheaper method/block calls, and this partially removes that opportunity.

Also on the semantics level, I think it would be cleaner if there are no ways to automatically convert a positional Hash to kwargs or vice-versa.
Don't you agree?

By having `ruby2_keywords` in Ruby 3, we're probably going to see some abuse of it, and some confusion about it.

If the goal is to separate positional and keyword arguments, why are we leaving a backdoor?

> The initial proposal for `ruby2_keywords` used a VM frame flag and not a hash object flag, and was thus localized.  Unfortunately, it didn't handle cases that people wanted it to handle.  I think Ruby's goal of programmer happiness should outweigh minor theoretical performance issues.

Yes the frame flag approach was better performance-wise in that it would only affect a known small set of call sites.

> > I think it's a straightforward way to mark exactly which call sites need the legacy conversion behavior.
> > There are likely rather few of those, and it seems worth to avoid degrading the performance of all other call sites with a `*rest` argument and no kwargs.
> 
> `Kernel#send_keyword_hash` requires changing the method definition or adding a separate method definition, when one of the main benefits of `ruby2_keywords` is that you do not need to modify an existing method definition.

Yes, it requires changing the method definition, but in a fairly obvious way.
I would argue everyone changing a delegation method knows very well which is the delegating call site, and marking it with `send_keyword_hash` is completely trivial.

I'd think it's even useful to mark and correspondingly to not mark some call sites which should not pass kwargs.

The current logic where Array#dup dup's the last argument if it's a tagged Hash (6081ddd6e6f2297862b3c7e898d28a76b8f9240b)
seems a particularly not nice workaround for the fact a tagged Hash might be used as kwargs multiple times.
`#send_keyword_hash` would solve this in a more clean, explicit and clear way.

I don't think there is much of a difference to add `ruby2_keywords`, a "visibility modifier" before `def` vs modifying a call site.
Both need modifications of the file.

----------------------------------------
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-82280

* 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>