1berry/ruby
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Avoid pinning storage_head in iseq_mark_and_move (#12880)

Browse Source 
* Avoid pinning `storage_head` in `iseq_mark_and_move`

This refactor changes the behavior of `iseq_mark_and_move` to avoid
pinning the `storage_head`. Previously pinning was required because they
could be gc'd during `iseq_set_sequence` it would be possible to end
up with a half build array of instructions. However, in order to
implement a moving immix algorithm we can't pin these objects so this
rafactoring changes the code to mark and move. To accomplish this, it was
required to add `iseq_size`, `iseq_encoded`, and the `mark_bits` union
to the `iseq_compile_data` struct. In addition `iseq_compile_data` sets
a bool for whether there is a single or list of mark bits. While this
change is needed for moving immix, it should be better for Ruby's GC
as well.

* Don't allocate mark_offset_bits for one word

If only one word is needed, we don't need to allocate mark_offset_bits
and can instead directly write to it.

---------

Co-authored-by: Peter Zhu <peter@peterzhu.ca>
This commit is contained in:
Eileen M. Uchitelle 2025-03-17 10:42:48 -04:00 committed by GitHub
parent 8d6f153fba
commit c85dffeee2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
Notes: git 2025-03-17 14:43:06 +00:00 
Merged-By: eileencodes <eileencodes@gmail.com>
3 changed files with 60 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
compile.c
View File

@ -1375,7 +1375,7 @@ new_adjust_body(rb_iseq_t *iseq, LABEL *label, int line)
}
static void
iseq_insn_each_markable_object(INSN *insn, void (*func)(VALUE, VALUE), VALUE data)
iseq_insn_each_markable_object(INSN *insn, void (*func)(VALUE *, VALUE), VALUE data)
{
const char *types = insn_op_types(insn->insn_id);
for (int j = 0; types[j]; j++) {
@ -1386,7 +1386,7 @@ iseq_insn_each_markable_object(INSN *insn, void (*func)(VALUE, VALUE), VALUE dat
case TS_VALUE:
case TS_IC: // constant path array
case TS_CALLDATA: // ci is stored.
func(OPERAND_AT(insn, j), data);
func(&OPERAND_AT(insn, j), data);
break;
default:
break;
@ -1395,9 +1395,9 @@ iseq_insn_each_markable_object(INSN *insn, void (*func)(VALUE, VALUE), VALUE dat
}
static void
iseq_insn_each_object_write_barrier(VALUE obj, VALUE iseq)
iseq_insn_each_object_write_barrier(VALUE * obj, VALUE iseq)
{
RB_OBJ_WRITTEN(iseq, Qundef, obj);
RB_OBJ_WRITTEN(iseq, Qundef, *obj);
}
static INSN *
@ -2607,16 +2607,21 @@ iseq_set_sequence(rb_iseq_t *iseq, LINK_ANCHOR *const anchor)
iseq_bits_t * mark_offset_bits;
int code_size = code_index;
iseq_bits_t tmp[1] = {0};
bool needs_bitmap = false;
if (ISEQ_MBITS_BUFLEN(code_index) == 1) {
mark_offset_bits = tmp;
mark_offset_bits = &ISEQ_COMPILE_DATA(iseq)->mark_bits.single;
ISEQ_COMPILE_DATA(iseq)->is_single_mark_bit = true;
}
else {
mark_offset_bits = ZALLOC_N(iseq_bits_t, ISEQ_MBITS_BUFLEN(code_index));
ISEQ_COMPILE_DATA(iseq)->mark_bits.list = mark_offset_bits;
ISEQ_COMPILE_DATA(iseq)->is_single_mark_bit = false;
}
ISEQ_COMPILE_DATA(iseq)->iseq_encoded = (void *)generated_iseq;
ISEQ_COMPILE_DATA(iseq)->iseq_size = code_index;
list = FIRST_ELEMENT(anchor);
insns_info_index = code_index = sp = 0;
@ -2827,15 +2832,16 @@ iseq_set_sequence(rb_iseq_t *iseq, LINK_ANCHOR *const anchor)
body->iseq_size = code_index;
body->stack_max = stack_max;
if (ISEQ_MBITS_BUFLEN(body->iseq_size) == 1) {
body->mark_bits.single = mark_offset_bits[0];
if (ISEQ_COMPILE_DATA(iseq)->is_single_mark_bit) {
body->mark_bits.single = ISEQ_COMPILE_DATA(iseq)->mark_bits.single;
}
else {
if (needs_bitmap) {
body->mark_bits.list = mark_offset_bits;
}
else {
body->mark_bits.list = 0;
body->mark_bits.list = NULL;
ISEQ_COMPILE_DATA(iseq)->mark_bits.list = NULL;
ruby_xfree(mark_offset_bits);
}
}
@ -12128,13 +12134,13 @@ iseq_build_kw(rb_iseq_t *iseq, VALUE params, VALUE keywords)
}
static void
iseq_insn_each_object_mark_and_pin(VALUE obj, VALUE _)
iseq_insn_each_object_mark_and_move(VALUE * obj, VALUE _)
{
rb_gc_mark(obj);
rb_gc_mark_and_move(obj);
}
void
rb_iseq_mark_and_pin_insn_storage(struct iseq_compile_data_storage *storage)
rb_iseq_mark_and_move_insn_storage(struct iseq_compile_data_storage *storage)
{
INSN *iobj = 0;
size_t size = sizeof(INSN);
@ -12159,7 +12165,7 @@ rb_iseq_mark_and_pin_insn_storage(struct iseq_compile_data_storage *storage)
iobj = (INSN *)&storage->buff[pos];
if (iobj->operands) {
iseq_insn_each_markable_object(iobj, iseq_insn_each_object_mark_and_pin, (VALUE)0);
iseq_insn_each_markable_object(iobj, iseq_insn_each_object_mark_and_move, (VALUE)0);
}
pos += (int)size;
}

45
iseq.c
View File

@ -232,7 +232,30 @@ iseq_scan_bits(unsigned int page, iseq_bits_t bits, VALUE *code, VALUE *original
}
static void
rb_iseq_mark_and_move_each_value(const rb_iseq_t *iseq, VALUE *original_iseq)
rb_iseq_mark_and_move_each_compile_data_value(const rb_iseq_t *iseq, VALUE *original_iseq)
{
unsigned int size;
VALUE *code;
const struct iseq_compile_data *const compile_data = ISEQ_COMPILE_DATA(iseq);
size = compile_data->iseq_size;
code = compile_data->iseq_encoded;
// Embedded VALUEs
if (compile_data->mark_bits.list) {
if(compile_data->is_single_mark_bit) {
iseq_scan_bits(0, compile_data->mark_bits.single, code, original_iseq);
}
else {
for (unsigned int i = 0; i < ISEQ_MBITS_BUFLEN(size); i++) {
iseq_bits_t bits = compile_data->mark_bits.list[i];
iseq_scan_bits(i, bits, code, original_iseq);
}
}
}
}
static void
rb_iseq_mark_and_move_each_body_value(const rb_iseq_t *iseq, VALUE *original_iseq)
{
unsigned int size;
VALUE *code;
@ -280,11 +303,9 @@ rb_iseq_mark_and_move_each_value(const rb_iseq_t *iseq, VALUE *original_iseq)
iseq_scan_bits(0, body->mark_bits.single, code, original_iseq);
}
else {
if (body->mark_bits.list) {
for (unsigned int i = 0; i < ISEQ_MBITS_BUFLEN(size); i++) {
iseq_bits_t bits = body->mark_bits.list[i];
iseq_scan_bits(i, bits, code, original_iseq);
}
for (unsigned int i = 0; i < ISEQ_MBITS_BUFLEN(size); i++) {
iseq_bits_t bits = body->mark_bits.list[i];
iseq_scan_bits(i, bits, code, original_iseq);
}
}
}
@ -327,7 +348,7 @@ rb_iseq_mark_and_move(rb_iseq_t *iseq, bool reference_updating)
if (ISEQ_BODY(iseq)) {
struct rb_iseq_constant_body *body = ISEQ_BODY(iseq);
rb_iseq_mark_and_move_each_value(iseq, reference_updating ? ISEQ_ORIGINAL_ISEQ(iseq) : NULL);
rb_iseq_mark_and_move_each_body_value(iseq, reference_updating ? ISEQ_ORIGINAL_ISEQ(iseq) : NULL);
rb_gc_mark_and_move(&body->variable.coverage);
rb_gc_mark_and_move(&body->variable.pc2branchindex);
@ -394,14 +415,8 @@ rb_iseq_mark_and_move(rb_iseq_t *iseq, bool reference_updating)
else if (FL_TEST_RAW((VALUE)iseq, ISEQ_USE_COMPILE_DATA)) {
const struct iseq_compile_data *const compile_data = ISEQ_COMPILE_DATA(iseq);
if (!reference_updating) {
/* The operands in each instruction needs to be pinned because
* if auto-compaction runs in iseq_set_sequence, then the objects
* could exist on the generated_iseq buffer, which would not be
* reference updated which can lead to T_MOVED (and subsequently
* T_NONE) objects on the iseq. */
rb_iseq_mark_and_pin_insn_storage(compile_data->insn.storage_head);
}
rb_iseq_mark_and_move_insn_storage(compile_data->insn.storage_head);
rb_iseq_mark_and_move_each_compile_data_value(iseq, reference_updating ? ISEQ_ORIGINAL_ISEQ(iseq) : NULL);
rb_gc_mark_and_move((VALUE *)&compile_data->err_info);
rb_gc_mark_and_move((VALUE *)&compile_data->catch_table_ary);

12
iseq.h
View File

@ -104,6 +104,16 @@ struct iseq_compile_data {
const VALUE err_info;
const VALUE catch_table_ary; /* Array */
/* Mirror fields from ISEQ_BODY so they are accessible during iseq setup */
unsigned int iseq_size;
VALUE *iseq_encoded; /* half-encoded iseq (insn addr and operands) */
bool is_single_mark_bit; /* identifies whether mark bits are single or a list */
union {
iseq_bits_t * list; /* Find references for GC */
iseq_bits_t single;
} mark_bits;
/* GC is not needed */
struct iseq_label_data *start_label;
struct iseq_label_data *end_label;
@ -194,7 +204,7 @@ VALUE *rb_iseq_original_iseq(const rb_iseq_t *iseq);
void rb_iseq_build_from_ary(rb_iseq_t *iseq, VALUE misc,
VALUE locals, VALUE args,
VALUE exception, VALUE body);
void rb_iseq_mark_and_pin_insn_storage(struct iseq_compile_data_storage *arena);
void rb_iseq_mark_and_move_insn_storage(struct iseq_compile_data_storage *arena);
VALUE rb_iseq_load(VALUE data, VALUE parent, VALUE opt);
VALUE rb_iseq_parameters(const rb_iseq_t *iseq, int is_proc);