Project

General

Profile

Actions

Feature #22134

open

Faster rb_scan_args() for keyword args (optimization)

Feature #22134: Faster rb_scan_args() for keyword args (optimization)

Added by luke-gru (Luke Gruber) 2 months ago. Updated about 18 hours ago.

Status:
Open
Assignee:
-
Target version:
-
[ruby-core:125877]

Description

Motivation

When using the rb_scan_args() API, often we want to find a value for a given keyword argument. In order to do this, we call rb_scan_args() like so:

VALUE str;
VALUE kwargs;
VALUE example;
rb_scan_args(argc, argv, "1:", &str, &kwargs); // duplicates the kwargs hash in argv
if (!NIL_P(kwargs)) rb_get_kwargs(kwargs, &id_example, 0, 1, &example); // mutates the duplicated kwargs hash to retrieve `example:`

This duplicates the keyword args hash given in argv. It would be nice to be able to grab a direct reference to the keyword hash
and to have a variant of rb_get_kwargs() that didn't mutate the passed in hash.

Proposal

Add a new valid format character for rb_scan_args():

VALUE str;
VALUE kwargs;
VALUE example;
rb_scan_args(argc, argv, "1:^", &str, &kwargs); // access kwargs directly from argv
if (!NIL_P(kwargs)) rb_get_kwargs_const(kwargs, &id_example, 0, 1, &example); // don't mutate the passed kwargs hash

This '^' character would only be valid after a ":'.

I have a pull request available for anyone that is interested.

Thank you!

Updated by nobu (Nobuyoshi Nakada) 2 months ago Actions #1 [ruby-core:125879]

What about this alongside HAVE_RB_SCAN_ARGS_BORROW_KEYWORDS:

#define RB_SCAN_ARGS_BORROW_KEYWORDS "^"

It is lengthy but can reduce duplication.

Updated by nobu (Nobuyoshi Nakada) about 18 hours ago Actions #2 [ruby-core:126608]

A small correction to my previous comment: RB_SCAN_ARGS_BORROW_KEYWORDS itself can be used for feature detection, so a separate HAVE_RB_SCAN_ARGS_BORROW_KEYWORDS is unnecessary. Extensions can define it as "" when absent to share the same format string across Ruby versions.

Also, how about naming the non-destructive API rb_lookup_kwargs rather than rb_get_kwargs_const? We could introduce rb_extract_kwargs as a more descriptive name for the existing destructive operation, while retaining rb_get_kwargs as a compatibility alias.

Updated by nobu (Nobuyoshi Nakada) about 18 hours ago Actions #3 [ruby-core:126609]

Measurements used the same Ruby development build on macOS arm64, with JIT disabled. The following are medians of seven samples, in nanoseconds per call:

Arguments Existing approach rb_lookup_kwargs Speedup
One keyword 67.4 54.3 1.24x
One keyword via a frozen hash splat 64.3 47.6 1.35x
Four keywords 121.1 106.6 1.14x
Sixteen keywords via a frozen hash splat 286.5 253.0 1.13x
Actions

Also available in: PDF Atom