ruby-changes:61974
From: =E5=8D=9C=E9=83=A8=E6=98=8C=E5=B9=B3 <ko1@a...>
Date: Mon, 29 Jun 2020 11:06:22 +0900 (JST)
Subject: [ruby-changes:61974] 86c869fb59 (master): ary_ensure_room_for_unshift: do not goto into a branch
https://git.ruby-lang.org/ruby.git/commit/?id=86c869fb59 From 86c869fb5967e1e75691721683c43c1d12f05966 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8D=9C=E9=83=A8=E6=98=8C=E5=B9=B3?= <shyouhei@r...> Date: Thu, 11 Jun 2020 10:28:34 +0900 Subject: ary_ensure_room_for_unshift: do not goto into a branch I'm not necessarily against every goto in general, but jumping into a branch is definitely a bad idea. Better refactor. diff --git a/array.c b/array.c index d6a2a7a..2d16287 100644 --- a/array.c +++ b/array.c @@ -1569,28 +1569,27 @@ rb_ary_behead(VALUE ary, long n) https://github.com/ruby/ruby/blob/trunk/array.c#L1569 } static VALUE -ary_ensure_room_for_unshift(VALUE ary, int argc) +make_room_for_unshift(VALUE ary, const VALUE *head, VALUE *sharedp, int argc, long capa, long len) +{ + if (head - sharedp < argc) { + long room = capa - len - argc; + room -= room >> 4; + MEMMOVE((VALUE *)sharedp + argc + room, head, VALUE, len); + head = sharedp + argc + room; + } + ARY_SET_PTR(ary, head - argc); + assert(ARY_SHARED_ROOT_OCCUPIED(ARY_SHARED_ROOT(ary))); + ary_verify(ary); + return ARY_SHARED_ROOT(ary); +} +static VALUE +ary_modify_for_unshift(VALUE ary, int argc) { long len = RARRAY_LEN(ary); long new_len = len + argc; long capa; const VALUE *head, *sharedp; - if (len > ARY_MAX_SIZE - argc) { - rb_raise(rb_eIndexError, "index %ld too big", new_len); - } - - if (ARY_SHARED_P(ary)) { - VALUE shared_root = ARY_SHARED_ROOT(ary); - capa = RARRAY_LEN(shared_root); - if (ARY_SHARED_ROOT_OCCUPIED(shared_root) && capa > new_len) { - rb_ary_modify_check(ary); - head = RARRAY_CONST_PTR_TRANSIENT(ary); - sharedp = RARRAY_CONST_PTR_TRANSIENT(shared_root); - goto makeroom_if_need; - } - } - rb_ary_modify(ary); capa = ARY_CAPA(ary); if (capa - (capa >> 6) <= new_len) { @@ -1606,21 +1605,7 @@ ary_ensure_room_for_unshift(VALUE ary, int argc) https://github.com/ruby/ruby/blob/trunk/array.c#L1605 ary_make_shared(ary); head = sharedp = RARRAY_CONST_PTR_TRANSIENT(ary); - goto makeroom; - makeroom_if_need: - if (head - sharedp < argc) { - long room; - makeroom: - room = capa - new_len; - room -= room >> 4; - MEMMOVE((VALUE *)sharedp + argc + room, head, VALUE, len); - head = sharedp + argc + room; - } - ARY_SET_PTR(ary, head - argc); - assert(ARY_SHARED_ROOT_OCCUPIED(ARY_SHARED_ROOT(ary))); - - ary_verify(ary); - return ARY_SHARED_ROOT(ary); + return make_room_for_unshift(ary, head, (void *)sharedp, argc, capa, len); } else { /* sliding items */ @@ -1632,6 +1617,34 @@ ary_ensure_room_for_unshift(VALUE ary, int argc) https://github.com/ruby/ruby/blob/trunk/array.c#L1617 return ary; } } +static VALUE +ary_ensure_room_for_unshift(VALUE ary, int argc) +{ + long len = RARRAY_LEN(ary); + long new_len = len + argc; + if (len > ARY_MAX_SIZE - argc) { + rb_raise(rb_eIndexError, "index %ld too big", new_len); + } + else if (! ARY_SHARED_P(ary)) { + return ary_modify_for_unshift(ary, argc); + } + else { + VALUE shared_root = ARY_SHARED_ROOT(ary); + long capa = RARRAY_LEN(shared_root); + if (! ARY_SHARED_ROOT_OCCUPIED(shared_root)) { + return ary_modify_for_unshift(ary, argc); + } + else if (new_len > capa) { + return ary_modify_for_unshift(ary, argc); + } + else { + rb_ary_modify_check(ary); + const VALUE * head = RARRAY_CONST_PTR_TRANSIENT(ary); + void *sharedp = (void *)RARRAY_CONST_PTR_TRANSIENT(shared_root); + return make_room_for_unshift(ary, head, sharedp, argc, capa, len); + } + } +} /* * call-seq: -- cgit v0.10.2 -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/