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: Initialize Assembler vectors with capacity #8437

Merged
merged 1 commit into from Sep 14, 2023

Conversation

k0kubun
Copy link
Member

@k0kubun k0kubun commented Sep 14, 2023

Looking at perf profiling results on 30k_ifelse, I noticed x86_split and alloc_regs spend a considerable amount of time in push_insn, which spends majority of time in Vec::push.

To make these Vec::push calls faster, this PR sets initial capacity to the vectors modified by push_insn.

Size of asm.insns

I checked the max asm.insns.len() in the following benchmarks:

benchmark max asm.insns.len()
lobsters 131
railsbench 131
ruby-lsp 190
30k_ifelse 65
30k_methods 65

To accommodate Insns for these benchmarks, I chose 256. size_of::<Insn>() is 80, so each vector has 80 * 256 = 20KiB. Having up to four of such vectors at a time would consume 80KiB extra memory, but it doesn't seem significant compared to what the interpreter uses, and these vectors are transient anyway.

Benchmarks: 1st itr

This speeds up the 1st itr of 30k_ifelse by 7%. It also speeds up lobsters, ruby-lsp, and 30k_methods. It doesn't seem to have a significant impact on RSS.

-----------  -----------  ----------  ---------  ----------  ----------  ---------  -------------  ------------
bench        before (ms)  stddev (%)  RSS (MiB)  after (ms)  stddev (%)  RSS (MiB)  after 1st itr  before/after
lobsters     1766.1       0.0         314.0      1659.5      0.0         317.1      1.06           1.06
railsbench   1512.9       0.0         103.5      1515.6      0.0         102.1      1.00           1.00
ruby-lsp     1526.7       0.0         106.8      1338.5      0.0         106.9      1.14           1.14
30k_ifelse   1378.1       0.0         98.9       1285.5      0.0         98.9       1.07           1.07
30k_methods  920.4        0.0         65.0       885.0       0.0         65.1       1.04           1.04
-----------  -----------  ----------  ---------  ----------  ----------  ---------  -------------  ------------

@matzbot matzbot requested a review from a team September 14, 2023 00:12
Copy link
Contributor

@maximecb maximecb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well done!

@maximecb maximecb merged commit cdc69da into ruby:master Sep 14, 2023
95 checks passed
@k0kubun k0kubun deleted the yjit-vector-cap branch September 14, 2023 15:44
k0kubun added a commit to Shopify/ruby that referenced this pull request Sep 21, 2023
k0kubun added a commit to Shopify/ruby that referenced this pull request Sep 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants