[pull] master from ruby:master - #1348
Merged
Merged
Conversation
ruby/prism@9df2a21 changed the comment style Now rdoc picks this up. The comment doesn't say much, seems fine to just remove ruby/prism@f703c615ae
With gcc 15.2.0 on aarch64 Ubuntu 26.4:
```console
hash.c: In function ‘rb_hash_s_create’:
hash.c:1982:20: warning: ‘hash’ may be used uninitialized [-Wmaybe-uninitialized]
1982 | return hash;
| ^~~~
hash.c:1920:11: note: ‘hash’ was declared here
1920 | VALUE hash, tmp;
| ^~~~
```
With gcc 15.2.0 on aarch64 Ubuntu 26.4:
```console
io_buffer.c: In function ‘io_buffer_flags_for_new’:
io_buffer.c:978:20: warning: comparison between ‘enum rb_io_buffer_flags’ and ‘enum <anonymous>’ [-Wenum-compare]
978 | if (allocation == RB_IO_BUFFER_ALLOCATION_FLAGS) {
| ^~
```
With gcc 15.2.0 on aarch64 Ubuntu 26.4:
```console
ractor_sync.c: In function ‘ractor_basket_new.constprop’:
ractor_sync.c:1093:82: warning: argument ‘type’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Wclobbered]
1093 | ractor_basket_new(rb_execution_context_t *ec, VALUE obj, enum ractor_basket_type type, bool exc)
| ~~~~~~~~~~~~~~~~~~~~~~~~^~~~
```
Prevent a tail call from discarding saved machine-register roots before `rb_gc_mark_roots` scans the machine stack.
For now it still returns a constant, but clarify when we're looking for the max bound, versus a miss, etc.
Instead of always allocating `ar_table` backed hashes in 160B
slots for them to have capacity of `RHASH_AR_TABLE_MAX_SIZE` (`8`),
they can now be allocated in any slot size between `64B` and `160B`.
`64B` is the minimum slot size that still allows transitioning to
an `st_table` if necessary.
The assumption is that there is a large number of literal hashes
and keyword argument hashes being built, and the overwhelming majority
of them aren't appended to.
When this assumption hold, this patch saves memory and reduce GC
pressure by allocating in a smaller slot than before.
However when the assumption doesn't hold, it has the oppositve effect
as the Hash will transition to an `st_table` when before it may have
stayed an `ar_table`.
Currently the main blocked for this change is that the compiler
has a tendency to generate code that make the final Hash size unknown.
For instance: `{a: 4, b: 4 + 1}` is compiled as:
```
0000 duphash {a: 4} ( 1)[Li]
0002 putspecialobject 1
0004 swap
0005 putobject :b
0007 putobject 4
0009 putobject_INT2FIX_1_
0010 opt_plus <calldata!mid:+, argc:1, ARGS_SIMPLE>[CcCr]
0012 opt_send_without_block <calldata!mid:core#hash_merge_ptr, argc:3, ARGS_SIMPLE>
```
Here `duphash` has no information about how large of a hash it needs to allocate.
For this patch to be more efficient the compiler should generate something like:
```
0000 putobject {a: 4} ( 1)[Li]
0002 putspecialobject 1
0004 swap
0005 putobject :b
0007 putobject 4
0009 putobject_INT2FIX_1_
0010 opt_plus <calldata!mid:+, argc:1, ARGS_SIMPLE>[CcCr]
0012 opt_send_without_block <calldata!mid:core#hash_merge_ptr, argc:3, ARGS_SIMPLE>
```
So that it's `hash_merge_ptr` that is responsible for allocating the final hash,
given it does have a much better clue of how large it would need to be.
Demo:
```ruby
require 'objspace'
def size(hash)
puts "size: #{hash.size}\tmemsize: #{ObjectSpace.memsize_of(hash)}"
end
size({})
size(a:1)
size(a:1, b:2)
size(a:1, b:2, c:3)
size(a:1, b:2, c:3, d:4)
size(a:1, b:2, c:3, d:4, e:5)
size(a:1, b:2, c:3, d:4, e:5, f:6)
size(a:1, b:2, c:3, d:4, e:5, f:6, g:7)
size(a:1, b:2, c:3, d:4, e:5, f:6, g:7, h:8)
size(a:1, b:2, c:3, d:4, e:5, f:6, g:7, h:8, i:9)
```
Before:
```
size: 0 memsize: 160
size: 1 memsize: 160
size: 2 memsize: 160
size: 3 memsize: 160
size: 4 memsize: 160
size: 5 memsize: 160
size: 6 memsize: 160
size: 7 memsize: 160
size: 8 memsize: 160
size: 9 memsize: 464
```
After:
```
size: 0 memsize: 64
size: 1 memsize: 64
size: 2 memsize: 64
size: 3 memsize: 80
size: 4 memsize: 96
size: 5 memsize: 128
size: 6 memsize: 128
size: 7 memsize: 160
size: 8 memsize: 160
size: 9 memsize: 448
```
This is designed to replace rb_ident_hash_new calls when such hashes are used purely with with true values. To implement this, add a set_alloc_with_size_and_type static function, and have set_alloc_with_size and rb_ident_set_new call it.
This used a hash with all true values, with is simple to convert to a set to save on memory when flattening array with a large number of nested elements.
Keep the payload builder uninlined in optimized GCC builds because inlining causes false-positive `-Wclobbered` warnings.
`hash_copy` will call `st_init_existing_table_with_size` which allocates and may trigger GC causing the current hash to be marked. So we need to zero-out the `st_table` to avoid a segfault.
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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )