Skip to content

Conversation

@pull
Copy link

@pull pull bot commented Aug 28, 2025

See Commits and Changes for more details.


Created by pull[bot] (v2.0.0-alpha.3)

Can you help keep this open source service alive? 💖 Please sponsor : )

byroot and others added 2 commits August 28, 2025 09:25
`vm_setinstancevariable` had a codepath to try to match the inline
cache for types other than T_OBJECT, but the cache population path
in `vm_setivar_slowpath` was exclusive to T_OBJECT, so `vm_setivar_default`
would never match anything.

This commit improves `vm_setivar_slowpath` so that it is capable of
filling the cache for all types, and adds a `vm_setivar_class` codepath
for `T_CLASS` and `T_MODULE`.

`vm_setivar`, `vm_setivar_default` and `vm_setivar_class` could be unified,
but based on the very explicit `NOINLINE` I assume they were split to minimize
codesize.

```
compare-ruby: ruby 3.5.0dev (2025-08-27T14:58:58Z merge-vm-setivar-d.. 5b749d8e53) +PRISM [arm64-darwin24]
built-ruby: ruby 3.5.0dev (2025-08-27T16:30:31Z setivar-cache-gene.. 4fe78ff) +PRISM [arm64-darwin24]

|                         |compare-ruby|built-ruby|
|:------------------------|-----------:|---------:|
|vm_ivar_set_on_instance  |     161.809|   164.688|
|                         |           -|     1.02x|
|vm_ivar_set_on_generic   |      58.769|   115.638|
|                         |           -|     1.97x|
|vm_ivar_set_on_class     |      70.034|   141.042|
|                         |           -|     2.01x|
```
The default non-transformed name, `ruby` target was added for the case
of `--program-transform-name` and similars, but it was occasionally
added even when no such option is used.
@pull pull bot locked and limited conversation to collaborators Aug 28, 2025
@pull pull bot added the ⤵️ pull label Aug 28, 2025
@pull pull bot merged commit 6023195 into turkdevops:master Aug 28, 2025
1 of 2 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants