[ruby-core:36558] [Ruby 1.9 - Bug #3924] Performance bug (in require?)

Issue #3924 has been updated by Xavier Shay.


Yusuke Endoh wrote:
> Hello, Xavier
> 
> Thank you for your contribution.  Your patch seems to attempt a good
> thing, but it is tremendously big and complex (for me) to review.
> Can you separate a sequence of small patches?  Note that "git log"
> is not what is wanted.
I will do this.

> 
> Some hints:
> 
> - Please try to minimize a patch.
>   - It would be good to use the existing code as possible as you can.
I realise this would be ideal, but I can't figure out how it works, or whether it would be compatible with the new algorithm.

>   - It would be good not to change the existing code without reason.
Agree, but I am hoping the speed up is sufficient reason. To alleviate I have tried to make the code easier to read than the old version.

> - Please use 4 space for indent, with 8 space tab.  (Emacs-style)
> - Please use C89.  Please don't use // comments.
> - Please don't export function without "rb_" prefix, to avoid symbol
>   conflicts.
I will do all of these.

> - Why did you remove enumerator.so from $" and add lib/enumerator.rb?
>   I'm afraid if it causes compatibility issue.
provide doesn't really make sense and is confusing. It's saying "Pretend this file has been loaded", and then we need to make allowances for files that don't really exist, which means lots of extra checking. Providing lib/enumerator.rb as a blank file obviates this problem.

> - It is slightly disturbing to define a new class with Array inherited.
>   Is it really needed?  Is there no alternative approach?
I agree and wish there was a good alternative. Unfortunately this is necessary because $LOADED_FEATURES is treated like an array by much existing code (particularly tests), but we need it as a case-insensitive hash for the new algorithm. Perhaps the hash could be exposed directly as a class (like CaseInsensitiveHash), but this means more code, is potentially more complicated, and means we are really changing the type of $LOADED_FEATURES. Thoughts?

> 
> Thanks,
> 
> -- 
> Yusuke Endoh <[e-mail]>


----------------------------------------
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