ruby-changes:72658
From: Peter <ko1@a...>
Date: Sat, 23 Jul 2022 02:29:55 +0900 (JST)
Subject: [ruby-changes:72658] e199ae3edc (master): Remove reference counting for all frozen arrays
https://git.ruby-lang.org/ruby.git/commit/?id=e199ae3edc From e199ae3edcead0271c6da3410eb02acd927739b7 Mon Sep 17 00:00:00 2001 From: Peter Zhu <peter@p...> Date: Fri, 22 Jul 2022 11:04:43 -0400 Subject: Remove reference counting for all frozen arrays The RARRAY_LITERAL_FLAG was added in commit 5871ecf956711fcacad7c03f2aef95115ed25bc4 to improve CoW performance for array literals by not keeping track of reference counts. This commit reverts that commit and has an alternate implementation that is more generic for all frozen arrays. Since frozen arrays cannot be modified, we don't need to set the RARRAY_SHARED_ROOT_FLAG and we don't need to do reference counting. --- array.c | 45 +++++++++++++++++---------------------------- compile.c | 4 ++-- internal/array.h | 9 --------- 3 files changed, 19 insertions(+), 39 deletions(-) diff --git a/array.c b/array.c index f398ac8e52..b0c4cee0bf 100644 --- a/array.c +++ b/array.c @@ -55,19 +55,19 @@ VALUE rb_cArray; https://github.com/ruby/ruby/blob/trunk/array.c#L55 * The length of the array when RARRAY_EMBED_FLAG is set. * endif * 12: RARRAY_SHARED_ROOT_FLAG - * The array is a shared root. The buffer this array points to is - * owned by this array but may be pointed to by other arrays. + * The array is a shared root that does reference counting. The buffer + * this array points to is owned by this array but may be pointed to + * by other arrays. + * Note: Frozen arrays may be a shared root without this flag being + * set. Frozen arrays do not have reference counting because + * they cannot be modified. Not updating the reference count + * improves copy-on-write performance. Their reference count is + * assumed to be infinity. * 13: RARRAY_TRANSIENT_FLAG * The buffer of the array is allocated on the transient heap. * 14: RARRAY_PTR_IN_USE_FLAG * The buffer of the array is in use. This is only used during * debugging. - * 15: RARRAY_LITERAL_FLAG - * The array is a shared root for array literals. This differs from - * RARRAY_SHARED_ROOT_FLAG in that this does not keep track of the - * reference count (it assumes that the reference count is infinity). - * This is used to improve copy-on-write performance since updating - * reference counts could cause system page invalidation. */ /* for OPTIMIZED_CMP: */ @@ -166,18 +166,20 @@ should_be_T_ARRAY(VALUE ary) https://github.com/ruby/ruby/blob/trunk/array.c#L166 const VALUE _value_ = (value); \ assert(!ARY_EMBED_P(_ary_)); \ assert(ARY_SHARED_P(_ary_)); \ - assert(!ARY_LITERAL_P(_ary_)); \ - assert(ARY_SHARED_ROOT_P(_value_) || ARY_LITERAL_P(_value_)); \ + assert(!OBJ_FROZEN(_ary_)); \ + assert(ARY_SHARED_ROOT_P(_value_) || OBJ_FROZEN(_value_)); \ RB_OBJ_WRITE(_ary_, &RARRAY(_ary_)->as.heap.aux.shared_root, _value_); \ } while (0) -#define ARY_SHARED_ROOT_OCCUPIED(ary) (!ARY_LITERAL_P(ary) && ARY_SHARED_ROOT_REFCNT(ary) == 1) +#define ARY_SHARED_ROOT_OCCUPIED(ary) (!OBJ_FROZEN(ary) && ARY_SHARED_ROOT_REFCNT(ary) == 1) #define ARY_SET_SHARED_ROOT_REFCNT(ary, value) do { \ assert(ARY_SHARED_ROOT_P(ary)); \ + assert(!OBJ_FROZEN(ary)); \ assert((value) >= 0); \ RARRAY(ary)->as.heap.aux.capa = (value); \ } while (0) #define FL_SET_SHARED_ROOT(ary) do { \ + assert(!OBJ_FROZEN(ary)); \ assert(!ARY_EMBED_P(ary)); \ assert(!RARRAY_TRANSIENT_P(ary)); \ FL_SET((ary), RARRAY_SHARED_ROOT_FLAG); \ @@ -259,7 +261,7 @@ ary_verify_(VALUE ary, const char *file, int line) https://github.com/ruby/ruby/blob/trunk/array.c#L261 const VALUE *ptr = ARY_HEAP_PTR(ary); const VALUE *root_ptr = RARRAY_CONST_PTR_TRANSIENT(root); long len = ARY_HEAP_LEN(ary), root_len = RARRAY_LEN(root); - assert(ARY_SHARED_ROOT_P(root) || ARY_LITERAL_P(root)); + assert(ARY_SHARED_ROOT_P(root) || OBJ_FROZEN(root)); assert(root_ptr <= ptr && ptr + len <= root_ptr + root_len); ary_verify(root); } @@ -588,7 +590,7 @@ ary_double_capa(VALUE ary, long min) https://github.com/ruby/ruby/blob/trunk/array.c#L590 static void rb_ary_decrement_share(VALUE shared_root) { - if (!ARY_LITERAL_P(shared_root)) { + if (!OBJ_FROZEN(shared_root)) { long num = ARY_SHARED_ROOT_REFCNT(shared_root); ARY_SET_SHARED_ROOT_REFCNT(shared_root, num - 1); } @@ -619,7 +621,7 @@ rb_ary_reset(VALUE ary) https://github.com/ruby/ruby/blob/trunk/array.c#L621 static VALUE rb_ary_increment_share(VALUE shared_root) { - if (!ARY_LITERAL_P(shared_root)) { + if (!OBJ_FROZEN(shared_root)) { long num = ARY_SHARED_ROOT_REFCNT(shared_root); assert(num >= 0); ARY_SET_SHARED_ROOT_REFCNT(shared_root, num + 1); @@ -982,15 +984,6 @@ rb_ary_tmp_new_fill(long capa) https://github.com/ruby/ruby/blob/trunk/array.c#L984 return ary; } -VALUE -rb_ary_literal_new(long capa) -{ - VALUE ary = ary_new(0, capa); - rb_ary_transient_heap_evacuate(ary, TRUE); - FL_SET(ary, RARRAY_LITERAL_FLAG); - return ary; -} - void rb_ary_free(VALUE ary) { @@ -1044,7 +1037,6 @@ static VALUE https://github.com/ruby/ruby/blob/trunk/array.c#L1037 ary_make_shared(VALUE ary) { assert(!ARY_EMBED_P(ary)); - assert(!ARY_LITERAL_P(ary)); ary_verify(ary); if (ARY_SHARED_P(ary)) { @@ -1056,8 +1048,6 @@ ary_make_shared(VALUE ary) https://github.com/ruby/ruby/blob/trunk/array.c#L1048 else if (OBJ_FROZEN(ary)) { rb_ary_transient_heap_evacuate(ary, TRUE); ary_shrink_capa(ary); - FL_SET_SHARED_ROOT(ary); - ARY_SET_SHARED_ROOT_REFCNT(ary, 1); return ary; } else { @@ -1077,7 +1067,6 @@ ary_make_shared(VALUE ary) https://github.com/ruby/ruby/blob/trunk/array.c#L1067 FL_SET_SHARED(ary); RB_DEBUG_COUNTER_INC(obj_ary_shared_create); ARY_SET_SHARED(ary, shared); - OBJ_FREEZE(shared); ary_verify(shared); ary_verify(ary); @@ -1348,7 +1337,7 @@ ary_make_partial(VALUE ary, VALUE klass, long offset, long len) https://github.com/ruby/ruby/blob/trunk/array.c#L1337 VALUE result = ary_alloc_heap(klass); assert(!ARY_EMBED_P(result)); - VALUE shared = ARY_LITERAL_P(ary) ? ary : ary_make_shared(ary); + VALUE shared = ary_make_shared(ary); ARY_SET_PTR(result, RARRAY_CONST_PTR_TRANSIENT(ary)); ARY_SET_LEN(result, RARRAY_LEN(ary)); diff --git a/compile.c b/compile.c index 7aca48d8dc..193a9a1c6d 100644 --- a/compile.c +++ b/compile.c @@ -4369,7 +4369,7 @@ compile_array(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE *node, int pop https://github.com/ruby/ruby/blob/trunk/compile.c#L4369 if ((first_chunk && stack_len == 0 && !node_tmp) || count >= min_tmp_ary_len) { /* The literal contains only optimizable elements, or the subarray is long enough */ - VALUE ary = rb_ary_literal_new(count); + VALUE ary = rb_ary_tmp_new(count); /* Create a hidden array */ for (; count; count--, node = node->nd_next) @@ -12349,7 +12349,7 @@ ibf_load_object_array(const struct ibf_load *load, const struct ibf_object_heade https://github.com/ruby/ruby/blob/trunk/compile.c#L12349 const long len = (long)ibf_load_small_value(load, &reading_pos); - VALUE ary = header->internal ? rb_ary_literal_new(len) : rb_ary_new_capa(len); + VALUE ary = header->frozen ? rb_ary_tmp_new(len) : rb_ary_new_capa(len); int i; for (i=0; i<len; i++) { diff --git a/internal/array.h b/internal/array.h index d8a824423d..0de82d9f30 100644 --- a/internal/array.h +++ b/internal/array.h @@ -21,7 +21,6 @@ https://github.com/ruby/ruby/blob/trunk/internal/array.h#L21 #define RARRAY_SHARED_FLAG ELTS_SHARED #define RARRAY_SHARED_ROOT_FLAG FL_USER12 #define RARRAY_PTR_IN_USE_FLAG FL_USER14 -#define RARRAY_LITERAL_FLAG FL_USER15 /* array.c */ VALUE rb_ary_last(int, const VALUE *, VALUE); @@ -36,7 +35,6 @@ void rb_ary_cancel_sharing(VALUE ary); https://github.com/ruby/ruby/blob/trunk/internal/array.h#L35 size_t rb_ary_size_as_embedded(VALUE ary); void rb_ary_make_embedded(VALUE ary); bool rb_ary_embeddable_p(VALUE ary); -VALUE rb_ary_literal_new(long capa); static inline VALUE rb_ary_entry_internal(VALUE ary, long offset); static inline bool ARY_PTR_USING_P(VALUE ary); @@ -120,13 +118,6 @@ ARY_SHARED_ROOT_REFCNT(VALUE ary) https://github.com/ruby/ruby/blob/trunk/internal/array.h#L118 return RARRAY(ary)->as.heap.aux.capa; } -static inline bool -ARY_LITERAL_P(VALUE ary) -{ - assert(RB_TYPE_P(ary, T_ARRAY)); - return FL_TEST_RAW(ary, RARRAY_LITERAL_FLAG); -} - static inline void RARY_TRANSIENT_SET(VALUE ary) { -- cgit v1.2.1 -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/