Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Load/require logic lacks path caching and indexing found in MRI #2794

Closed
headius opened this issue Apr 2, 2015 · 10 comments
Closed

Load/require logic lacks path caching and indexing found in MRI #2794

headius opened this issue Apr 2, 2015 · 10 comments
Labels
core JRuby 9000
Milestone
JRuby 9.3.0.0

Comments

@headius
Copy link
Member

headius commented Apr 2, 2015

MRI has a number of caches and indexes that improve the performance of load/require searching. We lack most of those caches, or have poor implementations of them.

The two key bits I know of:

  • The entries in the load path are made into absolute paths and held in a separate index used for file resolution.
  • The resolved requires are absolutized for LOADED_FEATURES and a mapping is maintained between the require name and the absolute name. This index is updated whenever LOADED_FEATURES is seen to have changed.

We need to implement our loading logic so that it performs similar mappings, both to improve lookup performance and to better match MRI behavior with regards to full paths.

This may push to post-9k since it does not fix any known compatibility issues.
@headius headius added this to the JRuby 9.0.0.0 milestone Apr 2, 2015
@mkristian
Copy link
Member 

jruby -e '$LOAD_PATH.each { |d| print d; puts File.exists?( d ) }'

shows two non-existing directories on the default load_path (only one for jruby-1.7.x).

this index should take into account that load_path entries do not exist ?!

@headius
Copy link
Member Author

headius commented Apr 6, 2015

What the...ok, those aren't even valid paths. Something's off.

@headius
Copy link
Member Author

headius commented Apr 6, 2015

Oops, I thought the true/false were part of the path.

But yeah, there's no reason to add paths that don't even exist.

@headius
Copy link
Member Author

headius commented Apr 17, 2015

Circling back to the two non-existent paths:

site_ruby turns out to be very important, as RubyGems uses it to know where to insert gem libs into the load path. Whether it exists or not, we need it there for RG.

The other path was our old "shared" stdlib dir that doesn't exist now, so I removed it.

headius added a commit that referenced this issue May 8, 2015
@headius
Add another layer of try/finally around rescue/ensure to reset $!.
2880c73 
Fixes #2794.
@headius headius mentioned this issue May 8, 2015
Closed
headius added a commit that referenced this issue May 8, 2015
@headius
Add another layer of try/finally around rescue/ensure to reset $!.
2a82982 
Fixes #2794.
@headius
Copy link
Member Author

headius commented May 8, 2015

I'm also seeing a lot of this when I run with -d... I think we've got some problems with circular require logic or we're simply warning too much:

/Users/headius/projects/jruby/lib/ruby/stdlib/rubygems/core_ext/kernel_require.rb:54: warning: loading in progress, circular require considered harmful - /Users/headius/projects/jruby/lib/ruby/stdlib/ant/ant.rb
                      require at org/jruby/RubyKernel.java:941
                      require at /Users/headius/projects/jruby/lib/ruby/stdlib/rubygems/core_ext/kernel_require.rb:54
                        <top> at /Users/headius/projects/jruby/lib/ruby/stdlib/ant/target.rb:2
                      require at org/jruby/RubyKernel.java:941
                      require at /Users/headius/projects/jruby/lib/ruby/stdlib/rubygems/core_ext/kernel_require.rb:54
                        <top> at /Users/headius/projects/jruby/lib/ruby/stdlib/ant/ant.rb:1
                      require at org/jruby/RubyKernel.java:941
                      require at /Users/headius/projects/jruby/lib/ruby/stdlib/rubygems/core_ext/kernel_require.rb:54
                        <top> at /Users/headius/projects/jruby/lib/ruby/stdlib/ant/ant.rb:10
                      require at org/jruby/RubyKernel.java:941
                      require at /Users/headius/projects/jruby/lib/ruby/stdlib/rubygems/core_ext/kernel_require.rb:54
                        <top> at /Users/headius/projects/jruby/lib/ruby/stdlib/ant.rb:1
                         load at org/jruby/RubyKernel.java:959
                       (root) at /Users/headius/projects/jruby/lib/ruby/stdlib/ant.rb:64
                       (root) at /Users/headius/projects/jruby/rakelib/commands.rake:1
                       (root) at /Users/headius/projects/jruby/rakelib/commands.rake:3
                       (root) at /Users/headius/projects/jruby/lib/ruby/gems/shared/gems/rake-10.4.2/lib/rake/rake_module.rb:1
       block in load_rakefile at /Users/headius/projects/jruby/lib/ruby/gems/shared/gems/rake-10.4.2/lib/rake/rake_module.rb:28
                         load at /Users/headius/projects/jruby/lib/ruby/gems/shared/gems/rake-10.4.2/lib/rake/default_loader.rb:10
                 load_imports at /Users/headius/projects/jruby/lib/ruby/gems/shared/gems/rake-10.4.2/lib/rake/application.rb:767
   block in raw_load_rakefile at /Users/headius/projects/jruby/lib/ruby/gems/shared/gems/rake-10.4.2/lib/rake/application.rb:697
                load_rakefile at /Users/headius/projects/jruby/lib/ruby/gems/shared/gems/rake-10.4.2/lib/rake/application.rb:94
  standard_exception_handling at /Users/headius/projects/jruby/lib/ruby/gems/shared/gems/rake-10.4.2/lib/rake/application.rb:176
                        <top> at /Users/headius/projects/jruby/lib/ruby/gems/shared/gems/rake-10.4.2/lib/rake/application.rb:93
                         load at org/jruby/RubyKernel.java:959
                        <top> at /Users/headius/projects/jruby/bin/rake:23

@headius headius closed this as completed in d6c5355 May 9, 2015
@headius
Copy link
Member Author

headius commented May 9, 2015

This was not supposed to be closed. Bad commit.

@headius headius reopened this May 9, 2015
@enebo enebo modified the milestone: JRuby 9.0.0.0 Jul 14, 2015
This was referenced Nov 20, 2018
Merged
Closed
This was referenced Nov 27, 2018
Open
Closed
@headius
Copy link
Member Author

headius commented Nov 27, 2018

From #408, the original MRI bug for this feature was in http://bugs.ruby-lang.org/issues/5767

@enebo enebo added this to the JRuby 9.2.7.0 milestone Jan 9, 2019
@headius headius mentioned this issue Mar 8, 2019
Closed
@headius
Copy link
Member Author

headius commented Mar 28, 2019

The pain...the pain of reading CRuby code. But this is close. PR coming soon.

@headius
Copy link
Member Author

headius commented Apr 8, 2019

Work continues on this but it will move to 9.2.8. The basic caching logic has been translated from MRI but it's poorly-adapted to Java and JRuby. I will rewrite it before submitting a PR.

@headius headius modified the milestones: JRuby 9.2.7.0, JRuby 9.2.8.0 Apr 8, 2019
@headius headius mentioned this issue Apr 25, 2019
Closed
@headius headius modified the milestones: JRuby 9.2.8.0, JRuby 9.2.9.0 Aug 1, 2019
@headius headius modified the milestones: JRuby 9.2.9.0, JRuby 9.2.10.0 Oct 16, 2019
@enebo enebo modified the milestones: JRuby 9.2.10.0, JRuby 9.3.0.0 Feb 11, 2020
@headius
Copy link
Member Author

headius commented May 24, 2021

Most of this logic was "ported" in #5764. There may be additional improvements possible.

@headius headius closed this as completed May 24, 2021
@headius headius mentioned this issue Sep 22, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
core JRuby 9000
Projects
None yet
Milestone
JRuby 9.3.0.0
Development

No branches or pull requests
3 participants
@headius @enebo @mkristian