[ruby-core:53274] [ruby-trunk - Bug #8048] require() features_index bloats size of ruby heap

Issue #8048 has been updated by tmm1 (Aman Gupta).


> +    Check_Type(short_feature, T_STRING);
> +    short_feature_cstr = RSTRING_PTR(short_feature);

This should be StringValueCStr(), because short_feature is created from rb_str_substr().

Is it possible for features to contain a NUL byte? Maybe it is preferable to add st_init_bintable() which can accept a string length.
----------------------------------------
Bug #8048: require() features_index bloats size of ruby heap
https://bugs.ruby-lang.org/issues/8048#change-37448

Author: tmm1 (Aman Gupta)
Status: Closed
Priority: Normal
Assignee: 
Category: 
Target version: 
ruby -v: 2.0.0-p0


The new features_index for require() in 2.0.0 creates a lot of extra ARRAY and STRING objects. In a big rails application, there are ~70k extra strings and ~70k extra arrays on the heap after boot.

$ RAILS_ENV=production ruby -e' puts RUBY_VERSION; require "./config/environment"; p [:NUM_LOADED_FEATURES, $LOADED_FEATURES.size]; GC.start; p ObjectSpace.count_objects.select{ |k,v| [:T_STRING, :T_ARRAY, :T_HASH].include? k } '
1.9.3
[:NUM_LOADED_FEATURES, 2933]
{:T_STRING=>291147, :T_ARRAY=>97402, :T_HASH=>9439}

$ RAILS_ENV=production ruby -e' puts RUBY_VERSION; require "./config/environment"; p [:NUM_LOADED_FEATURES, $LOADED_FEATURES.size]; GC.start; p ObjectSpace.count_objects.select{ |k,v| [:T_STRING, :T_ARRAY, :T_HASH].include? k } '
2.0.0
[:NUM_LOADED_FEATURES, 2945]
{:T_STRING=>363183, :T_ARRAY=>167871, :T_HASH=>9640}


The following patch reduces the number of arrays used by the features_index:

diff --git a/load.c b/load.c
index ea22d5b..1868518 100644
--- a/load.c
+++ b/load.c
@@ -186,11 +186,16 @@ features_index_add_single(VALUE short_feature, VALUE offset)
 {
     VALUE features_index, this_feature_index;
     features_index = get_loaded_features_index_raw();
-    if ((this_feature_index = rb_hash_lookup(features_index, short_feature)) == Qnil) {
-	this_feature_index = rb_ary_new();
-	rb_hash_aset(features_index, short_feature, this_feature_index);
+    this_feature_index = rb_hash_lookup(features_index, short_feature);
+
+    if (this_feature_index == Qnil) {
+        rb_hash_aset(features_index, short_feature, offset);
+    } else if (RB_TYPE_P(this_feature_index, T_FIXNUM)) {
+        this_feature_index = rb_ary_new3(2, this_feature_index, offset);
+        rb_hash_aset(features_index, short_feature, this_feature_index);
+    } else if (RB_TYPE_P(this_feature_index, T_ARRAY)) {
+        rb_ary_push(this_feature_index, offset);
     }
-    rb_ary_push(this_feature_index, offset);
 }
 
 /* Add to the loaded-features index all the required entries for
@@ -389,8 +394,16 @@ rb_feature_p(const char *feature, const char *ext, int rb, int expanded, const c
        or ends in '/'.  This includes both match forms above, as well
        as any distractors, so we may ignore all other entries in `features`.
      */
-    for (i = 0; this_feature_index != Qnil && i < RARRAY_LEN(this_feature_index); i++) {
-	long index = FIX2LONG(rb_ary_entry(this_feature_index, i));
+    for (i = 0; this_feature_index != Qnil; i++) {
+	long index;
+	if (RB_TYPE_P(this_feature_index, T_ARRAY)) {
+	  if (i >= RARRAY_LEN(this_feature_index)) break;
+	  index = FIX2LONG(rb_ary_entry(this_feature_index, i));
+	} else {
+	  if (i > 0) break;
+	  index = FIX2LONG(this_feature_index);
+	}
+
 	v = RARRAY_PTR(features)[index];
 	f = StringValuePtr(v);
 	if ((n = RSTRING_LEN(v)) < len) continue;



-- 
http://bugs.ruby-lang.org/