Followup: https://github.com/ruby/ruby/pull/17631
Now that complex `RObject` always have an IMEMO/fields, we no
longer need to have an external `st_table` to replicate the
memory layout of `RObject`.
Instead of spilling into a raw buffer, `RObject` now spills
into an `IMEMO/fields` object like other types.
From an instance variable layout standpoint, an extended `RObject`
is now identical to a `RTypedData`, as in they both store the
reference to their `IMEMO/fields` at the same offset (`VALUE * 2`).
One positive consequence of this is that the only case where a `T_OBJECT`
needs sweeping is if a finalizer was registered.
YJIT now side exit when it need to write a new ivar into a `RObject`
that is out of space. This is unlikely to cause a performance regression
as this codepath isn't supposed to happen after warmup given `max_iv_count`
is recorded on classes, so future objects should be large enough.
Hence it's best not to waste executable memory for such codepath.
ZJIT lost support for writting into extended `RObject`.
Most of the code to support it is there it's just missing an
implementation of `RBASIC_SET_SHAPE_ID`, which recently changed
to strip some bits out of the shape (see comments).
I will leave it to the ZJIT team to implement it (sorry).
There are a few future cleanups planned that I keep for followups:
- We can now get rid of the `ROBJECT_HEAP` flag, it's redundant with
the shape layout bits.
- `imemo_fields` can now embed the `st_table` when complex.
Co-Authored-By: John Hawthorn <john@hawthorn.email>
Co-authored-by: Randy Stauner <randy@r4s6.net>
Like the capacity part, the layout part of an object shape
almost never changes. The few exceptions are:
- On allocation.
- On being compacted by GC.
- When RObject oberflows.
As such it simplifies a lot of code if `RBASIC_SET_SHAPE_ID` strips
the layout bits, as we often copy the shape from IMEMO/fields to
the owner object and vice-versa.
Also change `RBASIC_SET_SHAPE_ID_WITH_CAPACITY` into
`RBASIC_SET_FULL_SHAPE_ID` so it can be used for assigning both
capacity and layout.
pm_setup_args mallocs one kw_arg buffer with references = 0. The inline_new branch then feeds that same pointer
to three callinfo constructions:
- 3789 — new_callinfo(... method_id="new" ..., kw_arg, 0) -> the opt_new ci
- 3795 — PUSH_SEND_R(... "initialize", ..., flags | VM_CALL_FCALL, kw_arg)
- 3800 — PUSH_SEND_R(... method_id="new", ..., flags, kw_arg) (fallback)
Tracing the refcount through rb_vm_ci_lookup()
1. It increments kwarg->references and allocates a fresh new_ci before the dedup st_update
2. The dedup (vm_ci_hash_cmp) compares kwarg contents, not the pointer. So if an earlier line already interned a new ci with the
identical keyword set, st_update returns that pre-existing ci (which holds a different buffer) and discards our new_ci.
3. Our kw_arg is now orphaned: references == 1, but the only holder is the discarded new_ci, which is a normal collectable imemo
4. An allocation like PUSH_INSN2 opt_new at or new_callinfo() can trigger a GC. References back to 0, kw_arg buffer freed.
5. new_callinfo() using the freed buffer, does argc += kw_arg->keyword_len (use-after-free)
The fix: Keep the buffer alive across the allocations in inline_new.
Fixes [Bug #22104]
In the past this used an ad-hoc weakref implementation that required
checking whether the referenced object was about to be swept. We
replaced this with the actual weakref implementation from the GC so
these VALUEs should always be either live or Qundef.
In many place ALLOC_V / ALLOCV_N is used as a safer `alloca`,
and to behave like stack memory, `ALLOCV` does scan its buffer
for references using `rb_gc_mark_locations`.
The problem is that it's quite slow, and in many cases, the
temporary buffer is known not to contain any references.
e.g. `ALLOCV_N(uint32_t, buf0, len)` can't possibly contain references,
as the element size is too small.
* Reserve 2 bits for expressing object layout
We would like to make instance variable reads in the JIT compiler faster
(as well as simplify the JIT implementation). Currently, in order to
read an instance variable, we have to:
1. Test for heap object
2. Load object to a 64 bit register
3. Mask the object header
4. Bit test against the masked header
5. JNE
6. Load field
We would like to:
1. Test for heap object
2. Load object shape to a 32 bit register
3. Bit test against the shape
4. JNE
5. Load field
The way we fetch instance variables is not consistent across objects.
In order to realize our goal, we need to encode object layout inside the
shape. If we encode object layout inside the shape, then the shape
itself will guarantee that the access pattern generated by the JIT
compiler is correct.
We should encode the following load patterns into the shape tag bits.
This way we can share shapes on transitions, but be able to
differentiate the access patterns for the JIT compiler. In other words,
two objects can have an `@a -> @b -> @c` transition and share the same
shape, but the tag bits can differentiate the access pattern so that the
JIT compiler can be confident that the machine code is correct.
Here are the patterns:
1. Embedded/Extended T_OBJECT Instance Variables
Objects with direct references to instance variables or via malloc
buffer
2. Objects with fields_objects fields
These are Data and TypedData objects. They have an associated axillary
imemo/fields object that stores the instance variables. The access
pattern is `object[2] + 2`. The fields object is the 3rd field, and the
instance variables start at +2 inside the fields object. The fields
object itself is a Ruby object, so it contains the usual header bits +
class headers.
3. Non Boxable Classes / Modules
This is similar to Objects with fields_objects, but the fields object is
stored at a different offset. We’re differentiating this from boxable
classes and modules because those are harder to support.
4. Other
"Other" pattern is for objects that are rare, or have
difficult-to-implement access patterns. This includes:
* Boxable classes and modules
* Structs (for now)
* Objects that use the geniv table
Proposed shape bit layout:
```
Current shape_id_t is 32 bits:
31 28 27 26 25 24 23 22 19 18 0
+-----------+--+--+--+--+--+------------+----------------------------+
| unused |L1|L0|OI|FR|CX| heap index | shape tree offset |
+-----------+--+--+--+--+--+------------+----------------------------+
| | | | | | |
| | | | | | +-- bits 0-18: SHAPE_ID_OFFSET_MASK
| | | | | +--------------- bits 19-22: SHAPE_ID_HEAP_INDEX_MASK
| | | | +------------------ bit 23: SHAPE_ID_FL_COMPLEX
| | | +--------------------- bit 24: SHAPE_ID_FL_FROZEN
| | +------------------------ bit 25: SHAPE_ID_FL_HAS_OBJECT_ID
+--+--------------------------- bits 26-27: SHAPE_ID_LAYOUT_MASK
```
The important part about these layout patterns is that they do not
reflect the _type_ of object, only how the object is laid out in memory.
For example, we currently treat structs as "other", but we can refactor
them to have the same layout as "Objects with fields_objects", and when
we do that they should get a different bit in the shape header.
This commit only reserves the two bits, it doesn't use them in the JIT
compiler yet.
Co-Authored-By: John Hawthorn <john@hawthorn.email>
Co-Authored-By: Max Bernstein <tekknolagi@gmail.com>
* Update gc.c
Co-authored-by: Nobuyoshi Nakada <nobu.nakada@gmail.com>
* Update shape.h
Co-authored-by: Jean Boussier <jean.boussier@gmail.com>
* fix function name
* Update shape.c
Co-authored-by: Jean Boussier <jean.boussier@gmail.com>
* fix function name
* Revert "Update shape.c"
This reverts commit 900711defc6c541a93f3393a350819ae88cf87f1.
* add comment
---------
Co-authored-by: John Hawthorn <john@hawthorn.email>
Co-authored-by: Max Bernstein <tekknolagi@gmail.com>
Co-authored-by: Nobuyoshi Nakada <nobu.nakada@gmail.com>
Co-authored-by: Jean Boussier <jean.boussier@gmail.com>
It's possible for st_init_existing_table_with_size to trigger GC. If
that happens we need to ensure that the table entries count doesn't
contain garbage data, or we'll try and mark random stuff
RHash isn't a good fit for storing `cdhash` as this force to allow
arbitrary hash types into RHash, which doesn't work with AR tables.
It also cause the cdhash to be larger than needed.
Now that we have 1024B slots, we can store up to 126 fields inline.
Objects larger than this are rare if not non-existent, hence we can
get rid of the `malloc` path for imemo/fields and simply transition
to `TOO_COMPLEX`.
This additionally allows to shrink `attr_index_t` from 16 to 8B.
Note: the ZJIT "ivar on extended" tests are renamed as "complex" because
"extended" AKA malloc allocated imemo/fields no longer exists.
They're now complex fields, AKA st tables.
rb_class_allocate_instance: start as complex when over max_fields
If `RCLASS_MAX_IV_COUNT` is over `max_fields`, allocating a large
slot to end up transitioning to `TOO_COMPLEX` is wasteful.
We might as well start as complex directly.
We always know when this contains a VALUE and even fire a write barrier
for it. We should use this when marking, support compaction, and no
longer need conservative marking.
Almost every objects are now WB protected, with just a few
exceptions, hence we can provide a much simpler interface.
It's also much easier to find the remaining unprotected objects.
[Bug #21952]
Solves the double-free or use after-free concern with boxes.
Now entries can safely be used for copy-on-write.
Also is likely necessary to make it save to read cvar from
secondary ractors, as allowed since: ab32c0e690b805cdaaf264ad4c3421696c588204
Usually RB_ALLOCV_N uses alloca for small allocations, and in that case
the value is 0, and we should not need to atomic exchange it back to 0.
I'm actually not sure why we need atomic operations here anyways.
There's no reason to ever call this and the "count" value is ignored.
Because this is part of the public API I left it as a stub, but it could
likely be removed.
For now the provided size is just for GC statistics, but in the future
we may want to forward it to C23's `free_sized` and passing an incorrect
size to it is undefined behavior.
Before this change, GC'ing any Ractor object caused you to lose all
enabled tracepoints across all ractors (even main). Now tracepoints are
ractor-local and this doesn't happen. Internal events are still global.
Fixes [Bug #19112]
to adopt strict shareable rule.
* (basically) shareable objects only refer shareable objects
* (exception) shareable objects can refere unshareable objects
but should not leak reference to unshareable objects to Ruby world
When we clone a complex imemo_fields, it calls creates the imemo_fields
using rb_imemo_fields_new_complex, which allocates and initializes a new
st_table. However, st_replace will directly replace any exisiting fields
in the st_table, causing it to leak.
For example, this script demonstrates the leak:
obj = Class.new
8.times do |i|
obj.instance_variable_set(:"@test#{i}", nil)
obj.remove_instance_variable(:"@test#{i}")
end
obj.instance_variable_set(:"@test", 1)
10.times do
100_000.times do
obj.dup
end
puts `ps -o rss= -p #{$$}`
end
Before:
26320
39296
52320
63136
75520
87008
97856
114800
120864
133504
After:
16288
20112
20416
20720
20800
20864
21184
21424
21904
21904
The imemo_fields_new function takes a capacity in the number of fields to
preallocate. rb_imemo_fields_new_complex_tbl is using it incorrectly because
it is preallocating sizeof(struct rb_fields) number of fields.
We should not assume that a complex imemo_field takes only one additional
VALUE space. This is fragile as it will break if we add additional fields
to complex imemo_field.
imemo_tmpbuf is not write-barrier protected and uses mark maybe to mark
the buffer it holds. The normal rb_imemo_new creates a write-barrier
protected object which can make the tmpbuf miss marking references.
