Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

YJIT: Speculate block arg for c_func_method(&nil) calls #12326

Merged
merged 1 commit into from
Dec 13, 2024

Conversation

XrXr
Copy link
Member

@XrXr XrXr commented Dec 13, 2024

A good amount of call sites always pass nil as block argument, but the
nil doesn't show up in the context. Put a runtime guard for those
cases to handle it. Particular relevant for the ruby-lsp benchmark in
yjit-bench. Up to a 2% speedup across headline benchmarks.

master: ruby 3.4.0dev (2024-12-12T17:26:06Z master c0caf1cc1a) +YJIT +PRISM [x86_64-linux]
+patch: ruby 3.4.0dev (2024-12-12T18:42:41Z +patch c639b0918b) +YJIT +PRISM [x86_64-linux]

--------------  -----------  ----------  -----------  ----------  --------------  -------------
bench           master (ms)  stddev (%)  +patch (ms)  stddev (%)  +patch 1st itr  master/+patch
activerecord    186.6        0.7         185.6        0.7         1.001           1.005        
chunky-png      503.3        0.1         501.8        0.1         1.000           1.003        
erubi-rails     924.8        0.6         922.9        0.6         0.980           1.002        
hexapdf         1903.8       2.5         1986.2       2.8         1.015           0.958        
liquid-c        56.3         1.2         55.7         1.3         1.004           1.012        
liquid-compile  57.9         1.9         58.1         1.9         0.988           0.998        
liquid-render   81.4         1.1         80.0         1.3         1.009           1.018        
lobsters        862.3        1.5         864.1        1.6         1.048           0.998        
mail            120.8        0.6         120.8        0.7         1.001           1.000        
psych-load      1638.7       0.1         1622.9       0.0         1.010           1.010        
railsbench      1944.8       0.3         1941.2       0.1         1.001           1.002        
rubocop         112.6        6.7         112.2        6.6         0.997           1.003        
ruby-lsp        131.8        2.0         120.5        2.0         1.024           1.094        
sequel          68.8         0.7         67.0         1.0         1.032           1.027        
--------------  -----------  ----------  -----------  ----------  --------------  -------------
Legend:
- +patch 1st itr: ratio of master/+patch time for the first benchmarking iteration.
- master/+patch: ratio of master/+patch time. Higher is better for +patch. Above 1 represents a speedup.

hexapdf seems to have a lot of run-to-run variation. On a repeat run:

master: ruby 3.4.0dev (2024-12-12T17:26:06Z master c0caf1cc1a) +YJIT +PRISM [x86_64-linux]
+patch: ruby 3.4.0dev (2024-12-12T18:42:41Z +patch c639b0918b) +YJIT +PRISM [x86_64-linux]

-------  -----------  ----------  -----------  ----------  --------------  -------------
bench    master (ms)  stddev (%)  +patch (ms)  stddev (%)  +patch 1st itr  master/+patch
hexapdf  1996.2       2.7         2012.2       2.2         1.011           0.992        
-------  -----------  ----------  -----------  ----------  --------------  -------------
Legend:
- +patch 1st itr: ratio of master/+patch time for the first benchmarking iteration.
- master/+patch: ratio of master/+patch time. Higher is better for +patch. Above 1 represents a speedup.

So I want to say this patch doesn't cause any headliners to slowdown.

@matzbot matzbot requested a review from a team December 13, 2024 00:15
@XrXr XrXr force-pushed the yjit-cfunc-block-arg branch from 8ae3c4c to e1594ff Compare December 13, 2024 00:15
A good amount of call sites always pass nil as block argument, but the
nil doesn't show up in the context. Put a runtime guard for those
cases to handle it. Particular relevant for the `ruby-lsp` benchmark in
`yjit-bench`. Up to a 2% speedup across headline benchmarks.

Co-authored-by: Takashi Kokubun <takashikkbn@gmail.com>
Co-authored-by: Maxime Chevalier-Boisvert <maxime.chevalierboisvert@shopify.com>
Co-authored-by: Aaron Patterson <tenderlove@ruby-lang.org>
Co-authored-by: Kevin Menard <kevin@nirvdrum.com>
Co-authored-by: Randy Stauner <randy.stauner@shopify.com>
@XrXr XrXr force-pushed the yjit-cfunc-block-arg branch from e1594ff to cd696ba Compare December 13, 2024 00:17
@maximecb maximecb merged commit f3a1176 into ruby:master Dec 13, 2024
76 checks passed
@maximecb maximecb deleted the yjit-cfunc-block-arg branch December 13, 2024 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants