Skip to content

Avoid pinning storage_head in iseq_mark_and_move #568

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
eileencodes wants to merge 1 commit into from
Closed

Avoid pinning storage_head in iseq_mark_and_move #568

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 18 additions & 15 deletions compile.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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++) {
Expand All @@ -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;
Expand All @@ -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 *
Expand Down Expand Up @@ -2607,15 +2607,14 @@ 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;
}
else {
mark_offset_bits = ZALLOC_N(iseq_bits_t, ISEQ_MBITS_BUFLEN(code_index));
}
mark_offset_bits = ZALLOC_N(iseq_bits_t, ISEQ_MBITS_BUFLEN(code_index));

ISEQ_COMPILE_DATA(iseq)->iseq_encoded = (void *)generated_iseq;
ISEQ_COMPILE_DATA(iseq)->iseq_size = code_index;
ISEQ_COMPILE_DATA(iseq)->mark_bits.list = mark_offset_bits;
ISEQ_COMPILE_DATA(iseq)->is_single_mark_bit = false;

list = FIRST_ELEMENT(anchor);
insns_info_index = code_index = sp = 0;
Expand Down Expand Up @@ -2829,13 +2828,17 @@ iseq_set_sequence(rb_iseq_t *iseq, LINK_ANCHOR *const anchor)

if (ISEQ_MBITS_BUFLEN(body->iseq_size) == 1) {
body->mark_bits.single = mark_offset_bits[0];
ISEQ_COMPILE_DATA(iseq)->is_single_mark_bit = true;
ISEQ_COMPILE_DATA(iseq)->mark_bits.single = mark_offset_bits[0];
ruby_xfree(mark_offset_bits);
}
else {
if (needs_bitmap) {
body->mark_bits.list = mark_offset_bits;
}
else {
body->mark_bits.list = 0;
ISEQ_COMPILE_DATA(iseq)->mark_bits.list = 0;
ruby_xfree(mark_offset_bits);
}
}
Expand Down Expand Up @@ -12128,13 +12131,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);
Expand All @@ -12159,7 +12162,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;
}
Expand Down
45 changes: 30 additions & 15 deletions iseq.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
12 changes: 11 additions & 1 deletion iseq.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
Loading