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

Migrate Ruboto to JRuby 9K #4004

Closed
donv opened this issue Jul 9, 2016 · 55 comments
Closed

Migrate Ruboto to JRuby 9K #4004

donv opened this issue Jul 9, 2016 · 55 comments

Comments

@donv
Copy link
Member

donv commented Jul 9, 2016

@headius @enebo
With the announcement of EOL of JRuby 1.7, I hope we can push Ruboto to use JRuby 9000.

I can do the testing and packaging changes needed in Ruboto, but the JRuby changes needed have proven too much for me.

JRuby master is being tested in travis: https://travis-ci.org/ruboto/ruboto
You should be able to trigger new builds when there are JRuby changes.

@donv donv added this to the JRuby 9.1.4.0 milestone Jul 9, 2016
@headius
Copy link
Member

headius commented Jul 9, 2016

👍

I'm trying to read between the thousands of lines in that output. I know of some things that need to be changed, but I'm not sure about everything...especially Java 7isms that have crept into things. How far back are we supporting? I know recent Android have support for much of Java 7, yes?

@headius
Copy link
Member

headius commented Jul 9, 2016

A bit of brainstorming.

The biggest hit, if we at least require Android Java 7 support, is that invokedynamic/java.lang.invoke are not available. Invokedynamic we can do without; I think there's only a couple features that always use invokedynamic, and they don't have to. The java.lang.invoke package will be harder.

I think it might be possible to implement method handles using reflection. The invocation and lookup are basically taken care of. The adapters are all simple to implement and can just be generic wrappers. It's a bit of work, but it would be a useful library for others. Come to think of it...maybe someone has started this already?

@donv
Copy link
Member Author

donv commented Jul 9, 2016

Lots of Java 7 features are supported, and I think that is done in the Android tooling so we don't have to worry about that for now.

InvokeDynamic byte code is not available.

The main problem are JVM APIs as you have mentioned, and that they are eager loaded even if they will not be used.

@donv
Copy link
Member Author

donv commented Jul 9, 2016

And I can definitely help sifting through the logs to identify problems. 😄

@oneiros
Copy link

oneiros commented Jan 26, 2017

The java.lang.invoke package will be harder.

Do you still think this is even possible? I spent some time looking into this and the package is used all over the place. A quick grep finds imports in ~50 files and then there are the imlicit usages, e.g. invokebinder (imported in 12 files).

If the answer is still yes:

I think it might be possible to implement method handles using reflection. The invocation and lookup are basically taken care of. The adapters are all simple to implement and can just be generic wrappers.

Could you maybe elaborate a bit more on this?

I would not mind spending some more time on this, but my Java knowledge is a bit rusty and I am only starting to find my way around the jruby code base.

Anyway, any additional information is highly appreciated. I am constantly amazed by the jruby project. Keep up the good work.

And by the way: I guess #2471 and #2982 are kind of duplicates of this and might be closed as such.

@headius
Copy link
Member

headius commented Jan 26, 2017

@oneiros The idea I had would be to provide a library that looks mostly the same as java.lang.invoke but uses reflection under the covers. That might be a big project, though, so I think we want to look at a more conservative approach short term.

However the situation may not be as bad as we think. On Android, we still don't JIT code. We'd like to in the future, but not being able to now means we're always using JRuby's interpreter(s), which don't use invokedynamic. And if we get to a point where we do want to JIT, we can deal with the few cases that always use indy.

So I think what would help this process along is if someone started going step-by-step to build and deploy JRuby on Android and fixing/reporting the issues they run into as they come up. Does that seem like a good path forward?

@oneiros
Copy link

oneiros commented Jan 27, 2017

First of all, thanks a lot for your reply. I am probably not very helpful, so I doubly appreciate you getting back to me.

However the situation may not be as bad as we think. On Android, we still don't JIT code. We'd like to in the future, but not being able to now means we're always using JRuby's interpreter(s), which don't use invokedynamic.

That is true. But as you stated yourself above, invokedynamic is not the big problem, MethodHandle is.

The idea I had would be to provide a library that looks mostly the same as java.lang.invoke but uses reflection under the covers.

This kind of exists in the form of Rémi Forax's jsr292-backport (see https://code.google.com/archive/p/jvm-language-runtime/). It even includes a commandline tool/ant task to retroactively change existing .jar-files to use the backport instead of the actual java.lang.invoke classes.

