Repository navigation
fix(desktop): drop the class data sharing archive, which installed apps never used - #436
Merged
Merged
Conversation
…ps never used The JVM checks every jar an AppCDS archive names at the absolute path it had when the archive was dumped (SharedClassPathEntry::validate in JDK 21's filemap.cpp), with its size and modification time, before its "moved together" prefix match. On a user's machine the CI paths don't exist, so the whole archive was rejected: the installed release loaded 0 classes from it. A moved local image only used it while the build folder still existed. No JDK 21 option avoids the check. Dumping at the default install path wouldn't help either: jpackage's DMG copy gives every jar a new modification time, which JDK 25's relocatable AOTClassLocation check still compares. So the archive cost about 70 MB installed and 15 MB of the DMG for nothing. Removes CdsArchive, its launcher options, the dump hook, recordCdsClassList and cds/app.classlist, and says why in AGENTS.md and testing.md.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…archive # Conflicts: # app/desktop/build.gradle.kts # docs/development/testing.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The AppCDS archive added in #421,
ketch.jsa, never applied to an installed app. This removes it.Why it never worked
FileMapInfo::validate_shared_path_table()callsSharedClassPathEntry::validate()on each jar at the absolute path it had when the archive was dumped, checking its size and modification time. Only the JDK'smodulesfile is looked up at its current location. A missing path ("Required classpath entry does not exist") rejects the whole archive. That happens beforevalidate_app_class_paths()tries its "moved together" prefix match, which only compares names.ArchiveRelocationModeonly sets the memory address the archive is mapped at.Alternatives considered
/Applications/Ketch.app,C:\Program Files\Ketch,/opt/ketch): would still fail. jpackage's DMG copy gives every jar a new modification time (1791430891 in the image, 1791430967 in the DMG). The MSI and deb are built from a copy too, and MSI cabinets store times only to 2 seconds in local time.AOTClassLocationConfig::check_classpathschecks the jars where they are now, but still compares their modification times, so packaged builds hit the same problem.-XX:+AutoCreateSharedArchive: needs a path each user can write to. JDK 21's launcher config only expands$APPDIR,$BINDIRand$ROOTDIR, and the installed app folder is read-only (on macOS, writing into it breaks the code signature).So the archive cost about 70 MB installed and 15 MB of the DMG for no gain.
Changes
CdsArchive, its three-XXlauncher options, the dump step,recordCdsClassListandapp/desktop/cds/app.classlist.JAVA_TOOL_OPTIONS=-Xlog:class+load,cds:file=…and the build folder renamed away.Testing
createDistributable, then the image copied withdittoand the build'sappfolder renamed away. The copy has noketch.jsaand its config passes no CDS flags. It starts without CDS warnings, andcodesign --verify --strictpasses on jpackage's original signature.Conflict with #435
#435 edits the
ketch.jsaparagraph in testing.md, which this PR deletes, and adds an-Xlogline next to the removed launcher options. Whichever merges second should keep #435's-Xlogline and this PR's testing.md text.Possible follow-up
On JDK 25 the launcher config can read environment variables, so
-XX:+AutoCreateSharedArchivewith a per-user cache path would build the archive on the user's machine, where paths and times match. The native messaging host also starts a JVM and would share that archive, and quitting takes longer while it is written.