This should never be true. I added an `rb_bug` in case it was and it
wasn't true in any of btest or test-all.
Co-authored-by: Aaron Patterson <tenderlove@ruby-lang.org>
[Bug #21710]
- struct.c: `struct_alloc`
It is possible for a `NEWOBJ` tracepoint call back to write fields
into a newly allocated object before `struct_alloc` had the time
to set the `RSTRUCT_GEN_FIELDS` flags and such.
Hence we can't blindly initialize the `fields_obj` reference to `0`
we first need to check no fields were added yet.
- object.c: `rb_class_allocate_instance`
Similarly, if a `NEWOBJ` tracepoint tries to set fields on the object,
the `shape_id` must already be set, as it's required on T_OBJECT to
know where to write fields.
`NEWOBJ_OF` had to be refactored to accept a `shape_id`.
When we dup a complex object we need to issue a writebarrier_remember
on the new object. This was caught by wbcheck inside the rubygems test
suite.
Co-authored-by: Aaron Patterson <tenderlove@ruby-lang.org>
It is much more convenient than storing the klass, especially
when dealing with `object_id` as it allows to update the id2ref
table without having to dereference the owner, which may be
garbage at that point.
The `p->field = rb_gc_location(p->field)` isn't ideal because it means all
references are rewritten on compaction, regardless of whether the referenced
object has moved. This isn't good for caches nor for Copy-on-Write.
`rb_gc_mark_and_move` avoid needless writes, and most of the time allow to
have a single function for both marking and updating references.
If `get_next_shape_internal` fail to return a shape, we must
transitiont to a complex shape. `shape_transition_object_id`
mistakenly didn't.
Co-Authored-By: Peter Zhu <peter@peterzhu.ca>
Previously it was possible for two atomic increments of next_shape_id
running concurrently to overflow MAX_SHAPE_ID. For this reason we need
to do the test atomically with the allocation in shape_alloc returning
NULL.
This avoids overflowing next_shape_id by repeatedly attempting a CAS.
Alternatively we could have allowed incrementing past MAX_SHAPE_ID and
then clamping in rb_shapes_count, but this seems simpler.
When sharing between threads we need both atomic reads and writes. We
probably didn't need to use this in some cases (where we weren't running
in multi-ractor mode) but I think it's best to be consistent.
This was using < so subtract one from the last shape id would have us
miss the last inserted shape. I think this is unlikely to have caused
issues because I don't think the newest shape will ever have edges.
We do need to use `- 1` because otherwise RSHAPE wraps around and
returns the root shape.
The `FL_FREEZE` flag is redundant with `SHAPE_ID_FL_FROZEN`, so
ideally it should be eliminated in favor of the later.
Doing so would eliminate the risk of desync between the two, but
also solve the problem of the frozen status being global in namespace
context (See Bug #21330).
The FL_EXIVAR is a bit redundant with the shape_id.
Now that the `shape_id` is embedded in all objects on all archs,
we can cheaply check if an object has any fields with a simple
bitmask.
This behave almost exactly as a T_OBJECT, the layout is entirely
compatible.
This aims to solve two problems.
First, it solves the problem of namspaced classes having
a single `shape_id`. Now each namespaced classext
has an object that can hold the namespace specific
shape.
Second, it open the door to later make class instance variable
writes atomics, hence be able to read class variables
without locking the VM.
In the future, in multi-ractor mode, we can do the write
on a copy of the `fields_obj` and then atomically swap it.
Considerations:
- Right now the `RClass` shape_id is always synchronized,
but with namespace we should likely mark classes that have
multiple namespace with a specific shape flag.
The type isn't opaque because Ruby isn't often compiled with LTO,
so for optimization purpose it's better to allow as much inlining
as possible.
However ideally only `shape.c` and `shape.h` should deal with
the actual struct, and everything else should just deal with opaque
`shape_id_t`.
Now that `rb_shape_traverse_from_new_root` has been eliminated there's
no longer any reason to pin these objects, because we no longer
need to traverse shapes downward during compaction.
Now that there no longer multiple shape roots, all we need to do
when moving an object from one slot to the other is to update the
`heap_index` part of the shape_id.
Since this never need to create a shape transition, it will always
work and never result in a complex shape.
This used to be protected because all shape code was
under a lock, but now that the shape tree is lock-free
we still need to lock around the red-black cache.
Co-Authored-By: Luke Gruber <luke.gruber@shopify.com>
