https://redmine.ruby-lang.org/
https://redmine.ruby-lang.org/favicon.ico?1711330511
2010-10-21T17:47:23Z
Ruby Issue Tracking System
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=13859
2010-10-21T17:47:23Z
naruse (Yui NARUSE)
naruse@airemix.jp
<ul><li><strong>Target version</strong> set to <i>1.9.3</i></li></ul><p>=begin<br>
As you know, it's a problem from gem's require.<br>
It is planed that 1.9.3 fixes this.<br>
=end</p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=13860
2010-10-21T23:18:38Z
luislavena (Luis Lavena)
luislavena@gmail.com
<ul></ul><p>=begin<br>
Yui,</p>
<p>Are you sure about this? because even disabling RubyGems and removing everything from $LOAD_PATH the IO operations performed by Ruby for a simple require are too much.</p>
<p>Ruby 1.8 also suffered from excessive IO operations even without RubyGems. I can provide Windows trace logs of these items if you want.<br>
=end</p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=16972
2011-05-15T15:39:11Z
xaviershay (Xavier Shay)
xavier-list@rhnh.net
<ul></ul><p>Progress update time!</p>
<p>tl;dr - I've made the performance linear, still need to do a bit more clean up though.</p>
<p>I have started switching out the current loop of $LOADED_FEATURES to use a hash look up (using st.h) in order to fix this problem. You can try it now by using <code>require_2</code> rather than <code>require</code> on my fork (or by changing the mapping of require at the bottom of <code>load.c</code>):<br>
<a href="https://github.com/xaviershay/ruby/compare/require-performance-fix" class="external">https://github.com/xaviershay/ruby/compare/require-performance-fix</a></p>
<p>This fork passes all ruby tests and rubyspec require tests. It can also load a rails stack. That doesn't mean it is correct (the tests are not comprehensive), but it's a start.</p>
<p>It displays near-linear performance. Here are some graphs:<br>
versus 1.9 - <a href="https://skitch.com/xaviershay/r9pwb/tc-load-time" class="external">https://skitch.com/xaviershay/r9pwb/tc-load-time</a><br>
versus 1.8 - <a href="https://skitch.com/xaviershay/r9pwn/tc-load-time" class="external">https://skitch.com/xaviershay/r9pwn/tc-load-time</a></p>
<p>This is a pretty big patch, and also my first on MRI so there are a few potential issues:</p>
<ul>
<li>This is pretty much a rewrite for load.c, since I couldn't get my head around <code>rb_feature_p</code>. That means it's risky. It is likely possibly to add a hash-table lookup to the existing architecture, but someone else would have to volunteer to do this. On the up side, it is far easier to follow the algorithm in the new code.</li>
<li>It doesn't use the file search functions in <code>file.c</code>, so I suspect there could be some safe level issues not covered yet.</li>
<li>I have made public one or two functions in <code>file.c</code> and <code>array.c</code>. Pretty sure I could rewrite what I have to avoid doing this.</li>
<li>
<code>rb_provide</code> no longer makes sense. It was being used in <code>enumerator.c</code> but I found another method of providing backwards compatility. Since it is in <code>intern.h</code>, it can maybe just be removed?</li>
<li>I use a proxy class for $LOADED_FEATURES to keep the loaded features hash up to date. Bit of a hack but I couldn't come up with a nicer way of doing it.</li>
<li>Is it OK to add a new member to the VM struct? I added new_loaded_features (to be renamed shortly).</li>
<li>Not tested on windows yet.</li>
</ul>
<p>Outstanding tasks still on my list (hopefully will be able to do in the next week):</p>
<ul>
<li>Autoload still uses the old rb_feature_provided.</li>
<li>It is much faster on synthetic benchmarks, but still slower on many real world cases because I have only optimized the general algorithm so far and still am using a lot of <code>rb_funcall</code>s that are not necessary.</li>
<li>I haven't marked all my methods as static yet.</li>
<li>Remove old require code once I'm satisfied the new code works.</li>
<li>Clean up the vm->new_loaded_features stuff.</li>
</ul>
<p>Re the original ticket description, I saw the <code>lstats</code> in profiling too but believe it is a symptom not a cause.</p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=16987
2011-05-16T07:31:12Z
mgenereu (Michael Genereux)
mgenereu@gmail.com
<ul></ul><p>Yes on the red herring of lstat. I removed the recursive lstat'ing (it's checking the path for symbolic links) and there was no speed increase.</p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=16989
2011-05-16T11:38:24Z
xaviershay (Xavier Shay)
xavier-list@rhnh.net
<ul></ul><p>Progress update:</p>
<p>Have done general cleanup to make the code more readable, including marking local methods as static. Optimized a lot of my code that was previously using rb_funcall, now it actually does things properly in C.</p>
<p>I have switched autoload over to the new system, but there is still a problem with it - ActiveRecord doesn't load properly due to a constant missing which should be autoloaded. Not sure what is causing this.</p>
<p>There is a failing test in rubyspec for autoload which I thought might be related, but it appears to be broken against ruby trunk also.</p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=17110
2011-05-20T19:28:31Z
xaviershay (Xavier Shay)
xavier-list@rhnh.net
<ul></ul><p>Progress update:</p>
<p>Fixed the autoload problem, my fork now loads a moderate sized rails app in the same time as ruby 1.9.2 (and far faster that 1.9.3dev 20s compared to 46s), as well as passing all other tests/specs. This proves the algorithm I think, the next step is to optimize the inner functions (such as rb_locate_file_with_extensions), since they still use plenty of rb_funcall to do string manipulation which could be far faster done in C. There is also room for improvement by caching rb_locate_file.</p>
<p>Code is still at <a href="https://github.com/xaviershay/ruby/tree/require-performance-fix" class="external">https://github.com/xaviershay/ruby/tree/require-performance-fix</a></p>
<p>I hope to have more time to work on it on Sunday.</p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=17144
2011-05-22T15:33:24Z
xaviershay (Xavier Shay)
xavier-list@rhnh.net
<ul></ul><p>Progress update:</p>
<p>Didn't get much time to work on this today, but came up with a benchmark to measure performance with a full $LOAD_PATH (suspected issue with slow rails loading), which indeed shows mine to have the same performance curve as 1.9.3, but with a higher multiplier:<br>
<a href="https://gist.github.com/985224" class="external">https://gist.github.com/985224</a> (graph in the comments)</p>
<p>Doubling the number of available extensions (.bogus, .bogus2, etc...) doubled the multiplier, suggesting improvement in rb_locate_file_with_extensions could be a win.</p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=17171
2011-05-23T19:56:22Z
xaviershay (Xavier Shay)
xavier-list@rhnh.net
<ul><li><strong>File</strong> <a href="/attachments/1733">require_performance.diff</a> <a class="icon-only icon-download" title="Download" href="/attachments/download/1733/require_performance.diff">require_performance.diff</a> added</li></ul><p>Progress update:</p>
<p>Didn't make much impact on the graph in prior update, but also may have been to eager to blame slow rails loading on it - rails only has ~50 entries in the load path.<br>
I added a cache to file expansion (so it remembers the expanded path of 'active_record" without having to search for it), which gave a ~0.5s speed up on a blank rails app, and ~6s on a medium sized one. This patch is now faster than 1.9.2 in all benchmarks I have, and <em>way</em> faster than 1.9.3dev (though it has been that for a while). I suspect there are further optimizations that could be made, especially now the code is easier to follow.</p>
<p>I have attached a diff. It passes <code>make test</code>, <code>make test-all</code>, and a few big rails test suites. There are still some outstanding issues on which need discussion.</p>
<ol>
<li>This is pretty much a rewrite for load.c, since I couldn't get my head around <code>rb_feature_p</code>. That means it's risky. It is likely possibly to add a hash-table lookup to the existing architecture, but someone else would have to volunteer to do this. On the up side, it is far easier to follow the algorithm in the new code.</li>
<li>It doesn't use the file search functions in <code>file.c</code>, so I suspect there could be some safe level issues not covered yet.</li>
<li>I have made public (non-static) one or two functions in <code>file.c</code> and <code>array.c</code>. Is this OK? Pretty sure I could rewrite to avoid doing this if necessary.</li>
<li>
<code>rb_provide</code> doesn't really make sense. It was being used in <code>enumerator.c</code> but I found another method of providing backwards compatibility. Since it is in <code>intern.h</code>, it can maybe just be removed?</li>
<li>I use a proxy class for $LOADED_FEATURES to keep the loaded features hash up to date. Is this OK? Bit of a hack but I couldn't come up with a nicer way of doing it.</li>
<li>Is it OK to add a new member to the VM struct? I added new_loaded_features (to be renamed shortly).</li>
<li>Not tested on windows yet.</li>
<li>The code would be nicer if some related functions could be moved to another file (rb_locate_*), I don't know the best way to do this though.</li>
</ol>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=17172
2011-05-23T20:47:12Z
xaviershay (Xavier Shay)
xavier-list@rhnh.net
<ul></ul><p>It appears this commit:<br>
<a href="https://github.com/xaviershay/ruby/commit/d7d6c41524ec8bcda8d6672e7d8b7ae812abc239" class="external">https://github.com/xaviershay/ruby/commit/d7d6c41524ec8bcda8d6672e7d8b7ae812abc239</a></p>
<p>breaks make clean && make with this error:<br>
<a href="https://gist.github.com/cb99899d0918a840ab76" class="external">https://gist.github.com/cb99899d0918a840ab76</a></p>
<p>I have no idea why. Perhaps switching over require to the new code? But the compile is failing still in C land... Suggestions welcome.</p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=17181
2011-05-24T10:29:06Z
normalperson (Eric Wong)
normalperson@yhbt.net
<ul></ul><p>I'll try to dedicate some time to helping on this but I can't promise<br>
anything. I'll take a longer look at Xavier's work later tonight or<br>
tomorrow.</p>
<p>However, I needed the following trivial patch to build with gcc 4.4.5<br>
on Debian:</p>
<p>diff --git a/variable.c b/variable.c<br>
index 551a7d5..908a649 100644<br>
--- a/variable.c<br>
+++ b/variable.c<br>
@@ -19,6 +19,7 @@<br>
#include "constant.h"<br>
#include "internal.h"</p>
<p>+VALUE rb_feature_provided_2(VALUE); /* move to ruby/intern.h ? */<br>
void rb_vm_change_state(void);<br>
void rb_vm_inc_const_missing_count(void);</p>
<p>--<br>
Eric Wong</p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=17183
2011-05-24T16:42:03Z
xaviershay (Xavier Shay)
xavier-list@rhnh.net
<ul></ul><p>I figured out how to make my branch compile (see my last comment), but I have no idea why I have to do this or why it works. It's a hack workaround, not a real fix. I can compile trunk no worries:</p>
<a name="First-few-steps-the-same-as-normal"></a>
<h1 >First few steps the same as normal<a href="#First-few-steps-the-same-as-normal" class="wiki-anchor">¶</a></h1>
<p>cd /path/to/source/directory<br>
autoconf<br>
cd /path/to/build/directory<br>
/path/to/source/directory/configure</p>
<a name="HACKS-follow"></a>
<h1 >HACKS follow<a href="#HACKS-follow" class="wiki-anchor">¶</a></h1>
<a name="The-parenthesis-arent-escaped-in-this-file-which-causes-a-bash-syntax-error"></a>
<h1 >The parenthesis aren't escaped in this file, which causes a bash syntax error.<a href="#The-parenthesis-arent-escaped-in-this-file-which-causes-a-bash-syntax-error" class="wiki-anchor">¶</a></h1>
<p>sed -i '' s/INIT_FUNCS(X)/INIT_FUNCS\\(X\\)/ ext/-test-/string/Makefile</p>
<a name="-e-picks-up-variables-from-my-environment-top_srcdir-isnt-set-properly-otherwise"></a>
<h1 >-e picks up variables from my environment, top_srcdir isn't set properly otherwise<a href="#-e-picks-up-variables-from-my-environment-top_srcdir-isnt-set-properly-otherwise" class="wiki-anchor">¶</a></h1>
<p>make -e top_srcdir=/path/to/source/directory</p>
<p>I have no experience with Makefiles/configure, can anyone suggest what might be causing this problem?</p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=17282
2011-05-28T10:11:15Z
xaviershay (Xavier Shay)
xavier-list@rhnh.net
<ul></ul><p>My last problem was related to the build system pushing things into $" and bypassing require.</p>
<p>I now have a patch that I woud like to submit for consideration:<br>
<a href="https://gist.github.com/996418" class="external">https://gist.github.com/996418</a></p>
<p>I am going to try and get some more eyeballs on this to test it more thoroughly:<br>
<a href="http://rhnh.net/2011/05/28/speeding-up-rails-startup-time" class="external">http://rhnh.net/2011/05/28/speeding-up-rails-startup-time</a></p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=17287
2011-05-28T22:23:07Z
rosenfeld (Rodrigo Rosenfeld Rosas)
rr.rosas@gmail.com
<ul></ul><p>Great work, Xavier!</p>
<p>I would like to try it but my RVM installation seems to be broken somehow:</p>
<p>rvm install ruby-head --patch /tmp/require-performance-fix.patch</p>
<p>Installing Ruby from source to:<br>
/home/rodrigo/.rvm/rubies/ruby-1.8.7-p334, this may take a while<br>
depending on your cpu(s)...</p>
<p>I don't know why it insists on 1.8.7, but I'll try it manually as soon<br>
as I have some time, thanks!</p>
<p>I'll let you know the feedbacks.</p>
<p>Regards,</p>
<p>Rodrigo.</p>
<p>Em 27-05-2011 22:11, Xavier Shay escreveu:</p>
<blockquote>
<p>Issue <a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Bug: Performance bug (in require?) (Closed)" href="https://redmine.ruby-lang.org/issues/3924">#3924</a> has been updated by Xavier Shay.</p>
<p>My last problem was related to the build system pushing things into $" and bypassing require.</p>
<p>I now have a patch that I woud like to submit for consideration:<br>
<a href="https://gist.github.com/996418" class="external">https://gist.github.com/996418</a></p>
<h2>I am going to try and get some more eyeballs on this to test it more thoroughly:<br>
<a href="http://rhnh.net/2011/05/28/speeding-up-rails-startup-time" class="external">http://rhnh.net/2011/05/28/speeding-up-rails-startup-time</a>
</h2>
<p>Bug <a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Bug: Performance bug (in require?) (Closed)" href="https://redmine.ruby-lang.org/issues/3924">#3924</a>: Performance bug (in require?)<br>
<a href="http://redmine.ruby-lang.org/issues/3924" class="external">http://redmine.ruby-lang.org/issues/3924</a></p>
<p>Author: Carsten Bormann<br>
Status: Open<br>
Priority: Normal<br>
Assignee:<br>
Category: core<br>
Target version: 1.9.3<br>
ruby -v: ruby 1.9.2p0 (2010-08-18 revision 29036) [x86_64-darwin10]</p>
<p>=begin<br>
Running irb< /dev/null in 1.9.2 causes 3016 calls to lstat64.</p>
<p>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.</p>
<p>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).<br>
=end</p>
</blockquote>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=17291
2011-05-29T01:23:08Z
luislavena (Luis Lavena)
luislavena@gmail.com
<ul><li><strong>ruby -v</strong> changed from <i>ruby 1.9.2p0 (2010-08-18 revision 29036) [x86_64-darwin10] </i> to <i>-</i></li></ul><p>On Fri, May 27, 2011 at 9:11 PM, Xavier Shay <a href="mailto:xavier-list@rhnh.net" class="email">xavier-list@rhnh.net</a> wrote:</p>
<blockquote>
<p>My last problem was related to the build system pushing things into $" and bypassing require.</p>
<p>I now have a patch that I woud like to submit for consideration:<br>
<a href="https://gist.github.com/996418" class="external">https://gist.github.com/996418</a></p>
</blockquote>
<p>Windows 7, x64, building with GCC 4.5.2 (32bits).</p>
<p>Patch applied to r31761, with the following warnings:</p>
<p>C:\Users\Luis\Projects\oss\ruby>git apply require-performance-fix-r31758.patch<br>
require-performance-fix-r31758.patch:115: trailing whitespace.<br>
/*<br>
require-performance-fix-r31758.patch:128: trailing whitespace.<br>
/*<br>
require-performance-fix-r31758.patch:648: trailing whitespace.<br>
basename</p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=17292
2011-05-29T01:53:24Z
jonforums (Jon Forums)
<ul></ul><p><a class="user active user-mention" href="https://redmine.ruby-lang.org/users/2336">@luis (Luis Lopez)</a>, if you have time, would you add results to your gist using a patched ruby_1_9_2 build?</p>
<p>Your 1.9.3dev results on mingw32 are amazing...it would be great to see if the Xavier's patch makes sense to backport before Yugui releases an updated 1.9.2</p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=17293
2011-05-29T01:57:20Z
luislavena (Luis Lavena)
luislavena@gmail.com
<ul></ul><p>Jon Forums wrote:</p>
<blockquote>
<p><a class="user active user-mention" href="https://redmine.ruby-lang.org/users/2336">@luis (Luis Lopez)</a>, if you have time, would you add results to your gist using a patched ruby_1_9_2 build?</p>
</blockquote>
<p>Patch doesn't apply against 1.9.2:</p>
<p>C:\Users\Luis\Projects\oss\ruby>git apply require-performance-fix-r31758.patch<br>
require-performance-fix-r31758.patch:115: trailing whitespace.<br>
/*<br>
require-performance-fix-r31758.patch:128: trailing whitespace.<br>
/*<br>
require-performance-fix-r31758.patch:648: trailing whitespace.<br>
basename = rb_funcall(rb_cFile, rb_intern("basename"), 2,<br>
require-performance-fix-r31758.patch:652: trailing whitespace.<br>
file_name_with_extension = rb_funcall(rb_cFile, rb_intern("join"), 2,<br>
require-performance-fix-r31758.patch:728: trailing whitespace.<br>
* This is the most common case, so optimize for it. If not found, fall<br>
warning: enc/make_encmake.rb has type 100644, expected 100755<br>
error: patch failed: enc/make_encmake.rb:3<br>
error: enc/make_encmake.rb: patch does not apply<br>
warning: ext/extmk.rb has type 100644, expected 100755<br>
error: patch failed: load.c:68<br>
error: load.c: patch does not apply<br>
error: patch failed: test/ruby/test_require.rb:339<br>
error: test/ruby/test_require.rb: patch does not apply<br>
error: patch failed: variable.c:19<br>
error: variable.c: patch does not apply<br>
error: patch failed: vm_core.h:324<br>
error: vm_core.h: patch does not apply</p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=17295
2011-05-29T06:37:59Z
xaviershay (Xavier Shay)
xavier-list@rhnh.net
<ul></ul><p>Rodrigo: Please try the latest version of rvm: rvm get head. Otherwise let me know what platform you are on.<br>
Luis: Sorry I'm not sure what versions I ran my benchmarks against. Poor form on my part, though there was a recent commit on May 23rd which I would imagine sped things up: r31712, 7d20942bf6dd54cf54d12d4c9f9dc86b9a813bb4</p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=17296
2011-05-29T06:39:57Z
xaviershay (Xavier Shay)
xavier-list@rhnh.net
<ul></ul><p>Actually Luis I just read your gist, my numbers match up with ruby 1.9.3dev (2011-05-28 trunk 31761) [i386-mingw32], I never saw it as bad as 52s for r31496.</p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=17304
2011-05-29T21:18:09Z
mame (Yusuke Endoh)
mame@ruby-lang.org
<ul></ul><p>Hello, Xavier</p>
<p>Thank you for your contribution. Your patch seems to attempt a good<br>
thing, but it is tremendously big and complex (for me) to review.<br>
Can you separate a sequence of small patches? Note that "git log"<br>
is not what is wanted.</p>
<p>Some hints:</p>
<ul>
<li>Please try to minimize a patch.
<ul>
<li>It would be good to use the existing code as possible as you can.</li>
<li>It would be good not to change the existing code without reason.</li>
</ul>
</li>
<li>Please use 4 space for indent, with 8 space tab. (Emacs-style)</li>
<li>Please use C89. Please don't use // comments.</li>
<li>Please don't export function without "rb_" prefix, to avoid symbol<br>
conflicts.</li>
<li>Why did you remove enumerator.so from $" and add lib/enumerator.rb?<br>
I'm afraid if it causes compatibility issue.</li>
<li>It is slightly disturbing to define a new class with Array inherited.<br>
Is it really needed? Is there no alternative approach?</li>
</ul>
<p>Thanks,</p>
<p>--<br>
Yusuke Endoh <a href="mailto:mame@tsg.ne.jp" class="email">mame@tsg.ne.jp</a></p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=17316
2011-05-30T00:29:07Z
rosenfeld (Rodrigo Rosenfeld Rosas)
rr.rosas@gmail.com
<ul></ul><p>Em 28-05-2011 18:38, Xavier Shay escreveu:</p>
<blockquote>
<p>Issue <a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Bug: Performance bug (in require?) (Closed)" href="https://redmine.ruby-lang.org/issues/3924">#3924</a> has been updated by Xavier Shay.</p>
<p>Rodrigo: Please try the latest version of rvm: rvm get head. Otherwise let me know what platform you are on.</p>
</blockquote>
<p>Hi Xavier, I'm always on latest RVM. I don't know yet why ruby-head is<br>
resolving to 1.8.7-p334... But I don't intend to debug RVM for now. I'll<br>
try to compile Ruby manually when I have some time...</p>
<p>I'm on Debian unstable.</p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=17318
2011-05-30T00:53:07Z
rosenfeld (Rodrigo Rosenfeld Rosas)
rr.rosas@gmail.com
<ul></ul><p>Em 29-05-2011 12:23, Rodrigo Rosenfeld Rosas escreveu:</p>
<blockquote>
<p>Em 28-05-2011 18:38, Xavier Shay escreveu:</p>
<blockquote>
<p>Issue <a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Bug: Performance bug (in require?) (Closed)" href="https://redmine.ruby-lang.org/issues/3924">#3924</a> has been updated by Xavier Shay.</p>
<p>Rodrigo: Please try the latest version of rvm: rvm get head.<br>
Otherwise let me know what platform you are on.</p>
</blockquote>
<p>Hi Xavier, I'm always on latest RVM. I don't know yet why ruby-head is<br>
resolving to 1.8.7-p334... But I don't intend to debug RVM for now.<br>
I'll try to compile Ruby manually when I have some time...</p>
<p>I'm on Debian unstable.</p>
</blockquote>
<p>Never mind. It seems the problem is that I tried installing 1.8.7 once<br>
but couldn't due to this issue:</p>
<p><a href="http://redmine.ruby-lang.org/issues/4556" class="external">http://redmine.ruby-lang.org/issues/4556</a></p>
<p>Then, ruby-head was always resolving to it. Now that I've applied the<br>
patch provided on issue manually to ~/.rvm/src, I could install 1.8.7<br>
(needed for developing Gitorious). After installing it, ruby-head is<br>
resolving correctly. I'll try it and let you know.</p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=17323
2011-05-30T07:42:02Z
xaviershay (Xavier Shay)
xavier-list@rhnh.net
<ul></ul><p>Yusuke Endoh wrote:</p>
<blockquote>
<p>Hello, Xavier</p>
<p>Thank you for your contribution. Your patch seems to attempt a good<br>
thing, but it is tremendously big and complex (for me) to review.<br>
Can you separate a sequence of small patches? Note that "git log"<br>
is not what is wanted.<br>
I will do this.</p>
</blockquote>
<blockquote>
<p>Some hints:</p>
<ul>
<li>Please try to minimize a patch.
<ul>
<li>It would be good to use the existing code as possible as you can.<br>
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.</li>
</ul>
</li>
</ul>
</blockquote>
<blockquote>
<ul>
<li>It would be good not to change the existing code without reason.<br>
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.</li>
</ul>
</blockquote>
<blockquote>
<ul>
<li>Please use 4 space for indent, with 8 space tab. (Emacs-style)</li>
<li>Please use C89. Please don't use // comments.</li>
<li>Please don't export function without "rb_" prefix, to avoid symbol<br>
conflicts.<br>
I will do all of these.</li>
</ul>
</blockquote>
<blockquote>
<ul>
<li>Why did you remove enumerator.so from $" and add lib/enumerator.rb?<br>
I'm afraid if it causes compatibility issue.<br>
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.</li>
</ul>
</blockquote>
<blockquote>
<ul>
<li>It is slightly disturbing to define a new class with Array inherited.<br>
Is it really needed? Is there no alternative approach?<br>
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?</li>
</ul>
</blockquote>
<blockquote>
<p>Thanks,</p>
<p>--<br>
Yusuke Endoh <a href="mailto:mame@tsg.ne.jp" class="email">mame@tsg.ne.jp</a></p>
</blockquote>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=17335
2011-05-30T16:33:54Z
shyouhei (Shyouhei Urabe)
shyouhei@ruby-lang.org
<ul></ul><p>=begin</p>
<p>Hi Xavier. I'm also reading your patch.</p>
<ul>
<li>
<p>Though the patch is big, after applying it's super readable than the<br>
original. Fairly good.</p>
<ul>
<li>I also like your writing static function declarations.</li>
</ul>
</li>
<li>
<p>Style things are already pointed out by Yusuke so I don't repeat.</p>
</li>
<li>
<p>As for the enumerator thing, I think it's worth considering your<br>
idea. That enumerator hack is for backward compatibility, so in a<br>
future when we don't need that hack anymore, we can just delete that<br>
lib/enumerator.rb. I think it's cool.</p>
</li>
<li>
<p>I'm not sure if your code is ready for case-insensitive filesystems<br>
like those in Windows. I'm sure you know the problem so I should<br>
have missed something.</p>
</li>
</ul>
<p>=end</p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=17346
2011-05-30T22:59:06Z
mame (Yusuke Endoh)
mame@ruby-lang.org
<ul></ul><p>Hello,</p>
<p>2011/5/30 Xavier Shay <a href="mailto:xavier-list@rhnh.net" class="email">xavier-list@rhnh.net</a>:</p>
<blockquote>
<blockquote>
<ul>
<li>Please try to minimize a patch.<br>
 - It would be good to use the existing code as possible as you can.<br>
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.</li>
</ul>
</blockquote>
</blockquote>
<p>Indeed, load.c is very complex, but I believe that this is because<br>
load.c is an accumulation of bad know-hows in the long history.<br>
So we should not throw it away without understanding, I think.</p>
<blockquote>
<blockquote>
<ul>
<li>It is slightly disturbing to define a new class with Array inherited.<br>
 Is it really needed?  Is there no alternative approach?<br>
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?</li>
</ul>
</blockquote>
</blockquote>
<p>Aha, I understand why the inheritance is used. It is to hook the<br>
array modification, right?</p>
<p>It is an impossible approach because some extension library can<br>
modify $LOADED_FEATURES directly by rb_ary_push.<br>
I found amalgalite to do so actually:</p>
<p><a href="http://www.google.com/codesearch/p?hl" class="external">http://www.google.com/codesearch/p?hl</a></p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=17348
2011-05-31T00:23:19Z
mame (Yusuke Endoh)
mame@ruby-lang.org
<ul></ul><p>Hello, Xavier</p>
<p><a href="http://rhnh.net/2011/05/28/speeding-up-rails-startup-time" class="external">http://rhnh.net/2011/05/28/speeding-up-rails-startup-time</a></p>
<p>In the above article, you said that the bottleneck is to find<br>
$LOADED_FEATURES in linear, but I doubt it. If you are right,<br>
1.8 should be also slow because 1.8 uses the same algorithm.<br>
Thus, I guess there is another bottleneck.</p>
<p>I tried your patch and test in <a href="https://gist.github.com/985224" class="external">https://gist.github.com/985224</a><br>
on Ubuntu.<br>
I could confirm that trunk is twice slower than 1.9.2p180.<br>
But I couldn't confirm that the speed up by your patch.<br>
I also performed the same benchmark after "gem install rails",<br>
but there is no difference.</p>
<a name="192p180"></a>
<h1 >1.9.2p180<a href="#192p180" class="wiki-anchor">¶</a></h1>
<p>$ ruby -v full_load_path_benchmark.rb<br>
ruby 1.9.2p180 (2011-02-18 revision 30909) [i686-linux]<br>
user system total real<br>
0 in load path 0.000000 0.000000 0.000000 ( 0.003764)<br>
500 in load path 0.040000 0.040000 0.080000 ( 0.081142)<br>
1000 in load path 0.060000 0.100000 0.160000 ( 0.159599)<br>
1500 in load path 0.130000 0.100000 0.230000 ( 0.235893)<br>
2000 in load path 0.120000 0.200000 0.320000 ( 0.317750)</p>
<a name="trunk"></a>
<h1 >trunk<a href="#trunk" class="wiki-anchor">¶</a></h1>
<p>$ ../ruby.org -v full_load_path_benchmark.rb<br>
ruby 1.9.3dev (2011-05-30 trunk 31824) [i686-linux]<br>
user system total real<br>
0 in load path 0.000000 0.000000 0.000000 ( 0.005887)<br>
500 in load path 0.090000 0.110000 0.200000 ( 0.208288)<br>
1000 in load path 0.160000 0.250000 0.410000 ( 0.406125)<br>
1500 in load path 0.200000 0.370000 0.570000 ( 0.586779)<br>
2000 in load path 0.240000 0.500000 0.740000 ( 0.749551)</p>
<a name="Xavier-patch"></a>
<h1 >Xavier patch<a href="#Xavier-patch" class="wiki-anchor">¶</a></h1>
<p>$ ../ruby.new -v full_load_path_benchmark.rb<br>
ruby 1.9.3dev (2011-05-30 trunk 31824) [i686-linux]<br>
user system total real<br>
0 in load path 0.000000 0.000000 0.000000 ( 0.006294)<br>
500 in load path 0.080000 0.210000 0.290000 ( 0.293921)<br>
1000 in load path 0.190000 0.390000 0.580000 ( 0.585095)<br>
1500 in load path 0.290000 0.540000 0.830000 ( 0.839902)<br>
2000 in load path 0.370000 0.740000 1.110000 ( 1.114302)</p>
<p>--<br>
Yusuke Endoh <a href="mailto:mame@tsg.ne.jp" class="email">mame@tsg.ne.jp</a></p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=17352
2011-05-31T01:59:05Z
mame (Yusuke Endoh)
mame@ruby-lang.org
<ul></ul><p>Hello,</p>
<p>2011/5/31 Yusuke ENDOH <a href="mailto:mame@tsg.ne.jp" class="email">mame@tsg.ne.jp</a>:</p>
<blockquote>
<p>Hello, Xavier</p>
<p><a href="http://rhnh.net/2011/05/28/speeding-up-rails-startup-time" class="external">http://rhnh.net/2011/05/28/speeding-up-rails-startup-time</a></p>
<p>In the above article, you said that the bottleneck is to find<br>
$LOADED_FEATURES in linear, but I doubt it. If you are right,<br>
1.8 should be also slow because 1.8 uses the same algorithm.<br>
Thus, I guess there is another bottleneck.</p>
<p>I tried your patch and test in <a href="https://gist.github.com/985224" class="external">https://gist.github.com/985224</a><br>
on Ubuntu.<br>
I could confirm that trunk is twice slower than 1.9.2p180.</p>
</blockquote>
<p>I investigated this issue with ko1 and tarui-san.</p>
<p>Trunk seems to create more objects during require than 1.9.2.<br>
These objects are not collected because the benchmark program<br>
stops it by GC.disable.</p>
<p>1.9.2: 0.30 sec (w/ GC.disable) -> 0.32 sec (w/o GC.disable)<br>
trunk: 0.73 sec (w/ GC.disable) -> 0.48 sec (w/o GC.disable)</p>
<p>The first issue is in the benchmark program.</p>
<p>Still, trunk is 1.5x slower than 1.9.2 because of the object<br>
generation.<br>
Each require does the something like this:</p>
<p>$LOADED_FEATURES.map {|f| File.expand_path(f) }</p>
<p>.<br>
This process creates many objects, i.e., strings. Typically,<br>
$LOADED_FEATURES are already expanded, so the process is not<br>
needed in normal cases. In fact, 1.9.2 expands the paths only<br>
when they are not absolute, like this:</p>
<p>$LOADED_FEATURES.map {|f| File.expand_path(f) if f is not absolute }</p>
<p>However, the check was removed at r30789,<br>
I cannot understand the reason of the change because the commit<br>
log is too quiet, but I found <a href="https://blade.ruby-lang.org/ruby-core/34593">[ruby-core:34593]</a> and guess that<br>
it motivated the change:</p>
<blockquote>
<p>Autoload treatment of absolute paths in $LOAD_PATH containing . or ..</p>
</blockquote>
<p>I think reverting r30789 is an good solution in the immediate<br>
future.</p>
<p>--<br>
Yusuke Endoh <a href="mailto:mame@tsg.ne.jp" class="email">mame@tsg.ne.jp</a></p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=17354
2011-05-31T02:29:09Z
mame (Yusuke Endoh)
mame@ruby-lang.org
<ul></ul><p>Hello,</p>
<p>I'd like to show a new benchmark on Ubuntu.</p>
<ul>
<li>trunk becomes as fast as 1.9.2p180 if r30789 is reverted</li>
<li>1.8.7 is slower than 1.9.2p180 on my environment</li>
</ul>
<p>Sorry for my sending many mails in a short time.</p>
<a name="192p180"></a>
<h1 >1.9.2p180<a href="#192p180" class="wiki-anchor">¶</a></h1>
<p>$ ruby -v full_load_path_benchmark.rb<br>
ruby 1.9.2p180 (2011-02-18 revision 30909) [i686-linux]<br>
user system total real<br>
0 in load path 0.010000 0.000000 0.010000 ( 0.004401)<br>
500 in load path 0.030000 0.040000 0.070000 ( 0.097179)<br>
1000 in load path 0.070000 0.110000 0.180000 ( 0.190188)<br>
1500 in load path 0.130000 0.120000 0.250000 ( 0.271191)<br>
2000 in load path 0.110000 0.230000 0.340000 ( 0.377725)</p>
<a name="trunk-with-r30789-reverted"></a>
<h1 >trunk with r30789 reverted<a href="#trunk-with-r30789-reverted" class="wiki-anchor">¶</a></h1>
<p>$ ../ruby -v full_load_path_benchmark.rb<br>
ruby 1.9.3dev (2011-05-30 trunk 31824) [i686-linux]<br>
user system total real<br>
0 in load path 0.010000 0.000000 0.010000 ( 0.010032)<br>
500 in load path 0.020000 0.050000 0.070000 ( 0.101144)<br>
1000 in load path 0.050000 0.120000 0.170000 ( 0.189199)<br>
1500 in load path 0.100000 0.150000 0.250000 ( 0.275388)<br>
2000 in load path 0.130000 0.200000 0.330000 ( 0.363190)</p>
<a name="187p302-Ubuntu-package"></a>
<h1 >1.8.7p302 (Ubuntu package)<a href="#187p302-Ubuntu-package" class="wiki-anchor">¶</a></h1>
<p>$ /usr/bin/ruby -v full_load_path_benchmark.rb<br>
ruby 1.8.7 (2010-08-16 patchlevel 302) [i686-linux]<br>
user system total real<br>
0 in load path 0.000000 0.000000 0.000000 ( 0.005125)<br>
500 in load path 0.030000 0.090000 0.120000 ( 0.142067)<br>
1000 in load path 0.080000 0.170000 0.250000 ( 0.271456)<br>
1500 in load path 0.090000 0.290000 0.380000 ( 0.419044)<br>
2000 in load path 0.140000 0.400000 0.540000 ( 0.560379)</p>
<a name="187p334-self-built-with-O2"></a>
<h1 >1.8.7p334 (self-built with -O2)<a href="#187p334-self-built-with-O2" class="wiki-anchor">¶</a></h1>
<p>$ ruby-1.8.7-p334 -v full_load_path_benchmark.rb<br>
ruby 1.8.7 (2011-02-18 patchlevel 334) [i686-linux]<br>
user system total real<br>
0 in load path 0.000000 0.000000 0.000000 ( 0.002295)<br>
500 in load path 0.040000 0.080000 0.120000 ( 0.114809)<br>
1000 in load path 0.060000 0.160000 0.220000 ( 0.226771)<br>
1500 in load path 0.130000 0.210000 0.340000 ( 0.337815)<br>
2000 in load path 0.130000 0.330000 0.460000 ( 0.455198)</p>
<p>--<br>
Yusuke Endoh <a href="mailto:mame@tsg.ne.jp" class="email">mame@tsg.ne.jp</a></p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=17355
2011-05-31T07:23:06Z
xaviershay (Xavier Shay)
xavier-list@rhnh.net
<ul></ul><p>On 30/05/11 11:55 PM, Yusuke ENDOH wrote:</p>
<blockquote>
<p>Hello,</p>
<p>2011/5/30 Xavier Shay<a href="mailto:xavier-list@rhnh.net" class="email">xavier-list@rhnh.net</a>:</p>
<blockquote>
<blockquote>
<ul>
<li>Please try to minimize a patch.
<ul>
<li>It would be good to use the existing code as possible as you can.<br>
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.</li>
</ul>
</li>
</ul>
</blockquote>
</blockquote>
<p>Indeed, load.c is very complex, but I believe that this is because<br>
load.c is an accumulation of bad know-hows in the long history.<br>
So we should not throw it away without understanding, I think.<br>
This is a fair point. Perhaps a good first step would be trying to<br>
refactor it with better variable and method names so that the intent of<br>
the code can reveal itself.</p>
</blockquote>
<blockquote>
<blockquote>
<blockquote>
<ul>
<li>It is slightly disturbing to define a new class with Array inherited.<br>
Is it really needed? Is there no alternative approach?<br>
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?</li>
</ul>
</blockquote>
</blockquote>
<p>Aha, I understand why the inheritance is used. It is to hook the<br>
array modification, right?<br>
correct</p>
</blockquote>
<blockquote>
<p>It is an impossible approach because some extension library can<br>
modify $LOADED_FEATURES directly by rb_ary_push.<br>
I found amalgalite to do so actually:</p>
<p><a href="http://www.google.com/codesearch/p?hl=ja#MyfZfO3_1cw/ext/amalgalite/amalgalite3_requires_bootstrap.c&q=LOADED_FEATURES%20lang:c%20rb_ary_push&l=151" class="external">http://www.google.com/codesearch/p?hl=ja#MyfZfO3_1cw/ext/amalgalite/amalgalite3_requires_bootstrap.c&q=LOADED_FEATURES%20lang:c%20rb_ary_push&l=151</a></p>
<blockquote>
<p>rb_ary_push( rb_gv_get( "$LOADED_FEATURES" ), require_name );</p>
</blockquote>
</blockquote>
<p>It seems that amalgalite is the only extension to do this in that<br>
search. While I realise that doesn't cover everything, it seems the<br>
impact is perhaps small? Would it be possible to instead work with the<br>
maintainer to fix, and publicize this change. It seems and odd API to be<br>
supporting.</p>
<p>Xavier</p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=17356
2011-05-31T07:23:06Z
xaviershay (Xavier Shay)
xavier-list@rhnh.net
<ul></ul><p>On 30/05/11 5:35 PM, Shyouhei Urabe wrote:</p>
<blockquote>
<ul>
<li>I'm not sure if your code is ready for case-insensitive filesystems<br>
like those in Windows. I'm sure you know the problem so I should<br>
have missed something.<br>
I don't have a Windows environment to test with. I would be most<br>
appreciative if anyone was able to try it out.</li>
</ul>
</blockquote>
<p>Xavier</p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=17357
2011-05-31T07:23:06Z
xaviershay (Xavier Shay)
xavier-list@rhnh.net
<ul></ul><p>On 31/05/11 1:16 AM, Yusuke ENDOH wrote:</p>
<blockquote>
<p>Hello, Xavier</p>
<p><a href="http://rhnh.net/2011/05/28/speeding-up-rails-startup-time" class="external">http://rhnh.net/2011/05/28/speeding-up-rails-startup-time</a></p>
<p>In the above article, you said that the bottleneck is to find<br>
$LOADED_FEATURES in linear, but I doubt it. If you are right,<br>
1.8 should be also slow because 1.8 uses the same algorithm.<br>
Thus, I guess there is another bottleneck.<br>
1.8 actually does exhibit the same performance curve, just over a<br>
greater value of N. See this graph (x-axis doubled from other benchmarks)</p>
</blockquote>
<p><a href="https://gist.github.com/c8d0d422a9203e1fe492#gistcomment-30971" class="external">https://gist.github.com/c8d0d422a9203e1fe492#gistcomment-30971</a></p>
<p>I don't doubt there are other bottlenecks too though.</p>
<blockquote>
<p>I tried your patch and test in <a href="https://gist.github.com/985224" class="external">https://gist.github.com/985224</a><br>
on Ubuntu.<br>
I could confirm that trunk is twice slower than 1.9.2p180.<br>
But I couldn't confirm that the speed up by your patch.<br>
I also performed the same benchmark after "gem install rails",<br>
but there is no difference.<br>
In that case perhaps it is related to the CASEFOLD_FILESYSTEM? I have<br>
been benchmarking on OSX.</p>
</blockquote>
<p>A related change was reverted recently in r31692.</p>
<p>Cheers,<br>
Xavier</p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=17358
2011-05-31T07:23:06Z
luislavena (Luis Lavena)
luislavena@gmail.com
<ul></ul><p>On Mon, May 30, 2011 at 6:00 PM, Xavier Shay <a href="mailto:xavier-list@rhnh.net" class="email">xavier-list@rhnh.net</a> wrote:</p>
<blockquote>
<blockquote>
<p>It is an impossible approach because some extension library can<br>
modify $LOADED_FEATURES directly by rb_ary_push.<br>
I found amalgalite to do so actually:</p>
<p><a href="http://www.google.com/codesearch/p?hl" class="external">http://www.google.com/codesearch/p?hl</a></p>
</blockquote>
</blockquote>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=17359
2011-05-31T07:23:06Z
luislavena (Luis Lavena)
luislavena@gmail.com
<ul></ul><p>On Mon, May 30, 2011 at 6:01 PM, Xavier Shay <a href="mailto:xavier-list@rhnh.net" class="email">xavier-list@rhnh.net</a> wrote:</p>
<blockquote>
<p>On 30/05/11 5:35 PM, Shyouhei Urabe wrote:</p>
<blockquote>
<ul>
<li>I'm not sure if your code is ready for case-insensitive filesystems<br>
 like those in Windows.  I'm sure you know the problem so I should<br>
 have missed something.</li>
</ul>
</blockquote>
<p>I don't have a Windows environment to test with. I would be most<br>
appreciative if anyone was able to try it out.</p>
</blockquote>
<p>I do have a Windows system and I've provided my initial numbers.</p>
<p>However, as was previously stated by other core members, the size of<br>
the patch is hard to process and analyze properly, which might not<br>
catch corner cases that were present in load.c and this newer code do<br>
not contemplate.</p>
<p>For me to completely test this on Windows will take more than a week,<br>
as I have multiple applications with different requirements and to<br>
properly perform this I need to track pre and post performance details<br>
of each of those applications to provide fair results.</p>
<h2>--<br>
Luis Lavena<br>
AREA 17</h2>
<p>Perfection in design is achieved not when there is nothing more to add,<br>
but rather when there is nothing more to take away.<br>
Antoine de Saint-Exupéry</p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=17360
2011-05-31T09:29:08Z
mame (Yusuke Endoh)
mame@ruby-lang.org
<ul></ul><p>Hello,</p>
<p>2011/5/31 Xavier Shay <a href="mailto:xavier-list@rhnh.net" class="email">xavier-list@rhnh.net</a>:</p>
<blockquote>
<blockquote>
<blockquote>
<p>rb_ary_push( rb_gv_get( "$LOADED_FEATURES" ), require_name );</p>
</blockquote>
</blockquote>
<p>It seems that amalgalite is the only extension to do this in that search.<br>
While I realise that doesn't cover everything, it seems the impact is<br>
perhaps small? Would it be possible to instead work with the maintainer to<br>
fix, and publicize this change. It seems and odd API to be supporting.</p>
</blockquote>
<p>I don't think so. When I try to add a path into $LOADED_FEATUES<br>
by extension library, rb_ary_push is the first way that came to<br>
mind.</p>
<p>2011/5/31 Xavier Shay <a href="mailto:xavier-list@rhnh.net" class="email">xavier-list@rhnh.net</a>:</p>
<blockquote>
<p>1.8 actually does exhibit the same performance curve, just over a greater<br>
value of N. See this graph (x-axis doubled from other benchmarks)</p>
</blockquote>
<p>Hmm... Okay. However, it is hard to fix this issue in 1.9.x,<br>
I think.<br>
To be honestly, $LOADED_FEATURES as an array is uncool, or even<br>
considered as a "specification bug", though.</p>
<blockquote>
<blockquote>
<p>I tried your patch and test in <a href="https://gist.github.com/985224" class="external">https://gist.github.com/985224</a><br>
on Ubuntu.<br>
I could confirm that trunk is twice slower than 1.9.2p180.<br>
But I couldn't confirm that the speed up by your patch.<br>
I also performed the same benchmark after "gem install rails",<br>
but there is no difference.</p>
</blockquote>
<p>In that case perhaps it is related to the CASEFOLD_FILESYSTEM? I have been<br>
benchmarking on OSX.</p>
</blockquote>
<p>I see!</p>
<blockquote>
<p>A related change was reverted recently in r31692.</p>
</blockquote>
<p>Then, is there no problem in os x now?</p>
<p>--<br>
Yusuke Endoh <a href="mailto:mame@tsg.ne.jp" class="email">mame@tsg.ne.jp</a></p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=17395
2011-05-31T21:08:50Z
xaviershay (Xavier Shay)
xavier-list@rhnh.net
<ul></ul><p>Hello,<br>
This is a long message, but I have tried to address most of the concerns with my patch. There are two sections: one for the technical detail of the patch, and one for benchmarks.</p>
<p>= The patch</p>
<p>I have split my patch up into a six smaller patches which I hope are easier to review.</p>
<p>Descriptions: <a href="https://gist.github.com/35060fbcefb25cf1a456" class="external">https://gist.github.com/35060fbcefb25cf1a456</a><br>
Patches: <a href="https://gist.github.com/58dbd6e72c1a1f47a415" class="external">https://gist.github.com/58dbd6e72c1a1f47a415</a><br>
GitHub: <a href="https://github.com/xaviershay/ruby/commits/require-patch" class="external">https://github.com/xaviershay/ruby/commits/require-patch</a></p>
<p>In addition I have addressed the following concerns from Yusuke:</p>
<ul>
<li>Please use 4 space for indent, with 8 space tab. (Emacs-style)</li>
<li>Please use C89. Please don't use // comments.</li>
<li>Please don't export function without "rb_" prefix, to avoid symbol<br>
conflicts.</li>
</ul>
<p>I think there are still two big concerns:</p>
<ol>
<li>Significantly different from original load.c</li>
<li>Compatibility with Windows</li>
<li>LoadedFeaturesProxy</li>
</ol>
<p>Regarding #1, I don't have anything further to add at the moment.<br>
Regarding #2, it will need to be tested thoroughly like all other platforms, but I don't believe it would result in significant architectural changes.</p>
<p>For #3, I was worried about it but on reflection I think it may not be a problem.</p>
<p>Yusuke wrote:<br>
"It is an impossible approach because some extension library can<br>
modify $LOADED_FEATURES directly by rb_ary_push."</p>
<p>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.<br>
This will degrade worst-case performance, but I don't think that matters because most requires will succeed.<br>
Does this make sense?</p>
<p>This would also mean the addition of <code>lib/enumerator.rb</code> in my patch would be unnecessary (though IMO probably still a good idea).</p>
<p>= Benchmarks</p>
<p>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.</p>
<p>Here are my updated timings, with descriptions of each. For the load path and require benchmark I have only included the final number in the test to allow for a more focused comparison.</p>
<p>Run on OSX 10.7</p>
<pre><code> 1.9.2p180 1.9.3* 1.9.3** 1.9.3***
</code></pre>
<p>load path 0.667376 0.866228 0.623411 0.699315<br>
requires 6.745875 7.921598 7.261326 0.285119<br>
new rails 2.172 1.412 1.624 1.082<br>
medium rails 18.372 17.59 15.855 10.488</p>
<ul>
<li>1.9.3r31827 (31/5)<br>
** 1.9.3r31827, with r30789 reverted<br>
*** 1.9.3r31827, with my patch</li>
</ul>
<p>== Benchmark descriptions</p>
<h3>load path - <a href="https://gist.github.com/985224" class="external">https://gist.github.com/985224</a>
</h3>
<p>Loading a file 50 times with 2000 entries in the $LOAD_PATH.<br>
This is an academic benchmark, since 2000 entries is a large amount. For comparison, our rails app load path only has 42 entries.</p>
<p>Performance for all versions is linear and roughly equivalent.<br>
Note that I removed the <code>GC.disable</code> that was in earlier versions of this benchmark.</p>
<h3>requires - <a href="https://gist.github.com/c8d0d422a9203e1fe492" class="external">https://gist.github.com/c8d0d422a9203e1fe492</a>
</h3>
<p>Loading 2500 files. This is a more relavent benchmark. Our Rails app loads ~2200 files, large apps can load as many as 9000 [1].<br>
All versions display an exponential increase in time as N increases. 1.8.7, though not included in this measurement, also displays such an exponential curve but it isn't as noticeable in comparison with 1.9 since the magnifier is far less [2].</p>
<p>[1] sent to me via private correspondence<br>
[2] see <a href="https://gist.github.com/c8d0d422a9203e1fe492" class="external">https://gist.github.com/c8d0d422a9203e1fe492</a></p>
<a name="new-rails"></a>
<h3 >new rails<a href="#new-rails" class="wiki-anchor">¶</a></h3>
<p>Using rails 3.0.7:</p>
<pre><code>rails new test-rails-app
cd test-rails-app
time ruby script/rails runner "puts 1"
</code></pre>
<a name="medium-rails"></a>
<h3 >medium rails<a href="#medium-rails" class="wiki-anchor">¶</a></h3>
<p>This is my main Rails code base. Sorry I cannot share it, though this benchmark is not required to make a case for this patch since it shows roughly the same percentage improvement as a blank rails app.</p>
<p>Thanks,<br>
Xavier</p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=17396
2011-05-31T21:13:50Z
xaviershay (Xavier Shay)
xavier-list@rhnh.net
<ul></ul><p>Argh I mangled my benchmark table. Here is a picture that is easy to read:<br>
<a href="https://img.skitch.com/20110531-qn7nukkyretiepysmytm8r1757.jpg" class="external">https://img.skitch.com/20110531-qn7nukkyretiepysmytm8r1757.jpg</a></p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=17400
2011-05-31T23:24:21Z
mame (Yusuke Endoh)
mame@ruby-lang.org
<ul></ul><p>Hello,</p>
<p>2011/5/31 Xavier Shay <a href="mailto:xavier-list@rhnh.net" class="email">xavier-list@rhnh.net</a>:</p>
<blockquote>
<p>Patches: <a href="https://gist.github.com/58dbd6e72c1a1f47a415" class="external">https://gist.github.com/58dbd6e72c1a1f47a415</a></p>
</blockquote>
<p>Awesome!</p>
<blockquote>
<p>In addition I have addressed the following concerns from Yusuke:</p>
<ul>
<li>Please use 4 space for indent, with 8 space tab. (Emacs-style)</li>
</ul>
</blockquote>
<p>Your patch still seems to use 8-space (1-tab) indent.</p>
<p>Yours:</p>
<p>+static VALUE<br>
+rb_locate_file(VALUE filename)<br>
+{</p>
<ul>
<li>
<pre><code> VALUE full_path = Qnil;
</code></pre>
</li>
</ul>
<pre><code>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 <mame@tsg.ne.jp>
</code></pre>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=17406
2011-06-01T07:17:34Z
tarui (Masaya Tarui)
tarui@prx.jp
<ul></ul><p>Hello,</p>
<p>Require performance has been imporved a little at r31875, i think.<br>
Please compare the proposal with it.</p>
<p>Thank you.</p>
<p>Yusuke wrote:</p>
<blockquote>
<p>2011/5/31 Xavier Shay :</p>
<blockquote>
<p>1.8 actually does exhibit the same performance curve, just over a greater<br>
value of N. See this graph (x-axis doubled from other benchmarks)<br>
Hmm... Okay. However, it is hard to fix this issue in 1.9.x,<br>
I think.<br>
To be honestly, $LOADED_FEATURES as an array is uncool, or even<br>
considered as a "specification bug", though.</p>
</blockquote>
</blockquote>
<p>I agree with you.</p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=17415
2011-06-01T07:59:09Z
xaviershay (Xavier Shay)
xavier-list@rhnh.net
<ul></ul><p>On 1/06/11 12:24 AM, Yusuke Endoh wrote:</p>
<blockquote>
<blockquote>
<p>In addition I have addressed the following concerns from Yusuke:</p>
<ul>
<li>Please use 4 space for indent, with 8 space tab. (Emacs-style)</li>
</ul>
</blockquote>
<p>Your patch still seems to use 8-space (1-tab) indent.<br>
Sorry I misunderstood what you were asking for. Too long working with<br>
ruby 2 spaces, I have forgotten what tabbing looks like :)</p>
</blockquote>
<p>After other things are resolved I will go through and ensure all of<br>
load.c is formatted correctly.</p>
<blockquote>
<blockquote>
<p>Yusuke wrote:<br>
"It is an impossible approach because some extension library can<br>
modify $LOADED_FEATURES directly by rb_ary_push."</p>
<p>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.</p>
</blockquote>
<p>Sorry I cannot get your point.</p>
<p>The following program should not load t.rb, should it?</p>
<p>$"[$".size] = File.expand_path("./t.rb")<br>
require("./t")</p>
<p>However, this actually load t.rb after your patch is applied:</p>
<p>$ cat t.rb<br>
p :loaded</p>
<p>$ ./ruby -I. -e '<br>
$"[$".size] = File.expand_path("./t.rb")<br>
require("./t")<br>
'<br>
:loaded</p>
<p>This is considerable incompatiblity, I think.</p>
<p>Of course, you can change LoadedFeaturesProxy to hook Array#[]=.<br>
But you cannot hook rb_ary_push. This is my concern.<br>
I was thinking of the case where non-file features are pushed to the<br>
array (such as enumerator.so and almagamite).<br>
rb_ary_push($LOADED_FEATURES, 'enumerator.so')<br>
require 'enumerator'</p>
</blockquote>
<p>Perhaps a scan of the load path to cover this case woudn't be too bad?<br>
(Not in my patch, just thinking out loud).</p>
<p>This does not cover your case though so perhaps not a great idea:<br>
rb_ary_push($LOADED_FEATURES, File.expand_path("./t"))<br>
require "./t"</p>
<p>Two other options that maybe you will like:</p>
<ol>
<li>store some metadata against entries in $LOADED_FEATURES to indicate<br>
whether they have been cached. rb_ary_push will not set this, so a quick<br>
scan can be made of $LOADED_FEATURES on require to see if a recache is<br>
needed</li>
<li>On require, recache if the count of $LOADED_FEATURES does not match<br>
the count of the cache. This isn't 100% correct --- you could rb_ary_pop<br>
then rb_ary push to work around it --- but perhaps if more acceptable?</li>
</ol>
<p>I think #2 if the limitation is acceptable. Items added by rb_ary_push<br>
would be located on the filesystem if possible (such as "./t" in your<br>
example), other wise left as is (such as "enumerator.so").</p>
<blockquote>
<blockquote>
<p>This would also mean the addition of <code>lib/enumerator.rb</code> in my patch would be unnecessary (though IMO probably still a good idea).</p>
</blockquote>
<p>Yes, it is a good idea, and will be accepted eventually.<br>
But I'm afraid if it is not the timing to do so.<br>
And we should discuss it separately.<br>
ok</p>
</blockquote>
<p>Thanks,<br>
Xavier</p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=17417
2011-06-01T09:23:07Z
shyouhei (Shyouhei Urabe)
shyouhei@ruby-lang.org
<ul></ul><p>Hi Yusuke,</p>
<p>(05/31/2011 01:58 AM), Yusuke ENDOH wrote:</p>
<blockquote>
<p>Still, trunk is 1.5x slower than 1.9.2 because of the object<br>
generation.<br>
Each require does the something like this:</p>
<p>$LOADED_FEATURES.map {|f| File.expand_path(f) }</p>
<p>.<br>
This process creates many objects, i.e., strings. Typically,<br>
$LOADED_FEATURES are already expanded, so the process is not<br>
needed in normal cases. In fact, 1.9.2 expands the paths only<br>
when they are not absolute, like this:</p>
<p>$LOADED_FEATURES.map {|f| File.expand_path(f) if f is not absolute }</p>
</blockquote>
<p>If it's the actual problem, why not cache the expanded path?</p>
<p><a href="https://github.com/shyouhei/ruby/commit/c229cb4" class="external">https://github.com/shyouhei/ruby/commit/c229cb4</a></p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=17471
2011-06-03T04:53:07Z
xaviershay (Xavier Shay)
xavier-list@rhnh.net
<ul></ul><p>On 31/05/11 3:55 PM, Xavier Shay wrote:</p>
<blockquote>
<p>Two other options that maybe you will like:</p>
<ol>
<li>store some metadata against entries in $LOADED_FEATURES to indicate<br>
whether they have been cached. rb_ary_push will not set this, so a quick<br>
scan can be made of $LOADED_FEATURES on require to see if a recache is<br>
needed</li>
<li>On require, recache if the count of $LOADED_FEATURES does not match<br>
the count of the cache. This isn't 100% correct --- you could rb_ary_pop<br>
then rb_ary push to work around it --- but perhaps if more acceptable?</li>
</ol>
<p>I think #2 if the limitation is acceptable. Items added by rb_ary_push<br>
would be located on the filesystem if possible (such as "./t" in your<br>
example), other wise left as is (such as "enumerator.so").<br>
I have put together a proof of concept of this:<br>
<a href="https://gist.github.com/8bb23db32d861cbe952f" class="external">https://gist.github.com/8bb23db32d861cbe952f</a></p>
</blockquote>
<p>It removes the need for both lib/enumerator.rb, and the changes I made<br>
to loading mkmf in ext/exmke.rb</p>
<p>I still need to test it with amalgamite (was on a plane so didn't have<br>
internet to get it).</p>
<p>If you think the limitations are ok, I will work it back into my other<br>
patch set.</p>
<p>Also, I note a change to load.c has just gone into trunk related to<br>
performance. I will add this to my list of benchmarks in the next few days.</p>
<p>Cheers,<br>
Xavier</p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=17508
2011-06-05T15:23:06Z
xaviershay (Xavier Shay)
xavier-list@rhnh.net
<ul></ul><p>On 1/06/11 8:17 AM, Masaya Tarui wrote:</p>
<blockquote>
<p>Issue <a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Bug: Performance bug (in require?) (Closed)" href="https://redmine.ruby-lang.org/issues/3924">#3924</a> has been updated by Masaya Tarui.</p>
<p>Hello,</p>
<p>Require performance has been imporved a little at r31875, i think.<br>
nice! It sure has.</p>
</blockquote>
<blockquote>
<p>Please compare the proposal with it</p>
</blockquote>
<pre><code> Mine 1.9.3r31923
</code></pre>
<p>2000 in load path 0.70 0.702834<br>
2500 requires 0.29 5.676016<br>
new rails app 1.08 1.346<br>
medium rails app 10.49 10.88</p>
<p>The underlying algorithm is still problematic (see 2500 requires<br>
benchmark), but I don't have strong evidence that it is affecting<br>
performance of real apps given it appears to perform well on rails<br>
applications. Assuming no contrary evidence appears, and without<br>
understanding this difference further, you probably won't want to apply<br>
my patch for a 1.9.3 point release given the risk of regressions. I<br>
still think it should be given consideration for a future major release<br>
though.</p>
<p>Out of interest ... how does your commit work? Is it an optimization<br>
perhaps my patch could benefit from? My concern with r31875 is that it<br>
makes that loop even harder to understand for those not as familiar with<br>
that part of the code.</p>
<p>Cheers,<br>
Xavier</p>
<blockquote>
<p>Thank you.</p>
<p>Yusuke wrote:</p>
<blockquote>
<p>2011/5/31 Xavier Shay :</p>
<blockquote>
<p>1.8 actually does exhibit the same performance curve, just over a greater<br>
value of N. See this graph (x-axis doubled from other benchmarks)<br>
Hmm... Okay. However, it is hard to fix this issue in 1.9.x,<br>
I think.<br>
To be honestly, $LOADED_FEATURES as an array is uncool, or even<br>
considered as a "specification bug", though.</p>
</blockquote>
</blockquote>
<p>I agree with you.</p>
<hr>
<p>Bug <a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Bug: Performance bug (in require?) (Closed)" href="https://redmine.ruby-lang.org/issues/3924">#3924</a>: Performance bug (in require?)<br>
<a href="http://redmine.ruby-lang.org/issues/3924" class="external">http://redmine.ruby-lang.org/issues/3924</a></p>
<p>Author: Carsten Bormann<br>
Status: Open<br>
Priority: Normal<br>
Assignee:<br>
Category: core<br>
Target version: 1.9.3<br>
ruby -v: -</p>
<p>=begin<br>
Running irb< /dev/null in 1.9.2 causes 3016 calls to lstat64.</p>
<p>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.</p>
<p>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).<br>
=end</p>
</blockquote>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=17614
2011-06-09T05:23:06Z
tarui (Masaya Tarui)
tarui@prx.jp
<ul></ul><blockquote>
<blockquote>
<p>Require performance has been imporved a little at r31875, i think.</p>
</blockquote>
<p>nice! It sure has.</p>
<blockquote>
<p>Please compare the proposal with it</p>
</blockquote>
<p>            Mine   1.9.3r31923<br>
2000 in load path    0.70   0.702834<br>
2500 requires      0.29   5.676016<br>
new rails app      1.08   1.346<br>
medium rails app     10.49  10.88</p>
<p>The underlying algorithm is still problematic (see 2500 requires benchmark),<br>
but I don't have strong evidence that it is affecting performance of real<br>
apps given it appears to perform well on rails applications. Assuming no<br>
contrary evidence appears, and without understanding this difference<br>
further, you probably won't want to apply my patch for a 1.9.3 point release<br>
given the risk of regressions. I still think it should be given<br>
consideration for a future major release though.</p>
</blockquote>
<p>Thank you for benchmarking. ( I don't have rails benchmarks. )<br>
It is good news that it's effective enough for rails.</p>
<p>I think that your idea is good, and heard Hash is already used in JRuby.<br>
But your patch is large and has some compatibility issue,<br>
so I'm afraid for reject at 1.9.3.</p>
<blockquote>
<p>Out of interest ... how does your commit work? Is it an optimization perhaps<br>
my patch could benefit from? My concern with r31875 is that it makes that<br>
loop even harder to understand for those not as familiar with that part of<br>
the code.</p>
</blockquote>
<p>In loaded_feature_path and outer of it,<br>
it takes the match LOADED_FEATURE and LOAD_PATH+target in each of<br>
LOADED_FEATURES and LOAD_PATH array for back compatibility ( i think ) .<br>
before patch, part of matching LOADED_FEATURE and target is in inner loop,<br>
so I only let it out.<br>
LOADED_FEATURE is not matched with target with many case, and patch<br>
makes return fast.</p>
<p>In your patch, cache processing is similar to its feature, and already<br>
optimized.</p>
<p>Regards,<br>
Masaya</p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=17768
2011-06-11T16:43:34Z
ko1 (Koichi Sasada)
<ul><li><strong>Status</strong> changed from <i>Open</i> to <i>Assigned</i></li><li><strong>Assignee</strong> set to <i>yugui (Yuki Sonoda)</i></li></ul>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=18094
2011-06-20T18:01:52Z
jskrzypek (Jarosław Skrzypek)
skrzypek.jarek@gmail.com
<ul></ul><p>You might be also interested in my very short patch: <a href="https://gist.github.com/1035322" class="external">https://gist.github.com/1035322</a> Basically it reorders conditions to start with fastest ones and execute slower ones only if needed. Here is also some description: <a href="http://www.lunarlogicpolska.com/blog/2011/06/14/tracing-processes-for-fun-and-profit.html" class="external">http://www.lunarlogicpolska.com/blog/2011/06/14/tracing-processes-for-fun-and-profit.html</a></p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=18161
2011-06-23T12:23:20Z
nahi (Hiroshi Nakamura)
nakahiro@gmail.com
<ul></ul><p>Hi all,</p>
<p>On Wed, Jun 1, 2011 at 09:14, Urabe Shyouhei <a href="mailto:shyouhei@ruby-lang.org" class="email">shyouhei@ruby-lang.org</a> wrote:</p>
<blockquote>
<blockquote>
<p>This process creates many objects, i.e., strings. Â Typically,<br>
$LOADED_FEATURES are already expanded, so the process is not<br>
needed in normal cases. Â In fact, 1.9.2 expands the paths only<br>
when they are not absolute, like this:</p>
<p>Â $LOADED_FEATURES.map {|f| File.expand_path(f) if f is not absolute }</p>
</blockquote>
<p>If it's the actual problem, why not cache the expanded path?</p>
<p><a href="https://github.com/shyouhei/ruby/commit/c229cb4" class="external">https://github.com/shyouhei/ruby/commit/c229cb4</a></p>
</blockquote>
<p>I did a benchmark > <a href="https://gist.github.com/1041791" class="external">https://gist.github.com/1041791</a> (full)</p>
<p>Measured loading time of 2 Rails 3.0.7 apps</p>
<ul>
<li>emptyApp: script/rails generate emptyApp</li>
<li>slow-rails: by Joe Van Dyk (<a href="https://github.com/joevandyk/slow-rails" class="external">https://github.com/joevandyk/slow-rails</a>)</li>
</ul>
<p>Interpreters</p>
<ul>
<li>ruby 1.9.2p274 (2011-06-06 revision 31932) [x86_64-linux]</li>
<li>ruby 1.9.3dev (2011-06-22 trunk 32204) [x86_64-linux]</li>
<li>ruby 1.9.3dev (2011-06-22 trunk 32204) [x86_64-linux] + Shyouhei's<br>
expand_path cache patch<br>
(<a href="https://github.com/shyouhei/ruby/commit/c229cb4" class="external">https://github.com/shyouhei/ruby/commit/c229cb4</a>)</li>
</ul>
<p>Results (average wall clock time of 'time ruby script/rails runner 0' 10 times)</p>
<ul>
<li>
<p>1.9.2p274</p>
<ul>
<li>emptyApp: 1.87 [sec]</li>
<li>slow-rails: 8.69 [sec]</li>
</ul>
</li>
<li>
<p>1.9.3dev of today</p>
<ul>
<li>emptyApp: 1.35 [sec] (39% faster than 1.9.2)</li>
<li>slow-rails: 6.37 [sec] (36% faster than 1.9.2)</li>
</ul>
</li>
<li>
<p>1.9.3dev + Shyouhei's expand_path cache patch</p>
<ul>
<li>emptyApp: 1.07 [sec] (26% faster than 1.9.3dev)</li>
<li>slow-rails: 3.81 [sec] (67% faster than 1.9.3dev)</li>
</ul>
</li>
</ul>
<p>Awesome result.</p>
<p>Anyone can imagine a downside of this? It could not work as expected<br>
if the result of rb_file_expand_path <em>changes</em> during require (adding<br>
a new file during require should work.) Do we need to care such a<br>
case?</p>
<p>Regards,<br>
// NaHi</p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=18163
2011-06-23T12:53:06Z
mame (Yusuke Endoh)
mame@ruby-lang.org
<ul></ul><p>Hi,</p>
<p>2011/6/23 Hiroshi Nakamura <a href="mailto:nakahiro@gmail.com" class="email">nakahiro@gmail.com</a>:</p>
<blockquote>
<p>Anyone can imagine a downside of this? It could not work as expected<br>
if the result of rb_file_expand_path <em>changes</em> during require (adding<br>
a new file during require should work.) Do we need to care such a<br>
case?</p>
</blockquote>
<p>As far as I look the patch, paths that are once stored in cache<br>
will never be collected. It may be considered as a kind of<br>
memory leaks.</p>
<p>I guess that it is not a problem in casual cases. But I'm not sure<br>
if it is really OK.</p>
<p>--<br>
Yusuke Endoh <a href="mailto:mame@tsg.ne.jp" class="email">mame@tsg.ne.jp</a></p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=18178
2011-06-23T19:53:08Z
nahi (Hiroshi Nakamura)
nakahiro@gmail.com
<ul></ul><p>Hi,</p>
<p>2011/6/20 Jarosław Skrzypek <a href="mailto:skrzypek.jarek@gmail.com" class="email">skrzypek.jarek@gmail.com</a>:</p>
<blockquote>
<p>You might be also interested in my very short patch: <a href="https://gist.github.com/1035322" class="external">https://gist.github.com/1035322</a> Basically it reorders conditions to start with fastest ones and execute slower ones only if needed. Here is also some description: <a href="http://www.lunarlogicpolska.com/blog/2011/06/14/tracing-processes-for-fun-and-profit.html" class="external">http://www.lunarlogicpolska.com/blog/2011/06/14/tracing-processes-for-fun-and-profit.html</a></p>
</blockquote>
<p>Good perf improvement. Yugui-san, it could be a candidate for 1.9.2<br>
medication, if the backporting 1.9.3dev patches are big to apply.</p>
<p>Regards,<br>
// NaHi</p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=18581
2011-06-28T06:17:05Z
nahi (Hiroshi Nakamura)
nakahiro@gmail.com
<ul><li><strong>Status</strong> changed from <i>Assigned</i> to <i>Closed</i></li></ul><p>I close this ticket as 'Duplicated'. Please refer <a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Bug: Performance bug (in require?) (Closed)" href="https://redmine.ruby-lang.org/issues/3924">#3924</a>.</p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=18621
2011-06-28T16:20:57Z
cabo (Carsten Bormann)
cabo@tzi.org
<ul></ul><p>Well, you can't close both tickets as duplicates of each other.<br>
Which one stays open? The Windows one or this one, which has the discussion?</p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=18628
2011-06-28T20:33:11Z
nahi (Hiroshi Nakamura)
nakahiro@gmail.com
<ul><li><strong>Status</strong> changed from <i>Closed</i> to <i>Open</i></li></ul><p>Oops, I'm very sorry. I wanted to close only <a class="issue tracker-2 status-5 priority-4 priority-default closed" title="Feature: Initializing and loading Rails environment takes really long on Windows XP (Closed)" href="https://redmine.ruby-lang.org/issues/3906">#3906</a>. Reopened it.</p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=19063
2011-07-11T09:00:11Z
kosaki (Motohiro KOSAKI)
kosaki.motohiro@gmail.com
<ul><li><strong>Status</strong> changed from <i>Open</i> to <i>Assigned</i></li></ul><p>Nahi-san, Yugui-san, May I ask current status of this?</p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=19078
2011-07-11T22:37:00Z
nahi (Hiroshi Nakamura)
nakahiro@gmail.com
<ul></ul><p>Issues left are;</p>
<ul>
<li>Find a way to merge Xavier's work.</li>
<li>Merge Shyouhei's optimization by cache.</li>
</ul>
<p>I suggest to change Target version to 1.9.X.</p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=19079
2011-07-11T22:58:44Z
antares (Michael Klishin)
<ul></ul><p>I second Hiroshi's suggestion to solve this for 1.9.x and not just 1.9.3.</p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=19081
2011-07-11T23:05:59Z
svenfuchs (Sven Fuchs)
svenfuchs@artweb-design.de
<ul></ul><p>+1 on 1.9x</p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=19139
2011-07-13T18:53:10Z
Eregon (Benoit Daloze)
<ul></ul><p>On 11 July 2011 15:58, Michael Klishin <a href="mailto:redmine@ruby-lang.org" class="email">redmine@ruby-lang.org</a> wrote:</p>
<blockquote>
<p>Issue <a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Bug: Performance bug (in require?) (Closed)" href="https://redmine.ruby-lang.org/issues/3924">#3924</a> has been updated by Michael Klishin.</p>
<p>I second Hiroshi's suggestion to solve this for 1.9.x and not just 1.9.3.</p>
</blockquote>
<p>1.9.x means it should be solved for next 1.9, so 1.9.4 or later, it is<br>
not about backporting.<br>
It can not be merged in 1.9.3, because it is late.</p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=19434
2011-07-21T14:02:35Z
kosaki (Motohiro KOSAKI)
kosaki.motohiro@gmail.com
<ul><li><strong>Target version</strong> changed from <i>1.9.3</i> to <i>2.0.0</i></li></ul><p>I've switched the target version to 1.9.x. This issue is absolutely important, but sadly it's too late. ;-)</p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=23735
2012-02-12T12:20:20Z
shiba (satoshi shiba)
shiba@rvm.jp
<ul></ul><p>Hi,</p>
<p>Yusuke ENDOH wrote:</p>
<blockquote>
<p>Hi,</p>
<p>2011/6/23 Hiroshi Nakamura <a href="mailto:nakahiro@gmail.com" class="email">nakahiro@gmail.com</a>:</p>
<blockquote>
<p>Anyone can imagine a downside of this? It could not work as expected<br>
if the result of rb_file_expand_path <em>changes</em> during require (adding<br>
a new file during require should work.) Do we need to care such a<br>
case?</p>
</blockquote>
<p>As far as I look the patch, paths that are once stored in cache<br>
will never be collected. It may be considered as a kind of<br>
memory leaks.</p>
<p>I guess that it is not a problem in casual cases. But I'm not sure<br>
if it is really OK.</p>
</blockquote>
<p>May I ask current status of this problem?<br>
"rb_file_expand_path()" is still a bottleneck of "rb_require_safe()".<br>
So, I'm interested in this problem.</p>
<p>If memory leak is a problem, how about collecting strings at the time of mark phase.<br>
I made a patch which is based on Shyouhei's patch to resolve memory leak.<br>
A patch( <a href="https://gist.github.com/1805919" class="external">https://gist.github.com/1805919</a> ) collects cached strings at the time of mark phase.</p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=23740
2012-02-13T09:29:22Z
luislavena (Luis Lavena)
luislavena@gmail.com
<ul></ul><p>satoshi shiba wrote:</p>
<blockquote>
<p>May I ask current status of this problem?<br>
"rb_file_expand_path()" is still a bottleneck of "rb_require_safe()".<br>
So, I'm interested in this problem.</p>
<p>If memory leak is a problem, how about collecting strings at the time of mark phase.<br>
I made a patch which is based on Shyouhei's patch to resolve memory leak.<br>
A patch( <a href="https://gist.github.com/1805919" class="external">https://gist.github.com/1805919</a> ) collects cached strings at the time of mark phase.</p>
</blockquote>
<p>Please see Feature <a class="issue tracker-2 status-5 priority-4 priority-default closed" title="Feature: Cache expanded_load_path to reduce startup time (Closed)" href="https://redmine.ruby-lang.org/issues/5767">#5767</a>, proposed usage of cached expanded paths:</p>
<p><a href="https://bugs.ruby-lang.org/issues/5767" class="external">https://bugs.ruby-lang.org/issues/5767</a></p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=23742
2012-02-13T09:43:44Z
shiba (satoshi shiba)
shiba@rvm.jp
<ul></ul><p>Luis Lavena wrote:</p>
<blockquote>
<p>satoshi shiba wrote:</p>
<blockquote>
<p>May I ask current status of this problem?<br>
"rb_file_expand_path()" is still a bottleneck of "rb_require_safe()".<br>
So, I'm interested in this problem.</p>
<p>If memory leak is a problem, how about collecting strings at the time of mark phase.<br>
I made a patch which is based on Shyouhei's patch to resolve memory leak.<br>
A patch( <a href="https://gist.github.com/1805919" class="external">https://gist.github.com/1805919</a> ) collects cached strings at the time of mark phase.</p>
</blockquote>
<p>Please see Feature <a class="issue tracker-2 status-5 priority-4 priority-default closed" title="Feature: Cache expanded_load_path to reduce startup time (Closed)" href="https://redmine.ruby-lang.org/issues/5767">#5767</a>, proposed usage of cached expanded paths:</p>
<p><a href="https://bugs.ruby-lang.org/issues/5767" class="external">https://bugs.ruby-lang.org/issues/5767</a></p>
</blockquote>
<p>Thank you for telling me.<br>
I have missed that ticket.</p>
Ruby master - Bug #3924: Performance bug (in require?)
https://redmine.ruby-lang.org/issues/3924?journal_id=33140
2012-11-20T09:40:57Z
h.shirosaki (Hiroshi Shirosaki)
h.shirosaki@gmail.com
<ul><li><strong>Status</strong> changed from <i>Assigned</i> to <i>Closed</i></li></ul><p>Fixed by <a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Bug: require is slow in its bookkeeping; can make Rails startup 2.2x faster (Closed)" href="https://redmine.ruby-lang.org/issues/7158">#7158</a>.</p>