* [PATCH] use Array instead of custom struct for generic ivars
@ 2015-06-23 23:20 Eric Wong
0 siblings, 0 replies; only message in thread
From: Eric Wong @ 2015-06-23 23:20 UTC (permalink / raw)
To: spew
This reduces both code and object size while reducing the cognitive
overhead necessary to understand the code. Memory usage should be
slightly higher due to Array overheads, but still better than what we
had in Ruby 2.2 and earlier.
text data bss dec hex filename
2837117 22688 71576 2931381 2cbab5 ruby.before
2836221 22688 71576 2930485 2cb735 ruby.after
* array.c (rb_mem_clear): use memfill
(rb_ary_store_fill): new function
(rb_ary_store): use rb_ary_store_fill
* internal.h (rb_ary_store_fill): new declaration
* variable.c (struct gen_ivtbl): remove
(struct ivar_update): adjust type
(struct gen_ivar_compat_tbl): ditto
(struct gen_ivar_tag): ditto
(struct givar_copy): ditto
(gen_ivar_compat_tbl_i): adjust for type change
(gen_ivtbl_get): ditto
(generic_ivar_delete): ditto
(generic_ivar_get): ditto
(generic_ivar_update): ditto
(generic_ivar_defined): ditto
(rb_mark_generic_ivar): ditto
(rb_free_generic_ivar): ditto
(rb_generic_ivar_memsize): ditto
(generic_ivar_set): ditto
(gen_ivtbl_count): ditto
(gen_ivar_each_i): ditto
(gen_ivar_copy): ditto
(rb_copy_generic_ivar): ditto
(rb_ivar_count): ditto
(gen_ivtbl_bytes): remove
(gen_ivtbl_resize): remove
(gen_ivtbl_dup): remove
(gen_ivtbl_mark): remove
---
array.c | 28 +++++-----
internal.h | 1 +
variable.c | 170 +++++++++++++++++++------------------------------------------
3 files changed, 69 insertions(+), 130 deletions(-)
diff --git a/array.c b/array.c
index 5953dfc..4cce5bb 100644
--- a/array.c
+++ b/array.c
@@ -131,14 +131,20 @@ static ID id_cmp, id_div, id_power;
#define ARY_SET(a, i, v) RARRAY_ASET((assert(!ARY_SHARED_P(a)), (a)), (i), (v))
-void
-rb_mem_clear(register VALUE *mem, register long size)
+static inline void
+memfill(register VALUE *mem, register long size, register VALUE val)
{
while (size--) {
- *mem++ = Qnil;
+ *mem++ = val;
}
}
+void
+rb_mem_clear(register VALUE *mem, register long size)
+{
+ memfill(mem, size, Qnil);
+}
+
static void
ary_mem_clear(VALUE ary, long beg, long size)
{
@@ -147,14 +153,6 @@ ary_mem_clear(VALUE ary, long beg, long size)
});
}
-static inline void
-memfill(register VALUE *mem, register long size, register VALUE val)
-{
- while (size--) {
- *mem++ = val;
- }
-}
-
static void
ary_memfill(VALUE ary, long beg, long size, VALUE val)
{
@@ -803,6 +801,12 @@ rb_ary_s_create(int argc, VALUE *argv, VALUE klass)
void
rb_ary_store(VALUE ary, long idx, VALUE val)
{
+ rb_ary_store_fill(ary, idx, val, Qnil);
+}
+
+void
+rb_ary_store_fill(VALUE ary, long idx, VALUE val, VALUE fill)
+{
long len = RARRAY_LEN(ary);
if (idx < 0) {
@@ -821,7 +825,7 @@ rb_ary_store(VALUE ary, long idx, VALUE val)
ary_double_capa(ary, idx);
}
if (idx > len) {
- ary_mem_clear(ary, len, idx - len + 1);
+ ary_memfill(ary, len, idx - len + 1, fill);
}
if (idx >= len) {
diff --git a/internal.h b/internal.h
index 84eb6e2..36bfb55 100644
--- a/internal.h
+++ b/internal.h
@@ -639,6 +639,7 @@ void rb_ary_set_len(VALUE, long);
void rb_ary_delete_same(VALUE, VALUE);
VALUE rb_ary_tmp_new_fill(long capa);
size_t rb_ary_memsize(VALUE);
+void rb_ary_store_fill(VALUE ary, long idx, VALUE val, VALUE fill);
#ifdef __GNUC__
#define rb_ary_new_from_args(n, ...) \
__extension__ ({ \
diff --git a/variable.c b/variable.c
index e130ee1..c69fe8e 100644
--- a/variable.c
+++ b/variable.c
@@ -26,16 +26,10 @@ static int const_update(st_data_t *, st_data_t *, st_data_t, int);
static st_table *generic_iv_tbl;
static st_table *generic_iv_tbl_compat;
-/* per-object */
-struct gen_ivtbl {
- long numiv; /* only uses 32-bits */
- VALUE ivptr[1]; /* flexible array */
-};
-
struct ivar_update {
union {
st_table *iv_index_tbl;
- struct gen_ivtbl *ivtbl;
+ VALUE ivtbl;
} u;
st_data_t index;
int extended;
@@ -949,7 +943,7 @@ rb_alias_variable(ID name1, ID name2)
}
struct gen_ivar_compat_tbl {
- struct gen_ivtbl *ivtbl;
+ VALUE ivtbl;
st_table *tbl;
};
@@ -958,8 +952,8 @@ gen_ivar_compat_tbl_i(st_data_t id, st_data_t index, st_data_t arg)
{
struct gen_ivar_compat_tbl *a = (struct gen_ivar_compat_tbl *)arg;
- if ((long)index < a->ivtbl->numiv) {
- VALUE val = a->ivtbl->ivptr[index];
+ if ((long)index < RARRAY_LEN(a->ivtbl)) {
+ VALUE val = RARRAY_AREF(a->ivtbl, index);
if (val != Qundef) {
st_add_direct(a->tbl, id, (st_data_t)val);
}
@@ -968,12 +962,12 @@ gen_ivar_compat_tbl_i(st_data_t id, st_data_t index, st_data_t arg)
}
static int
-gen_ivtbl_get(VALUE obj, struct gen_ivtbl **ivtbl)
+gen_ivtbl_get(VALUE obj, VALUE *ivtbl)
{
st_data_t data;
if (st_lookup(generic_iv_tbl, (st_data_t)obj, &data)) {
- *ivtbl = (struct gen_ivtbl *)data;
+ *ivtbl = (VALUE)data;
return 1;
}
return 0;
@@ -1014,17 +1008,17 @@ rb_generic_ivar_table(VALUE obj)
static VALUE
generic_ivar_delete(VALUE obj, ID id, VALUE undef)
{
- struct gen_ivtbl *ivtbl;
+ VALUE ivtbl;
if (gen_ivtbl_get(obj, &ivtbl)) {
st_table *iv_index_tbl = RCLASS_IV_INDEX_TBL(rb_obj_class(obj));
st_data_t index;
if (st_lookup(iv_index_tbl, (st_data_t)id, &index)) {
- if ((long)index < ivtbl->numiv) {
- VALUE ret = ivtbl->ivptr[index];
+ if ((long)index < RARRAY_LEN(ivtbl)) {
+ VALUE ret = RARRAY_AREF(ivtbl, index);
- ivtbl->ivptr[index] = Qundef;
+ RARRAY_ASET(ivtbl, index, Qundef);
return ret == Qundef ? undef : ret;
}
}
@@ -1035,15 +1029,15 @@ generic_ivar_delete(VALUE obj, ID id, VALUE undef)
static VALUE
generic_ivar_get(VALUE obj, ID id, VALUE undef)
{
- struct gen_ivtbl *ivtbl;
+ VALUE ivtbl;
if (gen_ivtbl_get(obj, &ivtbl)) {
st_table *iv_index_tbl = RCLASS_IV_INDEX_TBL(rb_obj_class(obj));
st_data_t index;
if (st_lookup(iv_index_tbl, (st_data_t)id, &index)) {
- if ((long)index < ivtbl->numiv) {
- VALUE ret = ivtbl->ivptr[index];
+ if ((long)index < RARRAY_LEN(ivtbl)) {
+ VALUE ret = RARRAY_AREF(ivtbl, index);
return ret == Qundef ? undef : ret;
}
@@ -1052,37 +1046,6 @@ generic_ivar_get(VALUE obj, ID id, VALUE undef)
return undef;
}
-static size_t
-gen_ivtbl_bytes(size_t n)
-{
- return sizeof(struct gen_ivtbl) + n * sizeof(VALUE) - sizeof(VALUE);
-}
-
-struct gen_ivtbl *
-gen_ivtbl_resize(struct gen_ivtbl *old, long n)
-{
- long len = old ? old->numiv : 0;
- struct gen_ivtbl *ivtbl = xrealloc(old, gen_ivtbl_bytes(n));
-
- ivtbl->numiv = n;
- for (; len < n; len++) {
- ivtbl->ivptr[len] = Qundef;
- }
-
- return ivtbl;
-}
-
-struct gen_ivtbl *
-gen_ivtbl_dup(const struct gen_ivtbl *orig)
-{
- size_t s = gen_ivtbl_bytes(orig->numiv);
- struct gen_ivtbl *ivtbl = xmalloc(s);
-
- memcpy(ivtbl, orig, s);
-
- return ivtbl;
-}
-
static long
iv_index_tbl_newsize(struct ivar_update *ivup)
{
@@ -1100,33 +1063,22 @@ generic_ivar_update(st_data_t *k, st_data_t *v, st_data_t u, int existing)
{
VALUE obj = (VALUE)*k;
struct ivar_update *ivup = (struct ivar_update *)u;
- long newsize;
- int ret = ST_CONTINUE;
- struct gen_ivtbl *ivtbl;
if (existing) {
- ivtbl = (struct gen_ivtbl *)*v;
- if ((long)ivup->index >= ivtbl->numiv) {
- goto resize;
- }
- ret = ST_STOP;
- }
- else {
- FL_SET(obj, FL_EXIVAR);
- ivtbl = 0;
-resize:
- newsize = iv_index_tbl_newsize(ivup);
- ivtbl = gen_ivtbl_resize(ivtbl, newsize);
- *v = (st_data_t)ivtbl;
+ ivup->u.ivtbl = (VALUE)*v;
+ return ST_STOP;
}
- ivup->u.ivtbl = ivtbl;
- return ret;
+
+ ivup->u.ivtbl = rb_ary_tmp_new(0);
+ FL_SET(obj, FL_EXIVAR);
+ *v = (st_data_t)ivup->u.ivtbl;
+ return ST_CONTINUE;
}
static VALUE
generic_ivar_defined(VALUE obj, ID id)
{
- struct gen_ivtbl *ivtbl;
+ VALUE ivtbl;
st_table *iv_index_tbl = RCLASS_IV_INDEX_TBL(rb_obj_class(obj));
st_data_t index;
@@ -1134,7 +1086,7 @@ generic_ivar_defined(VALUE obj, ID id)
if (!st_lookup(iv_index_tbl, (st_data_t)id, &index)) return Qfalse;
if (!gen_ivtbl_get(obj, &ivtbl)) return Qfalse;
- if (((long)index < ivtbl->numiv) && (ivtbl->ivptr[index] != Qundef))
+ if ((long)index < RARRAY_LEN(ivtbl) && RARRAY_AREF(ivtbl, index) != Qundef)
return Qtrue;
return Qfalse;
@@ -1143,7 +1095,7 @@ generic_ivar_defined(VALUE obj, ID id)
static int
generic_ivar_remove(VALUE obj, ID id, st_data_t *valp)
{
- struct gen_ivtbl *ivtbl;
+ VALUE ivtbl;
st_data_t key = (st_data_t)id;
st_data_t index;
st_table *iv_index_tbl = RCLASS_IV_INDEX_TBL(rb_obj_class(obj));
@@ -1152,32 +1104,22 @@ generic_ivar_remove(VALUE obj, ID id, st_data_t *valp)
if (!st_lookup(iv_index_tbl, key, &index)) return 0;
if (!gen_ivtbl_get(obj, &ivtbl)) return 0;
- if ((long)index < ivtbl->numiv) {
- if (ivtbl->ivptr[index] != Qundef) {
- ivtbl->ivptr[index] = Qundef;
+ if ((long)index < RARRAY_LEN(ivtbl)) {
+ if (RARRAY_AREF(ivtbl, index) != Qundef) {
+ RARRAY_ASET(ivtbl, index, Qundef);
return 1;
}
}
return 0;
}
-static void
-gen_ivtbl_mark(const struct gen_ivtbl *ivtbl)
-{
- long i;
-
- for (i = 0; i < ivtbl->numiv; i++) {
- rb_gc_mark(ivtbl->ivptr[i]);
- }
-}
-
void
rb_mark_generic_ivar(VALUE obj)
{
- struct gen_ivtbl *ivtbl;
+ VALUE ivtbl;
if (gen_ivtbl_get(obj, &ivtbl)) {
- gen_ivtbl_mark(ivtbl);
+ rb_gc_mark(ivtbl);
}
}
@@ -1185,10 +1127,10 @@ void
rb_free_generic_ivar(VALUE obj)
{
st_data_t key = (st_data_t)obj;
- struct gen_ivtbl *ivtbl;
+ VALUE ivtbl;
- if (st_delete(generic_iv_tbl, &key, (st_data_t *)&ivtbl))
- xfree(ivtbl);
+ (void)st_delete(generic_iv_tbl, &key, (st_data_t *)&ivtbl);
+ /* rb_ary_clear is not suitable here since that can realloc, rely on GC */
if (generic_iv_tbl_compat) {
st_table *tbl;
@@ -1201,21 +1143,23 @@ rb_free_generic_ivar(VALUE obj)
RUBY_FUNC_EXPORTED size_t
rb_generic_ivar_memsize(VALUE obj)
{
- struct gen_ivtbl *ivtbl;
+ VALUE ivtbl;
if (gen_ivtbl_get(obj, &ivtbl))
- return gen_ivtbl_bytes(ivtbl->numiv);
+ return rb_ary_memsize(ivtbl);
return 0;
}
static size_t
-gen_ivtbl_count(const struct gen_ivtbl *ivtbl)
+gen_ivtbl_count(const VALUE ivtbl)
{
long i;
size_t n = 0;
+ long len = RARRAY_LEN(ivtbl);
+ const VALUE *ptr = RARRAY_CONST_PTR(ivtbl);
- for (i = 0; i < ivtbl->numiv; i++) {
- if (ivtbl->ivptr[i] != Qundef) {
+ for (i = 0; i < len; i++) {
+ if (ptr[i] != Qundef) {
n++;
}
}
@@ -1355,9 +1299,9 @@ generic_ivar_set(VALUE obj, ID id, VALUE val)
st_update(generic_iv_tbl, (st_data_t)obj, generic_ivar_update,
(st_data_t)&ivup);
- ivup.u.ivtbl->ivptr[ivup.index] = val;
+ rb_ary_store_fill(ivup.u.ivtbl, ivup.index, val, Qundef);
- if (FL_ABLE(obj)) RB_OBJ_WRITTEN(obj, Qundef, val);
+ if (FL_ABLE(obj)) RB_OBJ_WRITTEN(obj, Qundef, ivup.u.ivtbl);
}
VALUE
@@ -1485,7 +1429,8 @@ obj_ivar_each(VALUE obj, int (*func)(ANYARGS), st_data_t arg)
}
struct gen_ivar_tag {
- struct gen_ivtbl *ivtbl;
+ VALUE ivtbl;
+
int (*func)(ID key, VALUE val, st_data_t arg);
st_data_t arg;
};
@@ -1495,8 +1440,8 @@ gen_ivar_each_i(st_data_t key, st_data_t index, st_data_t data)
{
struct gen_ivar_tag *arg = (struct gen_ivar_tag *)data;
- if ((long)index < arg->ivtbl->numiv) {
- VALUE val = arg->ivtbl->ivptr[index];
+ if ((long)index < RARRAY_LEN(arg->ivtbl)) {
+ VALUE val = RARRAY_AREF(arg->ivtbl, index);
if (val != Qundef) {
return (arg->func)((ID)key, val, arg->arg);
}
@@ -1522,7 +1467,7 @@ gen_ivar_each(VALUE obj, int (*func)(ANYARGS), st_data_t arg)
struct givar_copy {
VALUE obj;
st_table *iv_index_tbl;
- struct gen_ivtbl *ivtbl;
+ VALUE ivtbl;
};
static int
@@ -1534,14 +1479,9 @@ gen_ivar_copy(ID id, VALUE val, st_data_t arg)
ivup.extended = 0;
ivup.u.iv_index_tbl = c->iv_index_tbl;
iv_index_tbl_extend(&ivup, id);
- if ((long)ivup.index >= c->ivtbl->numiv) {
- size_t newsize = iv_index_tbl_newsize(&ivup);
-
- c->ivtbl = gen_ivtbl_resize(c->ivtbl, newsize);
- }
- c->ivtbl->ivptr[ivup.index] = val;
+ rb_ary_store_fill(c->ivtbl, ivup.index, val, Qundef);
- if (FL_ABLE(c->obj)) RB_OBJ_WRITTEN(c->obj, Qundef, val);
+ if (FL_ABLE(c->obj)) RB_OBJ_WRITTEN(c->obj, Qundef, c->ivtbl);
return ST_CONTINUE;
}
@@ -1549,7 +1489,7 @@ gen_ivar_copy(ID id, VALUE val, st_data_t arg)
void
rb_copy_generic_ivar(VALUE clone, VALUE obj)
{
- struct gen_ivtbl *ivtbl;
+ VALUE ivtbl;
rb_check_frozen(clone);
@@ -1563,28 +1503,22 @@ rb_copy_generic_ivar(VALUE clone, VALUE obj)
}
if (gen_ivtbl_get(obj, &ivtbl)) {
struct givar_copy c;
- long i;
if (gen_ivtbl_count(ivtbl) == 0)
goto clear;
if (gen_ivtbl_get(clone, &c.ivtbl)) {
- for (i = 0; i < c.ivtbl->numiv; i++)
- c.ivtbl->ivptr[i] = Qundef;
+ rb_ary_clear(c.ivtbl);
}
else {
- c.ivtbl = gen_ivtbl_resize(0, ivtbl->numiv);
+ c.ivtbl = rb_ary_tmp_new(0);
FL_SET(clone, FL_EXIVAR);
+ st_add_direct(generic_iv_tbl, (st_data_t)clone, (st_data_t)c.ivtbl);
}
c.iv_index_tbl = iv_index_tbl_make(clone);
c.obj = clone;
gen_ivar_each(obj, gen_ivar_copy, (st_data_t)&c);
- /*
- * c.ivtbl may change in gen_ivar_copy due to realloc,
- * no need to free
- */
- st_insert(generic_iv_tbl, (st_data_t)clone, (st_data_t)c.ivtbl);
}
}
@@ -1638,7 +1572,7 @@ rb_ivar_count(VALUE obj)
break;
default:
if (FL_TEST(obj, FL_EXIVAR)) {
- struct gen_ivtbl *ivtbl;
+ VALUE ivtbl;
if (gen_ivtbl_get(obj, &ivtbl)) {
return gen_ivtbl_count(ivtbl);
--
EW
^ permalink raw reply related [flat|nested] only message in thread
only message in thread, other threads:[~2015-06-23 23:20 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-23 23:20 [PATCH] use Array instead of custom struct for generic ivars Eric Wong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).