Skip to content

Get rid of gen_fields_tbl.fields_count #13561

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 9, 2025
Merged

Get rid of gen_fields_tbl.fields_count #13561

merged 1 commit into from
Jun 9, 2025

Conversation

byroot
Copy link
Member

@byroot byroot commented Jun 9, 2025

This data is redundant because the shape already contains both the length and capacity of the object's fields.

So it both waste space and create the possibility of a desync between the two.

We also do not need to initialize everything to Qundef, this seem to be a left-over from pre-shape instance variables.

I'm also semi hoping this may solve the GC crash we've been seeing for the last 3 days:

ruby(sigsegv+0x54) [0xaaab7d46ae6c] /home/opc/ruby/src/master/signal.c:934
ruby(RVALUE_WB_UNPROTECTED+0x8) [0xaaab7d373430] /home/opc/ruby/src/master/gc/default/default.c:1195
ruby(rgengc_check_relation) /home/opc/ruby/src/master/gc/default/default.c:4300
ruby(gc_mark) /home/opc/ruby/src/master/gc/default/default.c:4369
ruby(rb_mark_generic_ivar+0x98) [0xaaab7d4de838] /home/opc/ruby/src/master/variable.c:1257
ruby(rb_gc_mark_children+0x38c) [0xaaab7d374074] /home/opc/ruby/src/master/gc.c:3179
ruby(gc_mark_stacked_objects+0x80) [0xaaab7d374360] /home/opc/ruby/src/master/gc/default/default.c:4563
ruby(gc_mark_stacked_objects_all+0xc) [0xaaab7d377408] /home/opc/ruby/src/master/gc/default/default.c:4622
ruby(gc_marks_rest) /home/opc/ruby/src/master/gc/default/default.c:5639

I never managed to reproduce it locally, but from the crash report we're marking garbage from a gen_fields_tbl, so that may be caused by a desync between the two.

@byroot
Get rid of gen_fields_tbl.fields_count
86ce50c 
This data is redundant because the shape already contains both the
length and capacity of the object's fields.

So it both waste space and create the possibility of a desync between
the two.

We also do not need to initialize everything to Qundef, this seem
to be a left-over from pre-shape instance variables.
@byroot byroot requested a review from peterzhu2118 June 9, 2025 08:42
@byroot byroot merged commit f9966b9 into ruby:master Jun 9, 2025
83 checks passed
@byroot byroot deleted the generic-ivar-no-fields-count branch June 9, 2025 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@peterzhu2118 peterzhu2118 peterzhu2118 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@byroot @peterzhu2118