RFR: 8263968: CDS: java/lang/ModuleLayer.EMPTY_LAYER should be singleton
With CDS and G1, test api/java_lang/ModuleLayer/Boot.html fails in JDK 16. After JDK-8253081 with CDS enabled two instances of empty layer appear in the runtime. One comes from default initialization of java/lang/ModuleLayer.EMPTY_LAYER, another one comes from CDS archive and is returned by ModuleLayer.boot().parents().get(0). The fix makes java/lang/ModuleLayer.EMPTY_LAYER a singleton with both CDS on and off, similar to java/lang/module/Configuration.EMPTY_CONFIGURATION. Testing: JCK 16, jtreg (including newly added test), pre-submit GitHub actions tests. ------------- Commit messages: - fix jcheck requirements - JDK-8263968: CDS: java/lang/ModuleLayer.EMPTY_LAYER should be singleton Changes: https://git.openjdk.java.net/jdk/pull/3131/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3131&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8263968 Stats: 24 lines in 3 files changed: 21 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/3131.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3131/head:pull/3131 PR: https://git.openjdk.java.net/jdk/pull/3131
On Mon, 22 Mar 2021 19:40:19 GMT, Aleksei Voitylov <avoitylov@openjdk.org> wrote:
With CDS and G1, test api/java_lang/ModuleLayer/Boot.html fails in JDK 16.
After JDK-8253081 with CDS enabled two instances of empty layer appear in the runtime. One comes from default initialization of java/lang/ModuleLayer.EMPTY_LAYER, another one comes from CDS archive and is returned by ModuleLayer.boot().parents().get(0). The fix makes java/lang/ModuleLayer.EMPTY_LAYER a singleton with both CDS on and off, similar to java/lang/module/Configuration.EMPTY_CONFIGURATION.
Testing: JCK 16, jtreg (including newly added test), pre-submit GitHub actions tests.
LGTM. Thanks for fixing this! ------------- Marked as reviewed by iklam (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3131
On Mon, 22 Mar 2021 19:40:19 GMT, Aleksei Voitylov <avoitylov@openjdk.org> wrote:
With CDS and G1, test api/java_lang/ModuleLayer/Boot.html fails in JDK 16.
After JDK-8253081 with CDS enabled two instances of empty layer appear in the runtime. One comes from default initialization of java/lang/ModuleLayer.EMPTY_LAYER, another one comes from CDS archive and is returned by ModuleLayer.boot().parents().get(0). The fix makes java/lang/ModuleLayer.EMPTY_LAYER a singleton with both CDS on and off, similar to java/lang/module/Configuration.EMPTY_CONFIGURATION.
Testing: JCK 16, jtreg (including newly added test), pre-submit GitHub actions tests.
Seems reasonable. Though I hadn't realized CDS intruded into the Java layer that way! ------------- Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3131
On Mon, 22 Mar 2021 19:40:19 GMT, Aleksei Voitylov <avoitylov@openjdk.org> wrote:
With CDS and G1, test api/java_lang/ModuleLayer/Boot.html fails in JDK 16.
After JDK-8253081 with CDS enabled two instances of empty layer appear in the runtime. One comes from default initialization of java/lang/ModuleLayer.EMPTY_LAYER, another one comes from CDS archive and is returned by ModuleLayer.boot().parents().get(0). The fix makes java/lang/ModuleLayer.EMPTY_LAYER a singleton with both CDS on and off, similar to java/lang/module/Configuration.EMPTY_CONFIGURATION.
Testing: JCK 16, jtreg (including newly added test), pre-submit GitHub actions tests.
Marked as reviewed by alanb (Reviewer). src/java.base/share/classes/java/lang/ModuleLayer.java line 156:
154: 155: static { 156: // Initialize EMPTY_LAYER from the archive.
Style wide you can drop L154 and L156 and the comment on EMPTY_LAYER will cover its initialization too. Otherwise looks okay but the more archiving we add does make it harder to change this code. ------------- PR: https://git.openjdk.java.net/jdk/pull/3131
On Tue, 23 Mar 2021 06:47:50 GMT, Alan Bateman <alanb@openjdk.org> wrote:
Aleksei Voitylov has updated the pull request incrementally with one additional commit since the last revision:
fix style
src/java.base/share/classes/java/lang/ModuleLayer.java line 156:
154: 155: static { 156: // Initialize EMPTY_LAYER from the archive.
Style wide you can drop L154 and L156 and the comment on EMPTY_LAYER will cover its initialization too.
Otherwise looks okay but the more archiving we add does make it harder to change this code.
Fixed according to your comments. Thanks! I was surprised to find CDS specific code in the java layer as well, but it's not the design choice I believe I can make with this bug fix. ------------- PR: https://git.openjdk.java.net/jdk/pull/3131
With CDS and G1, test api/java_lang/ModuleLayer/Boot.html fails in JDK 16.
After JDK-8253081 with CDS enabled two instances of empty layer appear in the runtime. One comes from default initialization of java/lang/ModuleLayer.EMPTY_LAYER, another one comes from CDS archive and is returned by ModuleLayer.boot().parents().get(0). The fix makes java/lang/ModuleLayer.EMPTY_LAYER a singleton with both CDS on and off, similar to java/lang/module/Configuration.EMPTY_CONFIGURATION.
Testing: JCK 16, jtreg (including newly added test), pre-submit GitHub actions tests.
Aleksei Voitylov has updated the pull request incrementally with one additional commit since the last revision: fix style ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/3131/files - new: https://git.openjdk.java.net/jdk/pull/3131/files/3a69084a..0cae1a2d Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3131&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3131&range=00-01 Stats: 2 lines in 1 file changed: 0 ins; 2 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/3131.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3131/head:pull/3131 PR: https://git.openjdk.java.net/jdk/pull/3131
On Tue, 23 Mar 2021 08:04:09 GMT, Aleksei Voitylov <avoitylov@openjdk.org> wrote:
With CDS and G1, test api/java_lang/ModuleLayer/Boot.html fails in JDK 16.
After JDK-8253081 with CDS enabled two instances of empty layer appear in the runtime. One comes from default initialization of java/lang/ModuleLayer.EMPTY_LAYER, another one comes from CDS archive and is returned by ModuleLayer.boot().parents().get(0). The fix makes java/lang/ModuleLayer.EMPTY_LAYER a singleton with both CDS on and off, similar to java/lang/module/Configuration.EMPTY_CONFIGURATION.
Testing: JCK 16, jtreg (including newly added test), pre-submit GitHub actions tests.
Aleksei Voitylov has updated the pull request incrementally with one additional commit since the last revision:
fix style
Marked as reviewed by alanb (Reviewer). ------------- PR: https://git.openjdk.java.net/jdk/pull/3131
On Tue, 23 Mar 2021 08:21:27 GMT, Alan Bateman <alanb@openjdk.org> wrote:
Aleksei Voitylov has updated the pull request incrementally with one additional commit since the last revision:
fix style
Marked as reviewed by alanb (Reviewer).
Thanks Ioi, David and Alan for your reviews! ------------- PR: https://git.openjdk.java.net/jdk/pull/3131
On Tue, 23 Mar 2021 08:04:09 GMT, Aleksei Voitylov <avoitylov@openjdk.org> wrote:
With CDS and G1, test api/java_lang/ModuleLayer/Boot.html fails in JDK 16.
After JDK-8253081 with CDS enabled two instances of empty layer appear in the runtime. One comes from default initialization of java/lang/ModuleLayer.EMPTY_LAYER, another one comes from CDS archive and is returned by ModuleLayer.boot().parents().get(0). The fix makes java/lang/ModuleLayer.EMPTY_LAYER a singleton with both CDS on and off, similar to java/lang/module/Configuration.EMPTY_CONFIGURATION.
Testing: JCK 16, jtreg (including newly added test), pre-submit GitHub actions tests.
Aleksei Voitylov has updated the pull request incrementally with one additional commit since the last revision:
fix style
Looks good. The need to retain singleton/identity semantics of constants like this when archiving in CDS has bitten us in the past. ------------- Marked as reviewed by redestad (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3131
On Wed, 24 Mar 2021 16:31:20 GMT, Claes Redestad <redestad@openjdk.org> wrote:
Aleksei Voitylov has updated the pull request incrementally with one additional commit since the last revision:
fix style
Looks good. The need to retain singleton/identity semantics of constants like this when archiving in CDS has bitten us in the past.
Unknown command `backport` - for a list of valid commands use `/help`.
There is the description on the SKARA/Backports page: https://wiki.openjdk.java.net/display/SKARA/Backports Backport Commit Command The /backport commit command can be used to quickly create a backport pull request for a given commit. Just navigate to the comment in source code hosting provider's web UI and add a comment consisting of "/backport <repo>". If the commit does not apply clean on the target repository then a message will be shown for the files with conflicts. The `The /backport commit command ` link points to: https://wiki.openjdk.java.net/display/SKARA/Commit+Commands#CommitCommands-/... Commit Commands Syntax Description Applies the commit onto the given branch in the given repository and then shows to a link to create a pull request with the changes. The branch is optional and defaults to the master branch. If the commit does not apply then an message is shown describing the files containing conflicts. Examples What is the right way to create a backport to jdk16u? ------------- PR: https://git.openjdk.java.net/jdk/pull/3131
On Mon, 22 Mar 2021 19:40:19 GMT, Aleksei Voitylov <avoitylov@openjdk.org> wrote:
With CDS and G1, test api/java_lang/ModuleLayer/Boot.html fails in JDK 16.
After JDK-8253081 with CDS enabled two instances of empty layer appear in the runtime. One comes from default initialization of java/lang/ModuleLayer.EMPTY_LAYER, another one comes from CDS archive and is returned by ModuleLayer.boot().parents().get(0). The fix makes java/lang/ModuleLayer.EMPTY_LAYER a singleton with both CDS on and off, similar to java/lang/module/Configuration.EMPTY_CONFIGURATION.
Testing: JCK 16, jtreg (including newly added test), pre-submit GitHub actions tests.
This pull request has now been integrated. Changeset: 133a63b4 Author: Aleksei Voitylov <avoitylov@openjdk.org> Committer: Claes Redestad <redestad@openjdk.org> URL: https://git.openjdk.java.net/jdk/commit/133a63b4 Stats: 22 lines in 3 files changed: 19 ins; 0 del; 3 mod 8263968: CDS: java/lang/ModuleLayer.EMPTY_LAYER should be singleton Reviewed-by: iklam, dholmes, alanb, redestad ------------- PR: https://git.openjdk.java.net/jdk/pull/3131
participants (6)
-
Alan Bateman
-
Aleksei Voitylov
-
Alexander Scherbatiy
-
Claes Redestad
-
David Holmes
-
Ioi Lam