-
-
Notifications
You must be signed in to change notification settings - Fork 925
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
Comments
👍 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? |
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? |
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. |
And I can definitely help sifting through the logs to identify problems. 😄 |
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:
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. |
@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? |
First of all, thanks a lot for your reply. I am probably not very helpful, so I doubly appreciate you getting back to me.
That is true. But as you stated yourself above, invokedynamic is not the big problem, MethodHandle is.
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.
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.) |
Small additional find: There is no java.lang.ClassValue on Android |
Ok, so we need to isolate the use of ClassValue. I believe that logic already in JRuby, via the |
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.
Instead of java.lang.ClassValue which ist not available on android. See jruby#4004.
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.
Instead of java.lang.ClassValue which ist not available on android. See jruby#4004.
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.
Since this will not run on android is not being used currently anyway, just comment it out for now. See jruby#4004.
#4431 made some changes to toplevel to properly "wrap" loads. Could be a suspect. |
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 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. |
@headius With the workaround, I have a working app! 🍾 🎉 |
@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. |
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. |
OK, so I see the use of dexmaker is integrated into |
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 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? |
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 @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. |
I'll do a rebase and squash of the ruboto_9k branch and create a pull request. |
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
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
@donv I will review the new PR by Monday! |
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. |
Android now supports invokedynamic, so there may be no reason we can't fully update to latest JRuby. Still with us @donv? |
Yes, I am still in! |
@headius Do you have references to the invokedynamic support, like what version of Android it is introduced in? |
Looks like it was introduced in Android O – API level 26 |
@donv It may be possible these days to get unmodified JRuby running, with invokedynamic, on Android. |
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
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 |
@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! |
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. |
@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! |
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. |
@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.
The text was updated successfully, but these errors were encountered: