Add cache for $LOAD_FEATURES and $LOAD_PATH#2353
Add cache for $LOAD_FEATURES and $LOAD_PATH#2353dbussink merged 2 commits intorubinius:masterfrom LTe:cache-for-require
Conversation
|
Looking at this, I think it's better to compose LoadPath with an array than to inherit from Array. Is there any reason you need to inherit from Array? |
|
Also wondering, how does this affect performance of for example starting Rails? Does it help there as because of the cache? |
|
Yes, rails app boot on ~13 sec and after patch ~11 :) |
|
I'm still unsure about a custom LoadPath class, is that needed at all for this change? It doesn't seem to be needed for the LOADED_FEATURES changes right? Also, is there a reason for all the sychronization in it? Array isn't thread safe either, so is there a reason this adds it? Were there cases where this was a problem? If the thread safety of LOADED_FEATURES / LOAD_PATH is an issue, I think we should address that separately so we don't mix different changes which makes discussing and reviewing them harder. |
No, this is feature from ruby 2.0. Each element in
I'm not sure - I was based on the implementation of
Agree, I will update this pull request and I will remove all changes related to |
Rubinius will create cache for require short paths. When user
require file ie:
require 'path'
CodeLoader.loaded_features_index => { 'path' => '/real/path' }
When we try require 'path' once again Rubinius will not travel via
$LOAD_PATH because loaded_features_index include 'path'. But when user
will try require '/real/path' Rubinius will find this feature in
$LOADED_FEATURES. In the case of not finding the file anywhere Rubinius
will try find file in $LOAD_PATH and fill in cache.
When $LOADED_FEATURES change we need to invalidate cache!
|
@dbussink I updated my pull request. |
|
Before patch After patch Runner on rails app with gems: Before patch After patch |
|
I based on @headius implementation jruby/jruby#452 |
Add cache for $LOAD_FEATURES and $LOAD_PATH
Rubinius will create cache for require short paths. When user
require file ie:
require 'path'
CodeLoader.loaded_features_index => { 'path' => '/real/path' }
When we try require 'path' once again Rubinius will not travel via
$LOAD_PATH because loaded_features_index include 'path'. But when user
will try require '/real/path' Rubinius will find this feature in
$LOADED_FEATURES. In the case of not finding the file anywhere Rubinius
will try find file in $LOAD_PATH and fill in cache.
When $LOADED_FEATURES change we need to invalidate cache!
Benchmark
Before patch:
After patch: