Repository navigation
Dist: give the runtime its base CDS archive, drop the AppCDS archive that was never made - #1386
Conversation
…that was never made The distribution tried to ship an AppCDS archive for the language server (generateAppCdsArchive, -XX:ArchiveClassesAtExit). An archive like that sits on the base archive of the runtime, and the slim runtime built by jlink had none (no classes.jsa next to jvm.dll), so the JVM could not write it, the task only warned, and nothing ever passed the archive to a JVM anyway. An installed runtime confirms it: no .jsa anywhere. jlink now runs with --generate-cds-archive, the 14 MB variant for heaps above 32 GB is left out of the copy, and assembleSlimCompilerDist fails if the runtime has no classes.jsa (bin/server or lib/server; macOS copies a full JDK, which has one), because the failure was silent before. The dead task and the -languageServerAppCdsTrain option it used are removed. The archive of the application is written by the JVM itself on the user's machine: wurst4vscode starts the server with -XX:+AutoCreateSharedArchive. An archive recorded at build time cannot be shipped, as the JVM refuses one for another jar path or modification time. Language server start to ready, interleaved, median of four starts: small project no archives 4.39 s, base archive only 4.42 s, both 3.71 s castle fight no archives 18.98 s, base archive only 19.25 s, both 17.28 s The base archive alone changes nothing; it is what makes the second possible.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dfe57916e5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
jlink writes the archive with a JVM of its own, and that JVM takes on the object header mode of the options it is started with. The CI exports JAVA_TOOL_OPTIONS=-XX:+UseCompactObjectHeaders for the tests, so jlink made only classes_coh.jsa and classes_nocoops_coh.jsa, no classes.jsa, and the guard added by this change failed the master build (reproduced with that environment and a clean dist folder: "The runtime has no base CDS archive"). A runtime which is started without the option, as the language server and grill do, cannot use a _coh archive. The jlink step now runs without JAVA_TOOL_OPTIONS, _JAVA_OPTIONS and JDK_JAVA_OPTIONS. The guard is also made on the jlink image itself: the dist folder is only added to by the copy, so an archive left by an earlier build could hide a missing one there. Under that environment, from a clean dist folder: classes.jsa is made and ends up in the runtime, classes_nocoops.jsa is left out.
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
What
The distribution has always tried to ship an AppCDS archive for the language server, and it never worked.
generateAppCdsArchivemade the archive with-XX:ArchiveClassesAtExit. Such an archive sits on the base archive of the runtime, and the slim jlink runtime had none (noclasses.jsanext tojvm.dll), so the JVM could not write it. The task was best effort and only warned..jsaanywhere under~/.wurst.This change:
--generate-cds-archiveand leaves the 14 MBclasses_nocoops.jsaout of the copy (it is for heaps above 32 GB);assembleSlimCompilerDistfail when the runtime has noclasses.jsa(bin/serverorlib/server; macOS copies a full JDK, which has one), because the failure was silent;-languageServerAppCdsTrainoption (RunArgs,Main,LanguageServerStarter,RunArgsTests);AGENTS.md.The archive of the application itself cannot be shipped: the JVM refuses an archive for another jar path or modification time (I saw the path mismatch). It is written by the JVM on the user's machine, which wurst4vscode now does (companion PR: wurstscript/wurst4vscode#153).
Measured
Language server, start to the initial build being ready, interleaved, median of four starts, with the extension's flags (
-XX:+AutoCreateSharedArchive -XX:SharedArchiveFile=… -Xlog:disable):The base archive alone changes nothing; it is what makes the application archive possible. The first session pays about 1.2 s more when it ends (it writes 28 MB). With the real distribution built from this branch: 3.5 s then 2.75 s on the small project.
Two things the JVM does that this relies on being handled by the caller (done in the extension): it does not replace an archive that no longer fits (a new jar), and it exits with a crash status when it cannot write the archive.
Checks
./gradlew assembleSlimCompilerDistbuilds on Windows: runtime hasbin/server/classes.jsa, noclasses_nocoops.jsa, guard passes.RunArgsTests,LanguageWorkerTest.