From: Yusuke Endoh Date: 2011-05-31T23:24:22+09:00 Subject: [ruby-core:36633] [Ruby 1.9 - Bug #3924] Performance bug (in require?) Issue #3924 has been updated by Yusuke Endoh. Hello, 2011/5/31 Xavier Shay : > Patches: �� �� ��https://gist.github.com/58dbd6e72c1a1f47a415 Awesome! > In addition I have addressed the following concerns from Yusuke: > - Please use 4 space for indent, with 8 space tab. ��(Emacs-style) Your patch still seems to use 8-space (1-tab) indent. Yours: +static VALUE +rb_locate_file(VALUE filename) +{ + VALUE full_path = Qnil; ~~~~~~~~ 1 tab Expected: +static VALUE +rb_locate_file(VALUE filename) +{ + VALUE full_path = Qnil; ~~~~ 4 spaces But okay, this is a minor problem. We can fix it easily later. > Yusuke wrote: > "It is an impossible approach because some extension library can > modify $LOADED_FEATURES directly by rb_ary_push." > > The hooks are only used to keep a cache up to date. If we get a cache-miss, we could just revert to scanning $LOADED_FEATURES again which would pick up any items that were added with rb_ary_push. Sorry I cannot get your point. The following program should not load t.rb, should it? $"[$".size] = File.expand_path("./t.rb") require("./t") However, this actually load t.rb after your patch is applied: $ cat t.rb p :loaded $ ./ruby -I. -e ' $"[$".size] = File.expand_path("./t.rb") require("./t") ' :loaded This is considerable incompatiblity, I think. Of course, you can change LoadedFeaturesProxy to hook Array#[]=. But you cannot hook rb_ary_push. This is my concern. > This would also mean the addition of `lib/enumerator.rb` in my patch would be unnecessary (though IMO probably still a good idea). Yes, it is a good idea, and will be accepted eventually. But I'm afraid if it is not the timing to do so. And we should discuss it separately. > = Benchmarks > > Yusuke I think I mislead you with the benchmarks. I do not expect full_load_path_benchmark.rb to be faster. I was only using it to ensure I didn't make that case any worse. See below for further details. Okay, I got it. Sorry for my obtuseness. However, I don't think that the incompatibility is ignorable. At least, I have no right to decide it. Matz and Yugui, what do you think? -- Yusuke Endoh ---------------------------------------- Bug #3924: Performance bug (in require?) http://redmine.ruby-lang.org/issues/3924 Author: Carsten Bormann Status: Open Priority: Normal Assignee: Category: core Target version: 1.9.3 ruby -v: - =begin Running irb < /dev/null in 1.9.2 causes 3016 calls to lstat64. For instance, there is a sequence of 28 repetitions each of lstat calls to all 6 non-empty path prefixes of /opt/local/lib/ruby1.9/1.9.1/irb.rb -- a total of 170 lstats apparently just to load this file; another set of lstats then occurs later for another 18 (times 6) times. Clearly, something is running amok in the calling sequence rb_require_safe -> realpath_rec -> lstat. Another example: Running a simple test with the baretest gem causes 17008 calls to lstat. According to perftools.rb, 80 % of the 1.2 seconds of CPU is used in Kernel#gem_original_require (and another 12 in GC, some of which may be caused by this). =end -- http://redmine.ruby-lang.org