Save one VALUE per embedded RTypedData #13190
Open
+28
−25
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
byroot
approved these changes
Apr 28, 2025
byroot
reviewed
Apr 28, 2025
test/ruby/test_gc.rb
Outdated
| @@ -411,6 +411,8 @@ def test_latest_gc_info_weak_references_count | |||
| before_weak_references_count = GC.latest_gc_info(:weak_references_count) | |||
| before_retained_weak_references_count = GC.latest_gc_info(:retained_weak_references_count) | |||
|
|
|||
| # Clear ary, so if ary itself is somewhere on the stack, it won't hold all references | |||
| ary.clear | |||
There was a problem hiding this comment.
We can probably merge that fix now, Koichi reported it just this morning.
This halves the amount of memory used for embedded RTypedData if they are one VALUE (8 bytes on 64-bit platforms) over the slot size limit. For Set, on 64-bit it uses an embedded 56-byte struct. With the previous implementation, the embedded structs starts at offset 32, resulting in a total size of 88. Since that is over the 80 byte limit, it goes to the next highest bucket, 160 bytes, wasting 72 bytes. This allows it to fit in a 80 byte bucket, which reduces the total size for small sets of from 224 bytes (160 bytes embedded, 64 bytes malloc, 72 bytes wasted in embedding) to 144 bytes (80 bytes embedded, 64 bytes malloc, 0 bytes wasted in embedding). Any other embedded RTypedData will see similar advantages if they are currently one VALUE over the limit. To implement this, remove the typed_flag from struct RTypedData. Embed the typed_flag information in the type member, which is now a tagged pointer using VALUE type, using the bottom low 2 bits as flags (1 bit for typed flag, the other for the embedded flag). To get the actual pointer, RTYPEDDATA_TYPE masks out the low 2 bits and then casts. That moves the RTypedData data pointer from offset 32 to offset 24 (on 64-bit). Vast amount of code in the internals (and probably external C extensions) expects the following code to work for both RData and non-embedded RTypedData: ```c DATA_PTR(obj) = some_pointer; ``` Allow this to work by moving the data pointer in RData between the dmark and dfree pointers, so it is at the same offset (24 on 64-bit). Other than these changes to the include files, the only changes needed were to gc.c, to account for the new struct layouts, handle setting the low bits in the type member, and to use RTYPEDDATA_TYPE(obj) instead of RTYPEDDATA(obj)->type.
5b9dcf9 to
a9eefae
Compare
|
Rebased against master as the initial first commit was already merged. |
This doesn't tag the data pointer, it tags the type pointer. So a call such as this would not be affected: // ruby_type_struct is a const rb_data_type_t
TypedData_Make_Struct(klass, type, &ruby_type_struct, pointer_to_garbage);This type of call could potentially be affected: TypedData_Make_Struct(klass, type, pointer_to_garbage, valid_pointer);However, I believe such a broken call would result in the same crash, because Does that alleviate your concern, @peterzhu2118 ? @nobu Do you see any issues with this, or is it OK to merge? |
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.
This halves the amount of memory used for embedded RTypedData if they
are one VALUE (8 bytes on 64-bit platforms) over the slot size limit.
For Set, on 64-bit it uses an embedded 56-byte struct. With the
previous implementation, the embedded structs starts at offset 32,
resulting in a total size of 88. Since that is over the 80 byte
limit, it goes to the next highest bucket, 160 bytes, wasting 72
bytes. This allows it to fit in a 80 byte bucket, which reduces
the total size for small sets of from 224 bytes (160 bytes
embedded, 64 bytes malloc, 72 bytes wasted in embedding) to 144
bytes (80 bytes embedded, 64 bytes malloc, 0 bytes wasted in
embedding).
Any other embedded RTypedData will see similar advantages if they
are currently one VALUE over the limit.
To implement this, remove the typed_flag from struct RTypedData.
Embedded the typed_flag information in the type member, which is
now a tagged pointer using VALUE type, using the bottom low 2 bits
as flags (1 bit for typed flag, the other for the embedded flag).
To get the actual pointer, RTYPEDDATA_TYPE masks out
the low 2 bits and then casts. That moves the RTypedData data
pointer from offset 32 to offset 24 (on 64-bit).
Vast amount of code in the internals (and probably external C
extensions) expects the following code to work for both RData and
non-embedded RTypedData:
Allow this to work by moving the data pointer in RData between
the dmark and dfree pointers, so it is at the same offset (24
on 64-bit).
Other than these changes to the include files, the only changes
needed were to gc.c, to account for the new struct layouts,
handle setting the low bits in the type member, and to use
RTYPEDDATA_TYPE(obj) instead of RTYPEDDATA(obj)->type.
The first commit fixes a non-deterministic failure in the gc tests
that was exposed by these changes.