review (cleanup) boot -> standard JRuby extension loading#5205
review (cleanup) boot -> standard JRuby extension loading#5205
Conversation
- moved *extend_proxy* to native (although its very useless atm) - *print_class* has been broken - failing with a NoMethodError for quite a long time (all 9K), thus a good excuse to remove
| addLazyBuiltin("java.rb", "java", "org.jruby.javasupport.Java"); | ||
| addLazyBuiltin("jruby.rb", "jruby", "org.jruby.ext.jruby.JRubyLibrary"); | ||
| addLazyBuiltin("jruby/util.rb", "jruby/util", "org.jruby.ext.jruby.JRubyUtilLibrary"); | ||
| addLazyBuiltin("jruby/type.rb", "jruby/type", "org.jruby.ext.jruby.JRubyTypeLibrary"); |
There was a problem hiding this comment.
org.jruby.ext.jruby.JRubyTypeLibrary class was gone for quite some time - as explained in the commit
| Class realFacadeClass = Class.forName("org.jruby.util.SunSignalFacade"); | ||
| return (SignalFacade)realFacadeClass.newInstance(); | ||
| } catch(Throwable e) { | ||
| return org.jruby.util.SunSignalFacade.class.newInstance(); |
There was a problem hiding this comment.
somehow ended up hitting this Class.forName spot (at least) twice while 'naively' profiling.
likely a false alarm, but I still did review the class and its signal.rb pieces ...
enebo
left a comment
There was a problem hiding this comment.
requiring both 'require "java"' or 'require "jruby"' is semantically required even though they both incidentally worked.
You have some code with if and else if not joined with {. Same for catch.
Depending on autoload feels weird to me but we run rails so it is ok by me I guess.
esp. since a JRuby.runtime causes JI loading of org.jruby internals `JRuby.load_ext` to be used by gems that get loaded on boot - RubyGems (e.g. openssl and psych)
... we're no longer exactly matching up with MRI's *prelude.rb* anyway
with MRI ... this also isn't an exact match *gem_prelude.rb* match
…ding JI" This reverts commit 98e97ba.
... we simply generate the Java listener on demand as its to be used
|
OK, cleaned up the code-style.
at this point than we could remove |
seems to makes sense as using JRuby internals make the runtime "dirty" e.g. ObjectSpace will find invalid class pieces such as for UNDEF still, its kind of a small "break" in term-of backwards compatibility although most scripts floating around do the explicit require 'jruby' before scripting with internals such as `JRuby.runtime` left-over from #5205 as noted at #5233
... while looking into
jrubyloading performance and making JI native, managed to review the boot process.there was some legacy and obvious things that we do not need to parse or can afford to lazy load instead.
the most interesting part (some cleanup noise through the commits) is an extension loading support :
JRuby::Util.load_ext(...)or the more officialJRuby.load_ext(ideally afterrequire 'jruby')avoiding
JRuby.runtime, and JI proxy loading the bootstrap Java class, JRuby is able to boot 5+% faster.note, that we already delivered a ~ 5% improvement in 9.2.0 (over 9.1) this is measured compared to 9.2.0
also this is very useful internally since self-reflecting (JI loading
org.jrubypieces) leads to a weird internal state (there were already some work-arounds on that from e.g.subclassesreturning weird junk dueUNDEF).NOTE: things to consider
require 'jruby'now, its auto-loaded from kernel which was likely not intentional - should only auto
require 'java'require 'jruby'(most exts tend to have an explicit
require 'jruby'or use the newload_extmechanism)load_extchanges to gems we tend to boot on every run (due RGs) :jruby-openssl,jruby-readline, psych, jar-dependencies... and get them to propagate releases (so this PR delivers the promised loading speed improvement)