Move object_id in attributes
#13159
Open
Move object_id in attributes
#13159
+706
−460
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comment has been minimized.
This comment has been minimized.
1aae814 to
d8c231b
Compare
casperisfine
commented
Apr 24, 2025
gc/default/default.c
Outdated
| } | ||
|
|
||
| if (rb_shape_has_object_id(shape)) { | ||
| // We could avoid locking if the object isn't shareable |
There was a problem hiding this comment.
For T_MODULE and T_CLASS it might be worth finding 8B in rb_classext_t so we can do some atomic CAS and find_or_create the object_id with no actual locking.
But if we need to also set the FL_SEEN_OBJ_ID flag, it might be more tricky than that?
f1b501e to
63e2d8d
Compare
casperisfine
commented
Apr 29, 2025
| void | ||
| rb_gc_obj_free_vm_weak_references(VALUE obj) | ||
| { | ||
| if (FL_TEST_RAW(obj, FL_SEEN_OBJ_ID)) { |
There was a problem hiding this comment.
FL_SEEN_OBJ_ID isn't very useful anymore, except for SHAPE_TOO_COMPLEX, but we could improve that too.
If we always check the shape flags, we could reclaim that flag.
Also more broadly, shape_id_t is 32bit on 64bit systems, which is way more than we need. If needing to lookup the shape to know if it has an object_id (or is frozen) we could actually pack these tags in the most significant bits of the shape_id.
7c3a66e to
d4219f3
Compare
This opens the door to store more informations in shapes, such as the `object_id` or object address in case it has been observed and the object has to be moved.
And get rid of the `obj_to_id_tbl` It's no longer needed, the `object_id` is now stored inline in the object. We still need the inverse table in case `_id2ref` is invoked, but we lazily build it by walking the heap if that happens.
We can't do unsychronized reads for shareable objects, which include all modules and classes.
Since we assume most programs don't ever build the id2obj table, we can just implement it as a simple TypedData object.
Co-Authored-By: Matt Valentine-House <matt@eightbitraptor.com>
Co-Authored-By: Matt Valentine-House <matt@eightbitraptor.com>
d4219f3 to
69b378b
Compare
Ivars no longer are the only thing stored inline via shapes, so keeping `iv_index` would be confusing.
Right now everything assume only one shape is too_complex. In the near future I'd like to have more than one complex shape, e.g. frozen and too_complex.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
And get rid of the
obj_to_id_tblIt's no longer needed, the
object_idis now stored inlinein the object.
We still need the inverse table in case
_id2refis invoked, butwe lazily build it by walking the heap if that happens.