Feature #11299: [PATCH] use Array instead of custom struct for generic ivars

• Status: Open
• Priority: Normal
• Assignee: normalperson (Eric Wong)
• Target version: -

[normalperson (Eric Wong)]

I'll commit this in a few days unless there's an objection.

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    |  28 +++++-----
 internal.h |   1 +
 variable.c | 170 +++++++++++++++++++------------------------------------------
 3 files changed, 69 insertions(+), 130 deletions(-)

* 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

[ko1 (Koichi Sasada)]

I'm weakly against this proposal because it will consumes more objects and increase GC pressure.
I don't think it is valuable compare with reducing binary size by 1KB.

BTW,

-    if (FL_ABLE(obj)) RB_OBJ_WRITTEN(obj, Qundef, val);
+    if (FL_ABLE(obj)) RB_OBJ_WRITTEN(obj, Qundef, ivup.u.ivtbl);

Can obj which is !FL_ABLE(obj) be here?

[matz (Yukihiro Matsumoto)]

I agree with Ko1 here. GC pressure from object allocation is far higher than the pressure from byte allocation.

Matz.

[normalperson (Eric Wong)]

[e-mail] wrote:

I'm weakly against this proposal because it will consumes more objects
and increase GC pressure.

Existing code with xrealloc still has GC pressure, too.

I don't think it is valuable compare with reducing binary size by 1KB.

Right, binary size is minor, but I'm happy with the LoC reduction.

-    if (FL_ABLE(obj)) RB_OBJ_WRITTEN(obj, Qundef, val);
+    if (FL_ABLE(obj)) RB_OBJ_WRITTEN(obj, Qundef, ivup.u.ivtbl);

Can obj which is !FL_ABLE(obj) be here?

Seems not. I missed that check in r50758
("variable.c: remove generic ivar support for special constants")
Will remove separately.

[normalperson (Eric Wong)]

[e-mail] wrote:

I agree with Ko1 here. GC pressure from object allocation is far
higher than the pressure from byte allocation.

This depends on the application, of course; and I expect GC will improve.

Reducing code and improving readability is more important, I think.