Skip to content

compile.c: deduplicate inline constant caches #13060

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

casperisfine
Copy link
Contributor

If a method reference the same constant twice or more, it can use a single constant cache for all the instructions.

puts RubyVM::InstructionSequence.compile(<<~RUBY).disasm
  Foo::Bar + Foo::Bar + Bar + Bar
RUBY

Before

== disasm: #<ISeq:<compiled>@<compiled>:1 (1,0)-(1,31)>
0000 opt_getconstant_path                   <ic:0 Foo::Bar>           (   1)[Li]
0002 opt_getconstant_path                   <ic:1 Foo::Bar>
0004 opt_plus                               <calldata!mid:+, argc:1, ARGS_SIMPLE>[CcCr]
0006 opt_getconstant_path                   <ic:2 Bar>
0008 opt_plus                               <calldata!mid:+, argc:1, ARGS_SIMPLE>[CcCr]
0010 opt_getconstant_path                   <ic:3 Bar>
0012 opt_plus                               <calldata!mid:+, argc:1, ARGS_SIMPLE>[CcCr]
0014 leave

After:

== disasm: #<ISeq:<compiled>@<compiled>:1 (1,0)-(1,31)>
0000 opt_getconstant_path                   <ic:0 Foo::Bar>           (   1)[Li]
0002 opt_getconstant_path                   <ic:0 Foo::Bar>
0004 opt_plus                               <calldata!mid:+, argc:1, ARGS_SIMPLE>[CcCr]
0006 opt_getconstant_path                   <ic:1 Bar>
0008 opt_plus                               <calldata!mid:+, argc:1, ARGS_SIMPLE>[CcCr]
0010 opt_getconstant_path                   <ic:1 Bar>
0012 opt_plus                               <calldata!mid:+, argc:1, ARGS_SIMPLE>[CcCr]
0014 leave

Copy link

launchable-app bot commented Apr 3, 2025

All Tests passed!

✖️no tests failed ✔️61916 tests passed(1 flake)

@casperisfine casperisfine force-pushed the shared-icc branch 2 times, most recently from 00fc686 to c8d25a1 Compare April 4, 2025 14:27
byroot and others added 3 commits April 7, 2025 10:19
If a method reference the same constant twice or more, it can
use a single constant cache for all the instructions.

```ruby
puts RubyVM::InstructionSequence.compile(<<~RUBY).disasm
  Foo::Bar + Foo::Bar + Bar + Bar
RUBY
```

Before

```
== disasm: #<ISeq:<compiled>@<compiled>:1 (1,0)-(1,31)>
0000 opt_getconstant_path                   <ic:0 Foo::Bar>           (   1)[Li]
0002 opt_getconstant_path                   <ic:1 Foo::Bar>
0004 opt_plus                               <calldata!mid:+, argc:1, ARGS_SIMPLE>[CcCr]
0006 opt_getconstant_path                   <ic:2 Bar>
0008 opt_plus                               <calldata!mid:+, argc:1, ARGS_SIMPLE>[CcCr]
0010 opt_getconstant_path                   <ic:3 Bar>
0012 opt_plus                               <calldata!mid:+, argc:1, ARGS_SIMPLE>[CcCr]
0014 leave
```

After:

```
== disasm: #<ISeq:<compiled>@<compiled>:1 (1,0)-(1,31)>
0000 opt_getconstant_path                   <ic:0 Foo::Bar>           (   1)[Li]
0002 opt_getconstant_path                   <ic:0 Foo::Bar>
0004 opt_plus                               <calldata!mid:+, argc:1, ARGS_SIMPLE>[CcCr]
0006 opt_getconstant_path                   <ic:1 Bar>
0008 opt_plus                               <calldata!mid:+, argc:1, ARGS_SIMPLE>[CcCr]
0010 opt_getconstant_path                   <ic:1 Bar>
0012 opt_plus                               <calldata!mid:+, argc:1, ARGS_SIMPLE>[CcCr]
0014 leave
```
@casperisfine
Copy link
Contributor Author

casperisfine commented Apr 7, 2025

Tested on Shopify's monolith.

Before:

718.34 MB  <iseq> (IMEMO)

After:

 714.05 MB  <iseq> (IMEMO)

So the gain is relatively modest (0.6%). But if we were to do the same for other types of caches, it might add up.

I also tried to graph these with a scatter plot.
Capture d’écran 2025-04-07 à 13 06 10

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