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.

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

jruby (ext) loading left-overs to consider #5233

Closed
kares opened this Issue Jun 28, 2018 · 6 comments

Comments

Projects
None yet
2 participants
@kares
Copy link
Member

kares commented Jun 28, 2018

... left-overs from #5205

  • do we force users extension authors to do a require 'jruby' ... loosely needed atm
    (most exts tend to have an explicit require 'jruby' or use the new load_ext mechanism)

  • JRuby auto-loads the java library (since ~ 1.6/1.7) think about NOT auto loading require 'jruby'
    now, its auto-loaded from kernel which was likely not intentional (should only auto require 'java')?

  • document extension loading (wiki): require 'jruby'; JRuby.load_ext('foo.bar.MyExtClass')

  • need to propagate load_ext changes to gems we tend to boot often (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)

@kares kares added the internal label Jun 28, 2018

@kares kares added this to the JRuby 9.2.1.0 milestone Jun 28, 2018

@enebo

This comment has been minimized.

Copy link
Member

enebo commented Jun 28, 2018

@kares will load_ext work on 9.1.x? It is not unsurmountable if it won't but this would make it much easier for extensions to update. 9.1.x is probably living until beginning of 2019 at least.

@kares

This comment has been minimized.

Copy link
Member Author

kares commented Jun 28, 2018

@enebo we could back-port if it makes sense ...
but than I am not sure we need that - all of these gems handle load_ext backwards compatibly.
your assumption would be that it would help adoption - migrating gems to an official way of ext loading?

9.1.x is probably living until beginning of 2019 at least.

that "long" for 9.1 - thought you guys do not plan on supporting multiple 2.x versions in parallel.

@enebo

This comment has been minimized.

Copy link
Member

enebo commented Jun 29, 2018

@kares I guess I did not realize load_ext worked on 9.1. If so then no reason to backport. Right now most exts I have written do this: 'Java::oj.OjLibrary.new.load(JRuby.runtime, false)'.

At this point I think we have reverted to the idea of supporting 2 versions only. The problem with only one 9.x is that we cannot just say 9.2.0.0 is out use it! We know it takes several point releases to iron over the work made (2.3->2.5 was larger than we actually planned since we skipped 2.4 too). I am hoping 9.2.x matures faster than 9.1.x but there has to be some overlap so people don't feel dropped and angry about it.

@kares

This comment has been minimized.

Copy link
Member Author

kares commented Jun 29, 2018

OK, just to clarify - load_ext does not work in 9.1 (unless backported) but the way we add support for it a newer gems is backward compatible :

+if JRuby::Util.respond_to?(:load_ext) # JRuby 9.2
+  JRuby::Util.load_ext('org.jruby.ext.openssl.OpenSSL')
+else; require 'jruby'
+  org.jruby.ext.openssl.OpenSSL.load(JRuby.runtime)
+end

(JRuby::Util has existed as an internal pre-loaded piece to help boot - somehow resurrected the concept just so "internal" loading works even without require 'jruby') ... thus your ext would do the "official" way :

require 'jruby'
if JRuby.respond_to?(:load_ext)
  JRuby.load_ext('oj.OjLibrary')
else
  Java::oj.OjLibrary.new.load(JRuby.runtime, false)
end

BTW some questions still remain e.g. whether to remove require 'jruby' happening on JRuby boot -
no opinions there really other than loading "less" so eventually JRuby could also NOT auto require 'java' at some point in the future ... maybe :)

@enebo

This comment has been minimized.

Copy link
Member

enebo commented Jul 2, 2018

@kares thanks. I suppose for backwards compat we need to do new and old way anyways so this is some good boilerplate which should be on our wiki somewhere too.

kares added a commit that referenced this issue Oct 3, 2018

upgrade jar_dependencies to 0.4.0 + use polyglot-maven 0.3.1
... jar_dependencies upgrade is due to JRuby (9.2) load_ext mechanism

#5233

kares added a commit that referenced this issue Oct 3, 2018

upgrade jar_dependencies to 0.4.0 + use polyglot-maven 0.3.1 (#5344)
... jar_dependencies upgrade is due to JRuby (9.2) load_ext mechanism

#5233
@kares

This comment has been minimized.

Copy link
Member Author

kares commented Oct 23, 2018

jar_dependencies has been updated already ^^ and now psych 3.0.3 is also in d18d98d
... so all the left-overs are handled -> JRuby shouldn't do any self reflection on its own, at least with its default commands such as gem, irb

@kares kares closed this Oct 23, 2018

kares added a commit that referenced this issue Oct 23, 2018

only require 'java', users should explicitly require 'jruby'
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.