Project

General

Profile

Feature #11299

[PATCH] use Array instead of custom struct for generic ivars

Added by normalperson (Eric Wong) over 4 years ago. Updated over 4 years ago.

Status:
Open
Priority:
Normal
Target version:
-
[ruby-core:69714]

Description

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

Files

History

Updated by ko1 (Koichi Sasada) over 4 years ago

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?

Updated by matz (Yukihiro Matsumoto) over 4 years ago

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

Matz.

Updated by normalperson (Eric Wong) over 4 years ago

ko1@atdot.net 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.

Updated by normalperson (Eric Wong) over 4 years ago

matz@ruby-lang.org 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.

Also available in: Atom PDF