Hello,

I saw the patch sent by Nobu Nokada, however I think
that a better tradeoff can be found,

1) Why the nd_file info was kept outside GC, so far?
2) Why do we now have a problem if this string is not garbage
  collected?

99.9% of the cases, GC is overkill, because the script is kept
in memory, and we want to be always able to reference the context
in which exceptions were thrown.
In some rare situations (0.1% of the cases, it just surfaced
recently) the code is just a kind of data/control feed.
In that later case, I propose ignoring the script filename completely.

I made the assumption that the distinction between those two cases
could also match the one already existing between load(f) and load(f,true)
(the load under an anonymous module).

Therefore I wrote a little patch that keeps the existing behavior
(doesn't do GC for the nd_file field), and avoids memory leaks
in the case the file was loaded in a private space.

The following patch leads to 0 memory consumption on the
test case "demo-bug.rb" (see http://www.ruby-talk.org/28754),
but beside that, I haven't done extensive tests (like the Rubicon
for instance).

Hope it helps!

-- Christian


--------------- patch for the memory consumption on repetitive
load ---------

*** ruby-1.6.5/eval.c   Tue Sep 18 06:47:02 2001
--- ruby-1.6/eval.c     Fri Dec 21 11:33:32 2001
***************
*** 5205,5211 ****

        DEFER_INTS;
        ruby_in_eval++;
!       rb_load_file(RSTRING(fname)->ptr);
        ruby_in_eval--;
        node = ruby_eval_tree;
        ALLOW_INTS;
--- 5205,5214 ----

        DEFER_INTS;
        ruby_in_eval++;
!       if (wrap)
!           rb_load_file_anon(RSTRING(fname)->ptr);
!       else
!           rb_load_file(RSTRING(fname)->ptr);
        ruby_in_eval--;
        node = ruby_eval_tree;
        ALLOW_INTS;
*** ruby-1.6.5/intern.h Thu Sep  6 10:42:22 2001
--- ruby-1.6/intern.h   Fri Dec 21 11:22:03 2001
***************
*** 286,291 ****
--- 286,292 ----
  EXTERN VALUE rb_argv;
  EXTERN VALUE rb_argv0;
  void rb_load_file _((char*));
+ void rb_load_file_anon _((char*));
  void ruby_script _((char*));
  void ruby_prog_init _((void));
  void ruby_set_argv _((int, char**));
*** ruby-1.6.5/parse.c  Mon Sep  3 08:38:16 2001
--- ruby-1.6/parse.c    Fri Dec 21 12:19:44 2001
***************
*** 5326,5332 ****
      lex_pbeg = lex_p = lex_pend = 0;
      ruby_sourceline = start - 1;

!     return yycompile(strdup(f), start);
  }

  static inline int
--- 5326,5334 ----
      lex_pbeg = lex_p = lex_pend = 0;
      ruby_sourceline = start - 1;

!     return yycompile(f, start);
  }

  static inline int
*** ruby-1.6.5/ruby.c   Wed Sep  5 09:52:18 2001
--- ruby-1.6/ruby.c     Fri Dec 21 12:20:17 2001
***************
*** 765,771 ****
  #endif
      }

!     if (script) {
        VALUE c = 1;            /* something not nil */
        VALUE line;
        char *p;
--- 765,771 ----
  #endif
      }

!     if (script == 1) {
        VALUE c = 1;            /* something not nil */
        VALUE line;
        char *p;
***************
*** 843,850 ****
        require_libraries();    /* Why here? unnatural */
        if (NIL_P(c)) return;
      }
      rb_compile_file(fname, f, line_start);
!     if (script && ruby__end__seen) {
        rb_define_global_const("DATA", f);
      }
      else if (f != rb_stdin) {
--- 843,854 ----
        require_libraries();    /* Why here? unnatural */
        if (NIL_P(c)) return;
      }
+     if (script == -1)
+       fname = "(anon)";
+     else
+       fname = strdup(fname);
      rb_compile_file(fname, f, line_start);
!     if (script == 1 && ruby__end__seen) {
        rb_define_global_const("DATA", f);
      }
      else if (f != rb_stdin) {
***************
*** 857,863 ****
--- 861,876 ----
  rb_load_file(fname)
      char *fname;
  {
+     fprintf( stderr, "CB: load:  %s\n", fname );
      load_file(fname, 0);
+ }
+
+ void
+ rb_load_file_anon(fname)
+     char *fname;
+ {
+     load_file(fname, -1);
  }

  static void


--------------- end of
patch -------------------------------------------------




> -----Original Message-----
> From: nobu.nokada / softhome.net [mailto:nobu.nokada / softhome.net]
> Sent: Friday, December 21, 2001 9:35 AM
> To: ruby-talk ML
> Subject: [ruby-talk:29209] Re: Constant loss of memory with Kernel::load
> in a loop
>
>
> At Fri, 21 Dec 2001 15:57:16 +0900,
> matz / ruby-lang.org (Yukihiro Matsumoto) wrote:
> > |> The original problem iirc was about repetitively loading the
> same file
> > |> over and over again.
> > |> If one refers to the "intern"ed version of that filename, wouldn't it
> > |> always refer to the same string?
> > |
> > |Yes.  It's similar to my proposal except using a particular
> > |table instead of one for Symbol.
> >
> > [ruby-talk:28803]? I don't think so.  Your patch scans filename
> > strings.  It's complete, but slow.
>
> No, later [ruby-talk:29152].  I didn't post the patch.
>
> > Christensen's proposal is
> > cheaper.  But I'm not sure if it's effective enough.  For example, it
> > does not prevent leaks like:
>
> Sure, although I didn't want to point out it :-).  It's only
> problem there's no way to know when filenames get unreferred.
>
> The only way I could think of is making nd_file a String VALUE
> and marking them directly from each NODE's.  It prevents leaks
> but doesn't make GC notably slow.  Though it may be a big
> modification.
>
>
> Nobu Nakada
>
>