Skip to content
  • eileencodes's avatar
    d25b74b3
    Resize arrays in `rb_ary_freeze` and use it for freezing arrays · d25b74b3
    eileencodes authored
    While working on a separate issue we found that in some cases
    `ary_heap_realloc` was being called on frozen arrays. To fix this, this
    change does the following:
    
    1) Updates `rb_ary_freeze` to assert the type is an array, return if
    already frozen, and shrink the capacity if it is not embedded, shared
    or a shared root.
    2) Replaces `rb_obj_freeze` with `rb_ary_freeze` when the object is
    always an array.
    3) In `ary_heap_realloc`, ensure the new capa is set with
    `ARY_SET_CAPA`. Previously the change in capa was not set.
    4) Adds an assertion to `ary_heap_realloc` that the array is not frozen.
    
    Some of this work was originally done in
    https://github.com/ruby/ruby/pull/2640, referencing this issue
    https://bugs.ruby-lang.org/issues/16291
    
    . There didn't appear to be any
    objections to this PR, it appears to have simply lost traction.
    
    The original PR made changes to arrays and strings at the same time,
    this PR only does arrays. Also it was old enough that rather than revive
    that branch I've made a new one. I added Lourens as co-author in addtion
    to Aaron who helped me with this patch.
    
    The original PR made this change for performance reasons, and while
    that's still true for this PR, the goal of this PR is to avoid
    calling `ary_heap_realloc` on frozen arrays. The capacity should be
    shrunk _before_ the array is frozen, not after.
    
    Co-authored-by: default avatarAaron Patterson <tenderlove@ruby-lang.org>
    Co-Authored-By: default avatarmethodmissing <lourens@methodmissing.com>
    d25b74b3
    Resize arrays in `rb_ary_freeze` and use it for freezing arrays
    eileencodes authored
    While working on a separate issue we found that in some cases
    `ary_heap_realloc` was being called on frozen arrays. To fix this, this
    change does the following:
    
    1) Updates `rb_ary_freeze` to assert the type is an array, return if
    already frozen, and shrink the capacity if it is not embedded, shared
    or a shared root.
    2) Replaces `rb_obj_freeze` with `rb_ary_freeze` when the object is
    always an array.
    3) In `ary_heap_realloc`, ensure the new capa is set with
    `ARY_SET_CAPA`. Previously the change in capa was not set.
    4) Adds an assertion to `ary_heap_realloc` that the array is not frozen.
    
    Some of this work was originally done in
    https://github.com/ruby/ruby/pull/2640, referencing this issue
    https://bugs.ruby-lang.org/issues/16291
    
    . There didn't appear to be any
    objections to this PR, it appears to have simply lost traction.
    
    The original PR made changes to arrays and strings at the same time,
    this PR only does arrays. Also it was old enough that rather than revive
    that branch I've made a new one. I added Lourens as co-author in addtion
    to Aaron who helped me with this patch.
    
    The original PR made this change for performance reasons, and while
    that's still true for this PR, the goal of this PR is to avoid
    calling `ary_heap_realloc` on frozen arrays. The capacity should be
    shrunk _before_ the array is frozen, not after.
    
    Co-authored-by: default avatarAaron Patterson <tenderlove@ruby-lang.org>
    Co-Authored-By: default avatarmethodmissing <lourens@methodmissing.com>
Loading