In theory this could be wonderful: jruby code would need no or minimal changes and ruboto could simply execute this task during installation of the jruby jar files.

Not surprisingly this fails in practice on a large and diverse code base such as jruby's. (To be precise, it complains about not finding com.kenai.jffi.Array for some reason.)

Still this code could serve as a starting point for a (minimal) implementation of the needed classes. But I have so far refrained from even looking at it, because the license is not clear. The google code page says LGPL, but I could not find anything about it in the repository.

So I think what would help this process along is if someone started going step-by-step to build and deploy JRuby on Android and fixing/reporting the issues they run into as they come up. Does that seem like a good path forward?

I guess you are right. This is probably tedious but also very pragmatic. I will give it a try.

(By the way, if anything above sounds like I know what I am doing - I do not. So thanks again for your patience.)

@oneiros
Copy link

oneiros commented Jan 27, 2017

Small additional find: There is no java.lang.ClassValue on Android ☹️

@headius
Copy link
Member

headius commented Jan 27, 2017

Ok, so we need to isolate the use of ClassValue. I believe that logic already in JRuby, via the invokedynamic.class.values option, but then we still need to be able to compile. We have used jsr292-mock for that, but it doesn't include ClassValue. Something to add.

oneiros added a commit to oneiros/jruby that referenced this issue Feb 16, 2017
See jruby#4004. Everything java.lang.invoke related is not
available on android, so for now revert to the old
behavior here.

This still makes some tests fail, but might suffice
to run on android.
oneiros added a commit to oneiros/jruby that referenced this issue Feb 16, 2017
Instead of java.lang.ClassValue which ist not available on
android. See jruby#4004.
oneiros added a commit to oneiros/jruby that referenced this issue Feb 16, 2017
Could not find a usage for this. And since MethodHandles
is not available on android, I simply removed it for the time
being. See jruby#4004.
oneiros added a commit to oneiros/jruby that referenced this issue Feb 16, 2017
oneiros added a commit to oneiros/jruby that referenced this issue Feb 16, 2017
Instead of java.lang.ClassValue which ist not available on
android. See jruby#4004.
oneiros added a commit to oneiros/jruby that referenced this issue Feb 16, 2017
This of course destroys any gains of caching the idTest, but
gets rid of MethodHandle et al in RubyModule which is
important to run on android. See jruby#4004.
oneiros added a commit to oneiros/jruby that referenced this issue Feb 16, 2017
Since this will not run on android is not being used
currently anyway, just comment it out for now. See jruby#4004.
@headius
Copy link
Member

headius commented Apr 29, 2017

#4431 made some changes to toplevel to properly "wrap" loads. Could be a suspect.

@donv
Copy link
Member Author

donv commented Apr 29, 2017

The ScriptingContainer instance is created at https://github.com/ruboto/test_app_9k/blob/master/src/org/ruboto/JRubyAdapter.java#L274

It is reflection based since we have the option of loading JRuby from the RubotoCore app. It should be using localContextScope SINGLETON and localVariableBehavior TRANSIENT.

I found a workaround for setting the constant: Setting a global variable and running Ruby code to set the constant based on the global variable.

@donv
Copy link
Member Author

donv commented Apr 29, 2017

@headius With the workaround, I have a working app! 🍾 🎉

@donv
Copy link
Member Author

donv commented Apr 29, 2017

@headius I have to take off 😞 Great progress, though! Hopefully we can fix the setting of the constant and then look at what changes should be merged into master.

I feel the changes that include the dex related code should not be in JRuby, but in Ruboto.

@headius
Copy link
Member

headius commented Apr 29, 2017

Woohoo!

I'll start reviewing the other changes so we get try to get them in for 9.1.9.0 or if they're too invasive 9.2.

@donv
Copy link
Member Author

donv commented Apr 30, 2017

OK, so I see the use of dexmaker is integrated into DynamicScopeGenerator.java. Do you see a good way to extract it from there? Or is it OK to include it in core JRuby?

@oneiros
Copy link

oneiros commented May 2, 2017

OK, so I see the use of dexmaker is integrated into DynamicScopeGenerator.java. Do you see a good way to extract it from there? Or is it OK to include it in core JRuby?

FWIW, when I did that, I was not aware of @headius' dexclient. If it is able to reliably produce dex code from the generated JVM bytecode in DynamicScopeGenerator.java then yes, it should probably not be in core JRuby. There would just need to be an interface for ruboto to hook into there and convert the bytecode to dex, similar to what is already done with JavaProxyClassFactory.

As for the other progress you made: That is wonderful 😄

As mentioned above, my efforts were largely a proof of concept. It was good for highlighting issues, but nothing I expected to ever be merged into JRuby. But if one of my commits does end up in JRuby master, I will probably frame it and hang it on my wall 🎉

On a more serious note: I wonder what the long term plan for android compatibility is. Fixing the issues I encountered is great, but afaict every new commit runs the risk of introducing new incompatibities. The JVM and android platforms have already diverged quite a lot, and given the legal troubles between Google and Oracle, I doubt that will change anytime soon. Is JRuby really that committed to android support that you will refrain from using Java 7/8 (and soon 9) features and APIs that are totally established in the Java world but simply do not exist on android?

Or is there maybe another way, making multi-platform support more explicit, maybe even with conditional compilation or at least more modularization, better isolating parts of the code that may use newer features?

@donv
Copy link
Member Author

donv commented May 5, 2017

Moved the changes in DynamicScopeGenerator to a custom copy in Ruboto. Also moved dexmaker jar to Ruboto. This simplifies the changes in JRuby a lot. @headius have you had time to look at them?

Having a custom copy of DynamicScopeGenerator is just a first step. We should follow the pattern of JavaProxyClassFactory as a next step, but that can happen after merge.

@oneiros You will get your name on this change 😄

As for worrying about the future: Android is so closely linked to Java, and there are so many developers wishing to use Java libraries with their Android apps, so I think this will not be a big problem in general. InvokeDynamic is the exception, but I guess JRuby will have an option to run without InDy for a long time.

@donv
Copy link
Member Author

donv commented May 5, 2017

I'll do a rebase and squash of the ruboto_9k branch and create a pull request.

donv pushed a commit that referenced this issue May 5, 2017
See #4004. Everything java.lang.invoke related is not
available on android, so for now revert to the old
behavior here.

This still makes some tests fail, but might suffice
to run on android.

Use jruby's ClassValue.

Instead of java.lang.ClassValue which ist not available on
android. See #4004.

Remove seemingly unused constant.

Could not find a usage for this. And since MethodHandles
is not available on android, I simply removed it for the time
being. See #4004.

Revert to old behavior, when indy disabled.

Just like in 8db25932200fb3ed7160066b184e6db2e29f1921

Do not init JITCompiler if it is not used.

See #4004.

Use jruby's ClassValue.

Instead of java.lang.ClassValue which ist not available on
android. See #4004.

Move idTest to where it is actually needed.

This of course destroys any gains of caching the idTest, but
gets rid of MethodHandle et al in RubyModule which is
important to run on android. See #4004.

Remove unused Method.

Since this will not run on android is not being used
currently anyway, just comment it out for now. See #4004.

Revert "relax seek-channel type requirement from FileChannel to SeekableByteChannel"

This reverts commit dc48f34.

No SeekableByteChannel on android. See #4004.

Work around missing java.nio libs on android.

See #4004.

Replace ClassValue with internal implementation.

Because java.lang.ClassValue is not available on android.
See #4004.

Add minimal MethodHandle implementation.

java.lang.invoke.Method* classes are not available on
android. See #4004.

Add code generation for android.

See #4004.

Move nullToNil out of the way.

To not have MethodHandles in Ruby.java as this
is not available on android. See #4004.

Guard against missing JITCompiler.

Necessry as JITCompiler will not get instantiated on android.
See #4004.

Remove unused methods getIdTest + newIdTest

Move Android logic to the Ruboto project

Removed extra white space

Remove the maven repository for dexmaker
donv pushed a commit that referenced this issue May 5, 2017
Return ObjectIdentityInvalidator if indy disabled.

See #4004. Everything java.lang.invoke related is not
available on android, so for now revert to the old
behavior here.

This still makes some tests fail, but might suffice
to run on android.

Use jruby's ClassValue.

Instead of java.lang.ClassValue which ist not available on
android. See #4004.

Remove seemingly unused constant.

Could not find a usage for this. And since MethodHandles
is not available on android, I simply removed it for the time
being. See #4004.

Revert to old behavior, when indy disabled.

Just like in 8db25932200fb3ed7160066b184e6db2e29f1921

Do not init JITCompiler if it is not used.

See #4004.

Use jruby's ClassValue.

Instead of java.lang.ClassValue which ist not available on
android. See #4004.

Move idTest to where it is actually needed.

This of course destroys any gains of caching the idTest, but
gets rid of MethodHandle et al in RubyModule which is
important to run on android. See #4004.

Remove unused Method.

Since this will not run on android is not being used
currently anyway, just comment it out for now. See #4004.

Revert "relax seek-channel type requirement from FileChannel to SeekableByteChannel"

This reverts commit dc48f34.

No SeekableByteChannel on android. See #4004.

Work around missing java.nio libs on android.

See #4004.

Replace ClassValue with internal implementation.

Because java.lang.ClassValue is not available on android.
See #4004.

Add minimal MethodHandle implementation.

java.lang.invoke.Method* classes are not available on
android. See #4004.

Add code generation for android.

See #4004.

Move nullToNil out of the way.

To not have MethodHandles in Ruby.java as this
is not available on android. See #4004.

Guard against missing JITCompiler.

Necessry as JITCompiler will not get instantiated on android.
See #4004.

Remove unused methods getIdTest + newIdTest

Move Android logic to the Ruboto project

Removed extra white space

Remove the maven repository for dexmaker
@headius
Copy link
Member

headius commented May 5, 2017

@donv I will review the new PR by Monday!

@oneiros
Copy link

oneiros commented May 8, 2017

InvokeDynamic is the exception.

Sadly, it is not. There is a handful of APIs that are simply not present on android (e.g. parts of java.nio). And the APIs that are present sometimes sport subtle differences.

Strangely enough, I stumbled upon this here yesterday: https://github.com/retropiler/retropiler. It is early days but might come handy in the future.

@headius
Copy link
Member

headius commented May 16, 2018

Android now supports invokedynamic, so there may be no reason we can't fully update to latest JRuby.

Still with us @donv?

@headius headius modified the milestones: JRuby 9.2.0.0, JRuby 9.2.1.0 May 16, 2018
@donv
Copy link
Member Author

donv commented May 18, 2018

Yes, I am still in!

@donv
Copy link
Member Author

donv commented May 23, 2018

@headius Do you have references to the invokedynamic support, like what version of Android it is introduced in?

@donv
Copy link
Member Author

donv commented May 23, 2018

Looks like it was introduced in Android O – API level 26

@headius
Copy link
Member

headius commented Oct 26, 2018

@donv It may be possible these days to get unmodified JRuby running, with invokedynamic, on Android.

@headius headius modified the milestones: JRuby 9.2.1.0, JRuby 9.3.0.0 Oct 26, 2018
@donv
Copy link
Member Author

donv commented Dec 20, 2018

I am restarting this effort now. Hopefully we can complete it during the holidays.

I have tried including jruby-complete-9.2.5.0.jar in a project, and the project builds fine using current multi-dex tooling.

During startup of JRuby is fails with

java.lang.ClassNotFoundException: Didn't find class "java.lang.invoke.SwitchPoint"

According to this source, SwitchPoint is not implemented on Android:

https://developer.android.com/reference/java/lang/invoke/package-summary

We should try not to load SwitchPoint unless it will be used. I have set the compile mode to OFF. Should this disable use of SwitchPoint? @headius ?

@headius
Copy link
Member

headius commented Apr 7, 2021

@donv I don't know if you are still out there but I would love to help close the book on the Android-specific patches and get everything working with JRuby 9.3. Contact me if you are available to help here!

@donv
Copy link
Member Author

donv commented Apr 7, 2021

I am still around, and we still have two Ruboto apps in production, one at Oslo Airport Gardermoen in the busses transferring passengers between the gates and the flights.

I can definitely help. All I need is motivation :) Your interrest and attention is much appreciated.

@headius
Copy link
Member

headius commented Apr 7, 2021

@donv This is a very old issue. Perhaps we should open a new one and start trying to shove the remaining patches into PRs for JRuby proper? I want to call this done and have Ruboto set up with JRuby 9.3 at release!

@headius
Copy link
Member

headius commented Apr 7, 2021

Executive decision: JRuby 9k has been integrated into Ruboto, but with patches. I will open a separate issue to get those patches back into JRuby for 9.3, so Ruboto can use it unmodified.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants