ruby-changes:71191
From: Peter <ko1@a...>
Date: Wed, 16 Feb 2022 23:50:42 +0900 (JST)
Subject: [ruby-changes:71191] 969ad5802d (master): Change feature_index from fake Array to darray
https://git.ruby-lang.org/ruby.git/commit/?id=969ad5802d From 969ad5802dfe60c254f2f30514233b05ece8049c Mon Sep 17 00:00:00 2001 From: Peter Zhu <peter@p...> Date: Tue, 15 Feb 2022 09:57:33 -0500 Subject: Change feature_index from fake Array to darray Using a fake (malloc) RArray is not friendly for the garbage collector. Fake RArray does not have a heap page, so it causes Variable Width Allocation to crash when we try to implement it on Arrays. This commit changes feature_index from a RArray to a darray. --- darray.h | 2 + load.c | 142 ++++++++++++++++++++++++++++++++++++++------------------------- 2 files changed, 89 insertions(+), 55 deletions(-) diff --git a/darray.h b/darray.h index 6c52b9b1f4..bb8199a98c 100644 --- a/darray.h +++ b/darray.h @@ -104,6 +104,8 @@ https://github.com/ruby/ruby/blob/trunk/darray.h#L104 rb_darray_make_impl((ptr_to_ary), size, sizeof(**(ptr_to_ary)), \ sizeof((*(ptr_to_ary))->data[0]), ruby_xcalloc) +#define rb_darray_data_ptr(ary) ((ary)->data) + // Set the size of the array to zero without freeing the backing memory. // Allows reusing the same array. // diff --git a/load.c b/load.c index 812fe2fe93..d171e1d92b 100644 --- a/load.c +++ b/load.c @@ -201,61 +201,95 @@ is_rbext_path(VALUE feature_path) https://github.com/ruby/ruby/blob/trunk/load.c#L201 return IS_RBEXT(RSTRING_PTR(feature_path) + len - rbext_len); } +typedef rb_darray(long) feature_indexes_t; + +struct features_index_add_single_args { + rb_vm_t *vm; + VALUE offset; + bool rb; +}; + +static int +features_index_add_single_callback(st_data_t *key, st_data_t *value, st_data_t raw_args, int existing) +{ + struct features_index_add_single_args *args = (struct features_index_add_single_args *)raw_args; + rb_vm_t *vm = args->vm; + VALUE offset = args->offset; + bool rb = args->rb; + + if (existing) { + VALUE this_feature_index = *value; + + if (FIXNUM_P(this_feature_index)) { + VALUE loaded_features = get_loaded_features(vm); + VALUE this_feature_path = RARRAY_AREF(loaded_features, FIX2LONG(this_feature_index)); + + feature_indexes_t feature_indexes; + rb_darray_make_with_gc(&feature_indexes, 2); + int top = (rb && !is_rbext_path(this_feature_path)) ? 1 : 0; + rb_darray_set(feature_indexes, top^0, FIX2LONG(this_feature_index)); + rb_darray_set(feature_indexes, top^1, FIX2LONG(offset)); + + assert(rb_darray_size(feature_indexes) == 2); + // assert feature_indexes does not look like a special const + assert(!SPECIAL_CONST_P((VALUE)feature_indexes)); + + *value = (st_data_t)feature_indexes; + } + else { + feature_indexes_t feature_indexes = (feature_indexes_t)this_feature_index; + long pos = -1; + + if (rb) { + VALUE loaded_features = get_loaded_features(vm); + for (size_t i = 0; i < rb_darray_size(feature_indexes); ++i) { + long idx = rb_darray_get(feature_indexes, i); + VALUE this_feature_path = RARRAY_AREF(loaded_features, idx); + Check_Type(this_feature_path, T_STRING); + if (!is_rbext_path(this_feature_path)) { + pos = i; + break; + } + } + } + + rb_darray_append_with_gc(&feature_indexes, FIX2LONG(offset)); + /* darray may realloc which will change the pointer */ + *value = (st_data_t)feature_indexes; + + if (pos >= 0) { + long *ptr = rb_darray_data_ptr(feature_indexes); + long len = rb_darray_size(feature_indexes); + MEMMOVE(ptr + pos, ptr + pos + 1, long, len - pos - 1); + ptr[pos] = FIX2LONG(offset); + } + } + } + else { + *value = offset; + } + + return ST_CONTINUE; +} + static void features_index_add_single(rb_vm_t *vm, const char* str, size_t len, VALUE offset, bool rb) { struct st_table *features_index; - VALUE this_feature_index = Qnil; st_data_t short_feature_key; - st_data_t data; Check_Type(offset, T_FIXNUM); short_feature_key = feature_key(str, len); features_index = get_loaded_features_index_raw(vm); - if (!st_lookup(features_index, short_feature_key, &data) || - NIL_P(this_feature_index = (VALUE)data)) { - st_insert(features_index, short_feature_key, (st_data_t)offset); - } - else if (FIXNUM_P(this_feature_index)) { - VALUE loaded_features = get_loaded_features(vm); - VALUE this_feature_path = RARRAY_AREF(loaded_features, FIX2LONG(this_feature_index)); - VALUE feature_indexes[2]; - int top = (rb && !is_rbext_path(this_feature_path)) ? 1 : 0; - feature_indexes[top^0] = this_feature_index; - feature_indexes[top^1] = offset; - this_feature_index = (VALUE)xcalloc(1, sizeof(struct RArray)); - RBASIC(this_feature_index)->flags = T_ARRAY; /* fake VALUE, do not mark/sweep */ - rb_ary_cat(this_feature_index, feature_indexes, numberof(feature_indexes)); - st_insert(features_index, short_feature_key, (st_data_t)this_feature_index); - } - else { - long pos = -1; - Check_Type(this_feature_index, T_ARRAY); - if (rb) { - VALUE loaded_features = get_loaded_features(vm); - for (long i = 0; i < RARRAY_LEN(this_feature_index); ++i) { - VALUE idx = RARRAY_AREF(this_feature_index, i); - VALUE this_feature_path = RARRAY_AREF(loaded_features, FIX2LONG(idx)); - Check_Type(this_feature_path, T_STRING); - if (!is_rbext_path(this_feature_path)) { - /* as this_feature_index is a fake VALUE, `push` (which - * doesn't wb_unprotect like as rb_ary_splice) first, - * then rotate partially. */ - pos = i; - break; - } - } - } - rb_ary_push(this_feature_index, offset); - if (pos >= 0) { - VALUE *ptr = (VALUE *)RARRAY_CONST_PTR_TRANSIENT(this_feature_index); - long len = RARRAY_LEN(this_feature_index); - MEMMOVE(ptr + pos, ptr + pos + 1, VALUE, len - pos - 1); - ptr[pos] = offset; - } - } + struct features_index_add_single_args args = { + .vm = vm, + .offset = offset, + .rb = rb, + }; + + st_update(features_index, short_feature_key, features_index_add_single_callback, (st_data_t)&args); } /* Add to the loaded-features index all the required entries for @@ -309,8 +343,7 @@ loaded_features_index_clear_i(st_data_t key, st_data_t val, st_data_t arg) https://github.com/ruby/ruby/blob/trunk/load.c#L343 { VALUE obj = (VALUE)val; if (!SPECIAL_CONST_P(obj)) { - rb_ary_free(obj); - ruby_sized_xfree((void *)obj, sizeof(struct RArray)); + rb_darray_free_with_gc((void *)obj); } return ST_DELETE; } @@ -480,18 +513,17 @@ rb_feature_p(rb_vm_t *vm, const char *feature, const char *ext, int rb, int expa https://github.com/ruby/ruby/blob/trunk/load.c#L513 as any distractors, so we may ignore all other entries in `features`. */ if (st_lookup(features_index, key, &data) && !NIL_P(this_feature_index = (VALUE)data)) { - for (i = 0; ; i++) { - VALUE entry; + for (size_t i = 0; ; i++) { long index; - if (RB_TYPE_P(this_feature_index, T_ARRAY)) { - if (i >= RARRAY_LEN(this_feature_index)) break; - entry = RARRAY_AREF(this_feature_index, i); - } - else { + if (FIXNUM_P(this_feature_index)) { if (i > 0) break; - entry = this_feature_index; + index = FIX2LONG(this_feature_index); } - index = FIX2LONG(entry); + else { + feature_indexes_t feature_indexes = (feature_indexes_t)this_feature_index; + if (i >= rb_darray_size(feature_indexes)) break; + index = rb_darray_get(feature_indexes, i); + } v = RARRAY_AREF(features, index); f = StringValuePtr(v); -- cgit v1.2.1 -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/