Skip to content

Turn rb_classext_t.fields into a T_IMEMO/class_fields #13411

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

Merged
merged 1 commit into from
Jun 12, 2025

Conversation

casperisfine
Copy link
Contributor

@casperisfine casperisfine commented May 22, 2025

This behave almost exactly as a T_OBJECT, the layout is entirely compatible.

This aims to solve two problems.

First, it solves the problem of namspaced classes having a single shape_id.
Now each namespaced classext has an object that can hold the namespace specific shape.

Second, it open the door to later make class instance variable writes atomics, hence be able to read class variables without locking the VM.
In the future, in multi-ractor mode, we can do the write on a copy of the fields_obj and then atomically swap it.

Considerations:

  • Right now the RClass shape_id is always synchronized, but with namespace we should likely mark classes that have multiple namespace with a specific shape flag.

This comment has been minimized.

@tenderlove
Copy link
Member

Just to follow up because @byroot asked me to look at the JIT side of things. I don't think this impacts Z/YJIT because we only emit special machine code if the object is a T_OBJECT, everything else just uses rb_ivar_get.

@byroot
Copy link
Member

byroot commented May 22, 2025

Ah thanks! I'll dig more into why the JITs are failing tomorrow then.

@casperisfine casperisfine force-pushed the class-fields-obj branch 10 times, most recently from 51398c3 to defa987 Compare May 23, 2025 20:42
@casperisfine casperisfine force-pushed the class-fields-obj branch 2 times, most recently from 7a42762 to d594ee0 Compare June 3, 2025 13:36
@casperisfine casperisfine force-pushed the class-fields-obj branch 2 times, most recently from ce2a8cd to a701536 Compare June 4, 2025 06:46
@casperisfine casperisfine force-pushed the class-fields-obj branch 4 times, most recently from 54f8834 to a701141 Compare June 11, 2025 07:41
@casperisfine casperisfine marked this pull request as ready for review June 11, 2025 08:08
@matzbot matzbot requested a review from a team June 11, 2025 08:08
@casperisfine casperisfine force-pushed the class-fields-obj branch 3 times, most recently from 8312c5c to 7065b2f Compare June 11, 2025 10:58
@casperisfine casperisfine changed the title Turn rb_classext_t.fields into a T_OBJECT Turn rb_classext_t.fields into a T_IMEMO/class_fields Jun 11, 2025
@casperisfine casperisfine force-pushed the class-fields-obj branch 3 times, most recently from cb8b5be to 0cc94bd Compare June 11, 2025 14:56
This behave almost exactly as a T_OBJECT, the layout is entirely
compatible.

This aims to solve two problems.

First, it solves the problem of namspaced classes having
a single `shape_id`. Now each namespaced classext
has an object that can hold the namespace specific
shape.

Second, it open the door to later make class instance variable
writes atomics, hence be able to read class variables
without locking the VM.
In the future, in multi-ractor mode, we can do the write
on a copy of the `fields_obj` and then atomically swap it.

Considerations:

  - Right now the `RClass` shape_id is always synchronized,
    but with namespace we should likely mark classes that have
    multiple namespace with a specific shape flag.
@byroot byroot merged commit 3abdd42 into ruby:master Jun 12, 2025
90 of 92 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants