| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
The AppCDS file is always written if --cache is specified or it's older than the JRuby JAR. If Java is 19 or newer, reading and writing are handled automatically by the JVM. Otherwise, the launcher decides whether to read or write.
|
Very nice! High priority to get this landed for the upcoming 10 release. I'll review. I chatted with @enebo as well and we agree that it would be good to include the Java version in the JSA filename, so every Java version gets its own file. Same would go for the log output when that is enabled. On my system, JRuby 10 + Java 21 JSA file is only 5.4MB so I don't think it's a big deal to leave a few lying around. We also chatted about where we should store these files. I think it's fine to go with JRuby's lib dir for now but we may want to change that to the bin dir or potentially even a user location like ~/.jruby. |
Sorry, something went wrong.
|
@mrnoname1000 You still have this marked as draft... is there more work coming? |
Sorry, something went wrong.
|
It's still a draft because I haven't implemented version in the JSA filename yet, but I'm home from work now so I can add that last feature, This PR should be working though if you want the existing work merged sooner. |
Sorry, something went wrong.
|
Go ahead and add that and then I'll review. |
Sorry, something went wrong.
|
I added the version string to the JSA filename as well as a flag to remove all the JSA files we may have generated. If we find out JSA files have compatibility guarantees between point releases, we can trim off parts of the version string to allow sharing between those releases. |
Sorry, something went wrong.
There was a problem hiding this comment.
There's some minor issues and some regressed behavior (--cache).
Sorry, something went wrong.
|
We have some command line tests under test/jruby that could be expanded for this new functionality. |
Sorry, something went wrong.
|
I'll retest locally with latest changes. 👍 |
Sorry, something went wrong.
|
So far everything is working well on JRuby 10! A wild question appears: should --rmcache delete logs too? Doesn't have to be answered for this one and I don't have a strong opinion one way or another. |
Sorry, something went wrong.
|
Issue with 9.4 under Java 11: $ jruby --logcache -e 1 Unrecognized VM option 'ArchiveClassesAtExit=/Users/headius/work/jruby94/lib/jruby-java11.0.17.jsa' Error: Could not create the Java Virtual Machine. Error: A fatal exception has occurred. Program will exit. I'm not sure this particular flag existed until after 11. Earlier CDS support required a couple flags and in some cases, multiple steps. I'm thinking maybe we don't include the automatic CDS feature in the launcher unless we're on a Java that supports one-shot "at exit" dumping. There's still the jruby-startup gem and its generate-appcds that can handle the old style flags. |
Sorry, something went wrong.
|
See here for the Java 11 equivalent, which does appear to require a two step process (dump class list, then generate CDS archive): https://docs.oracle.com/en/java/javase/11/tools/java.html#GUID-31503FCE-93D0-4175-9B4F-F6A738B2F4C4 This page documents the feature for Java 10, but it was still a "commercial" feature at that point: https://docs.oracle.com/javase/10/tools/java.htm#JSWOR-GUID-31503FCE-93D0-4175-9B4F-F6A738B2F4C4 I believe Java 13 is the first release to support the "one shot" ArchiveClassesAtExit flag: https://docs.oracle.com/en/java/javase/13/docs/specs/man/java.html#application-class-data-sharing And you appear to be right that 19 introduces the automatic flag: https://docs.oracle.com/en/java/javase/19/docs/specs/man/java.html#creating-dynamic-cds-archive-file-with--xxautocreatesharedarchive So I say we should disable the automatic dumping as well as the --cache flag when running Java 12 and earlier. It's just too much hassle right nowto support the feature with the two-step dance. @mrnoname1000 I can make this change after merging, or you can make it if you want. I'm keen to get this landed so I can do some other version-specific flags (e.g. for #8696). |
Sorry, something went wrong.
Java 13 introduces the "at exit" flag. Prior to that, creating a CDS archive requires a two-step dance of generating a class list and then using that list to create the archive. To make this feature simpler and focus on the JDK levels that support it the way we want, we'll limit it to Java 13+.
|
@mrnoname1000 I made the change... let me know if it looks ok. |
Sorry, something went wrong.
|
Hmm, my change doesn't seem to be enough. It's still trying to pass the Java 13+ flag: [] jruby94 $ jruby -e 1 Unrecognized VM option 'ArchiveClassesAtExit=/Users/headius/work/jruby94/lib/jruby-java11.0.17.jsa' Error: Could not create the Java Virtual Machine. Error: A fatal exception has occurred. Program will exit. [] jruby94 $ jruby --environment -e 1 JRuby Environment ================= JRuby executable: /Users/headius/work/jruby94/bin/jruby JRuby command line options: --environment -e 1 Current directory: /Users/headius/work/jruby94 Environment: JRUBY_HOME: /Users/headius/work/jruby94 JRUBY_OPTS: JAVA_OPTS: JAVACMD: /Library/Java/JavaVirtualMachines/temurin-11.jdk/Contents/Home/bin/java JAVA_HOME: /Library/Java/JavaVirtualMachines/temurin-11.jdk/Contents/Home Detected Java modules at /Library/Java/JavaVirtualMachines/temurin-11.jdk/Contents/Home Adding Java options from: /Users/headius/work/jruby94/bin/.jruby.java_opts Adding Java options from: /Users/headius/work/jruby94/bin/.jruby.module_opts Regenerating CDS archive at: /Users/headius/work/jruby94/lib/jruby-java11.0.17.jsa Java command line: /Library/Java/JavaVirtualMachines/temurin-11.jdk/Contents/Home/bin/java @/Users/headius/work/jruby94/bin/.jruby.java_opts @/Users/headius/work/jruby94/bin/.jruby.module_opts -Xss2048k -Djffi.boot.library.path=/Users/headius/work/jruby94/lib/jni -Djava.security.egd=file:/dev/urandom -XX:ArchiveClassesAtExit=/Users/headius/work/jruby94/lib/jruby-java11.0.17.jsa -Xlog:cds=off -Xlog:cds+dynamic=off --module-path /Users/headius/work/jruby94/lib/jruby.jar -classpath : -Djruby.home=/Users/headius/work/jruby94 -Djruby.lib=/Users/headius/work/jruby94/lib -Djruby.script=jruby -Djruby.shell=/bin/sh org.jruby.Main -e 1 |
Sorry, something went wrong.
|
Yeah definitely don't want to do that two step dance. The change you made should have bumped the required version up to 13, I can investigate in a few minutes here and I think we should add some version into to the debug logs. |
Sorry, something went wrong.
|
The AppCDS logic was linked with the modular Java logic, so any modular Java was coming up as wanting AppCDS enabled. I changed it to default to true/false depending on if it's supported, then allowing reassignment later. There is still some weirdness if --cache is passed and it isn't supported since -e 1 won't get appended, which may be surprising to the user.
Not sure, logs probably won't be big but could clutter up the lib directory. Deleting logs would probably be in the spirit of the option as long as it's documented. |
Sorry, something went wrong.
How would we test the new functionality that depends on the current version of Java? |
Sorry, something went wrong.
I'm not sure yet. We will add tests to test/jruby/test_command_line_options.rb (or whatever it's called) and run those on all the supported levels of 9.4 and 10. That's probably enough. The other option is having the test actually use multiple Javas and I have no idea how to do that cleanly. |
Sorry, something went wrong.
|
A new unfortunate wrinkle: Eclipse Temurin builds of OpenJDK on macos on Apple Silicon do not include the base JDK CDS dump, so you get cryptic errors: adoptium/adoptium-support#937 [] jruby94 $ pickjdk 17 /Library/Java/JavaVirtualMachines/temurin-17.jdk/Contents/Home [] jruby94 $ jruby --logcache -e 1 Error occurred during initialization of VM DynamicDumpSharedSpaces is unsupported when base CDS archive is not loaded [] jruby94 $ pickjdk 18 /Library/Java/JavaVirtualMachines/temurin-18.jdk/Contents/Home [] jruby94 $ jruby -e 1 Error occurred during initialization of VM -XX:ArchiveClassesAtExit is unsupported when base CDS archive is not loaded. Run with -Xlog:cds for more info. [] jruby94 $ pickjdk 19 /Library/Java/JavaVirtualMachines/temurin-19.jdk/Contents/Home [] jruby94 $ jruby -e 1 OpenJDK 64-Bit Server VM warning: -XX:ArchiveClassesAtExit is unsupported when base CDS archive is not loaded. Run with -Xlog:cds for more info. [] jruby94 $ pickjdk 20 /Library/Java/JavaVirtualMachines/temurin-20.jdk/Contents/Home [] jruby94 $ jruby -e 1 OpenJDK 64-Bit Server VM warning: -XX:ArchiveClassesAtExit is unsupported when base CDS archive is not loaded. Run with -Xlog:cds for more info. Switching to a Zulu build works correctly with all versions I tried. I expressed my discontent on the Adoptium issue above... hopefully they will have an answer or we'll need to also detect a CDS file in the JDK before starting. 😡 |
Sorry, something went wrong.
|
On my Zulu 21 install the built-in jsa is here: /Library/Java/JavaVirtualMachines/zulu-21.jdk/Contents/Home/lib/server/classes.jsa (.../Contents/Home is $JAVA_HOME) I suppose we could just check if there's any .jsa file in the $JAVA_HOME/lib/server dir? What a pain. |
Sorry, something went wrong.
All we need to do is find "$JAVA_HOME" -iname "*.jsa" | read -r _ but yeah recursing through JAVA_HOME would be a bit of an expensive check. Looks like it completely errors if the base CDS files aren't present, what do the release files look like for those builds? Edit: Missed the fact that they're always in lib/server, it actually wouldn't be that expensive with just a shell glob but a method that doesn't rely on the internal file structure of the JVM would still be nice. |
Sorry, something went wrong.
|
This is what the patch would look like, I can push it if you want: diff --git a/bin/jruby.sh b/bin/jruby.sh
index 8cd0aec5e3..66e080bac0 100755
--- a/bin/jruby.sh
+++ b/bin/jruby.sh
@@ -145,6 +145,18 @@ a_isempty() {
return 0
}
+# exists [FILE...]
+#
+# Returns 0 if all FILEs exist or none provided, otherwise returns 1
+exists() {
+ while [ "$#" -gt 0 ]; do
+ [ -e "$1" ] || return
+ shift
+ done
+
+ return 0
+}
+
# is_newer FILE OTHER...
#
# Returns 0 if FILE is newer than all OTHER files. If FILE doesn't exist,
@@ -500,7 +512,7 @@ add_log "Detected Java version: $java_version"
java_major=${java_version%%.*}
# AppCDS support
-if [ "$java_major" -ge 13 ]; then
+if [ "$java_major" -ge 13 ] && exists "$JAVA_HOME"/lib/server/*.jsa; then
java_has_appcds=true
else
java_has_appcds=false |
Sorry, something went wrong.
|
Let's hold off and do this additional detection separately. I heard back from the Temurin folks and they say builds since January should include the missing files. Maybe this problem will just go away, but if it doesn't we can add the detection in. |
Sorry, something went wrong.
|
My worry is just that JRuby will suddenly break for anyone with these JDKs unless they pass the as-yet undocumented --nocache flag. How does the inclusion of the base JSA file work? I assume it's just a build flag, so anyone who wanted a faster build could exclude that step. |
Sorry, something went wrong.
|
@mrnoname1000 That's a fair point. Let's go with the patch you have and we can refine it after we land this PR. |
Sorry, something went wrong.
|
Also, let's make --cache just exit with error when CDS is not easily available. |
Sorry, something went wrong.
|
An additional discussion or requirement: if we can't write to the JRuby lib dir, we shouldn't error... but should we warn? I'm not sure. |
Sorry, something went wrong.
|
Added the discussed features:
|
Sorry, something went wrong.
|
Actually the JSA file is created without write permissions for some reason so I changed it to just check the parent directory |
Sorry, something went wrong.
Yeah I think this is a holdover from the JDK-level archive that they don't want anyone to overwrite, but it's definitely a little weird. |
Sorry, something went wrong.
|
I've been running with this latest version since you pushed it and everything works great on Java 21 and 23 with JRuby master. I'm going to merge this and we can make additional tweaks there. Excellent work, @mrnoname1000, thank you! I see a few next steps:
I also still like the idea of making this a separate project (module, repository, whatever) that gets installed on build. If it's a gem, it would mean users can upgrade the launcher any time. But either way having it sourced from one place would mean we're always including the latest cool launcher script features. And of course we still need to explore what it might take to build a native executable with this and busybox, or else help @enebo finish his Rust launcher. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
The launcher now detects the Java version and uses the appropriate AppCDS flags. If I researched correctly, AppCDS is supported on Java 10+ and the AutoCreateSharedArchive flag on 19+. The modification time of the JRuby JAR is used to determine if the AppCDS archive should be explicitly regenerated.
The Java version currently does not factor into the regeneration calculation. The easiest solution would be to have an archive for every Java version, which would only involve sticking the relevant version in the filename, but could get unwieldy without a way to clean up old archives. We could also store the last-used version in a sidecar file, which also wouldn't be too bad since we already need access to the JRuby folder. I could use some input on what strategy we would prefer.