Skip to content

Commit

Permalink
Resize arrays in rb_ary_freeze and use it for freezing arrays
Browse files Browse the repository at this point in the history
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
ruby/ruby#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: Aaron Patterson <tenderlove@ruby-lang.org>
Co-Authored-By: methodmissing <lourens@methodmissing.com>
  • Loading branch information
3 people authored and hsbt committed Jul 12, 2024
1 parent ef5a0da commit 47704d9
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions ext/date/date_core.c
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ static VALUE sym_hour, sym_min, sym_sec, sym_sec_fraction, sym_zone;
#define f_add3(x,y,z) f_add(f_add(x, y), z)
#define f_sub3(x,y,z) f_sub(f_sub(x, y), z)

#define f_frozen_ary(...) rb_obj_freeze(rb_ary_new3(__VA_ARGS__))
#define f_frozen_ary(...) rb_ary_freeze(rb_ary_new3(__VA_ARGS__))

static VALUE date_initialize(int argc, VALUE *argv, VALUE self);
static VALUE datetime_initialize(int argc, VALUE *argv, VALUE self);
Expand Down Expand Up @@ -9466,7 +9466,7 @@ mk_ary_of_str(long len, const char *a[])
}
rb_ary_push(o, e);
}
rb_obj_freeze(o);
rb_ary_freeze(o);
return o;
}

Expand Down

0 comments on commit 47704d9

Please sign in to comment.