Skip to content

Conversation

@amosshi
Copy link
Contributor

@amosshi amosshi commented Dec 18, 2023

Backport of JDK-8123456

  • This is a complex change, because in the latest code base the code has been refactored to test/hotspot/jtreg/runtime/cds/ folder, while in Java 11 they are in different folders

This is an Unclean backport, because of

  • The original commit was changing the files in the folder test/hotspot/jtreg/runtime/cds/, while this folder does not exist in Java 11. Where
    1. file test/hotspot/jtreg/runtime/cds/MaxMetaspaceSize.java match to test/hotspot/jtreg/runtime/SharedArchiveFile/MaxMetaspaceSize.java in Java 11
    2. file test/hotspot/jtreg/runtime/cds/SharedStrings.java match to test/hotspot/jtreg/runtime/SharedArchiveFile/SharedStrings.java in Java 11
    3. file test/hotspot/jtreg/runtime/cds/appcds/MoveJDKTest.java match to test/hotspot/jtreg/runtime/appcds/MoveJDKTest.java in Java 11
    4. file test/hotspot/jtreg/runtime/cds/appcds/cacheObject/ArchivedModuleWithCustomImageTest.java matche to test/hotspot/jtreg/runtime/appcds/cacheObject/ArchivedModuleWithCustomImageTest.java in Java 11
    5. file test/hotspot/jtreg/runtime/cds/appcds/VerifyWithDefaultArchive.java does not exist in Java 11, it was added by JDK-8264337 on Java 17
    6. file test/hotspot/jtreg/runtime/cds/appcds/jcmd/JCmdTestDynamicDump.java does not exist in Java 11, it was added by JDK-8265393 on Java 17
  • So For those four (4) moved files
    1. we applied the change in the original commit
    2. and also all the missing commits in between
  • For those two (2) missing files
    • which were added in Java 17, and does not exist in Java 11
    • we simply add them

Tests

  • Test Succeeded in local laptop of MacOS M1 CPU Machine
  • PR: Issue found, under investigation
  • Test Machine: (to run)

Progress

  • Change must be properly reviewed (1 review required, with at least 1 Reviewer)
  • Change must not contain extraneous whitespace
  • JDK-8272552 needs maintainer approval
  • Commit message must refer to an issue

Issue

  • JDK-8272552: mark hotspot runtime/cds tests which ignore external VM flags (Sub-task - P4)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk11u-dev.git pull/2408/head:pull/2408
$ git checkout pull/2408

Update a local copy of the PR:
$ git checkout pull/2408
$ git pull https://git.openjdk.org/jdk11u-dev.git pull/2408/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 2408

View PR using the GUI difftool:
$ git pr show -t 2408

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk11u-dev/pull/2408.diff

Webrev

Link to Webrev Comment

@bridgekeeper
Copy link

bridgekeeper bot commented Dec 18, 2023

👋 Welcome back ashi! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@openjdk openjdk bot changed the title Backport db24eb1e6a0b777dc44f44ae3a1d75ad8d23d6d0 8233000: Mark vmTestbase/vm/mlvm/meth/stress/compiler/deoptimize test as stress test Dec 18, 2023
@openjdk
Copy link

openjdk bot commented Dec 18, 2023

This backport pull request has now been updated with issue from the original commit.

@openjdk openjdk bot added backport Port of a pull request already in a different code base rfr Pull request is ready for review labels Dec 18, 2023
@mlbridge
Copy link

mlbridge bot commented Dec 18, 2023

Webrevs

@amosshi amosshi changed the title 8233000: Mark vmTestbase/vm/mlvm/meth/stress/compiler/deoptimize test as stress test Backport 989f39f8106a22498053a4ca5f2becf8df5f2420 Dec 19, 2023
@openjdk openjdk bot changed the title Backport 989f39f8106a22498053a4ca5f2becf8df5f2420 8272552: mark hotspot runtime/cds tests which ignore external VM flags Dec 19, 2023
@openjdk
Copy link

openjdk bot commented Dec 19, 2023

This backport pull request has now been updated with issue from the original commit.

* @modules java.base/jdk.internal.misc
* java.management
* @run driver MaxMetaspaceSize
*/
Copy link
Contributor Author

@amosshi amosshi Dec 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Includes the changes from JDK-8243945 ( commit 46fe7e3 ) on this file

processArgs.add("-XX:MaxMetaspaceSize=3m");
processArgs.add("-XX:CompressedClassSpaceSize=1m");
processArgs.add("-XX:InitialBootClassLoaderMetaspaceSize=1m");
} else {
Copy link
Contributor Author

@amosshi amosshi Dec 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Includes the changes from JDK-8251158 (commit 7ba6a6b ) on this file


String msg = "Failed allocating metaspace object";
String msg = "OutOfMemoryError: Metaspace";
ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(processArgs);
Copy link
Contributor Author

@amosshi amosshi Dec 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Includes the changes from JDK-8261551 (commit 41657b1 ) on this file

* @build SharedStringsWb sun.hotspot.WhiteBox
* @run driver ClassFileInstaller -jar whitebox.jar sun.hotspot.WhiteBox
* @run driver jdk.test.lib.helpers.ClassFileInstaller -jar whitebox.jar sun.hotspot.WhiteBox
* @run driver SharedStrings
Copy link
Contributor Author

@amosshi amosshi Dec 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Includes the changes from JDK-8263549 ( commit a7aba2b ) on this file

import jdk.test.lib.process.ProcessTools;
import jdk.test.lib.process.OutputAnalyzer;
import jdk.test.lib.helpers.ClassFileInstaller;

Copy link
Contributor Author

@amosshi amosshi Dec 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Includes the changes from JDK-8263412 ( commit e834f99 ) on this file

* @library /test/lib
* @modules java.base/jdk.internal.misc
* java.management
* @build SharedStringsWb sun.hotspot.WhiteBox
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Includes the changes from JDK-8229267 ( commit e9b271d ) on this file

Paths.get(classDir, newFile),
Files.copy(Paths.get(outDir, "hello.jar"),
Paths.get(outDir, newFile),
StandardCopyOption.REPLACE_EXISTING);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Includes the changes from JDK-8251213 ( commit c1093dc ) on this file

import java.nio.file.Paths;
import java.nio.file.StandardCopyOption;
import jdk.test.lib.cds.CDSTestUtils;
import jdk.test.lib.process.OutputAnalyzer;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Includes the changes from JDK-8251213 ( commit c1093dc ) on this file

String java_home_src = System.getProperty("java.home");
String java_home_dst = System.getProperty("user.dir") + File.separator + "moved_jdk";
String java_home_dst = CDSTestUtils.getOutputDir() + File.separator + "moved_jdk";

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Includes the changes from JDK-8251267 ( commit 291ba97 ) on this file

"-version");
OutputAnalyzer out = TestCommon.executeAndLog(pb, "exec-dst");
out.shouldHaveExitValue(0);
out.shouldNotContain("shared class paths mismatch");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Includes the changes from JDK-8272335 ( commit 75a0642 ) on this file

}
}

static ProcessBuilder makeBuilder(String... args) throws Exception {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Includes the changes from JDK-8205946 ( commit a10d6e7 ) on this file

* @modules java.base/jdk.internal.misc
* java.management
* jdk.jartool/sun.tools.jar
* @compile test-classes/Hello.java
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Includes the changes from JDK-8229267 ( commit e9b271d ) on this file

* @modules java.base/jdk.internal.module
* java.management
* jdk.jlink
* jdk.compiler
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Includes the changes from JDK-8229267 ( commit e9b271d ) on this file

* jdk.jlink
* jdk.compiler
* @requires vm.flagless
* @library /test/jdk/lib/testlibrary /test/lib /test/hotspot/jtreg/runtime/cds/appcds
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Includes the changes from JDK-8202339 ( commit 1ed3649 ) on this file

* @run driver ClassFileInstaller -jar app.jar CheckArchivedModuleApp
* @run driver ClassFileInstaller -jar WhiteBox.jar sun.hotspot.WhiteBox
* @run driver jdk.test.lib.helpers.ClassFileInstaller -jar app.jar CheckArchivedModuleApp
* @run driver jdk.test.lib.helpers.ClassFileInstaller -jar WhiteBox.jar sun.hotspot.WhiteBox
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Includes the changes from JDK-8263549 ( commit a7aba2b ) on this file

import jdk.test.lib.compiler.CompilerUtils;
import jdk.test.lib.process.OutputAnalyzer;
import jdk.test.lib.process.ProcessTools;
import jdk.test.lib.helpers.ClassFileInstaller;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Includes the changes from JDK-8263412 ( commit e834f99 ) on this file

customJava.toString(),
"-XX:SharedArchiveFile=./ArchivedModuleWithCustomImageTest.jsa",
"-Xshare:dump"};
"-Xshare:dump", "-Xlog:cds"};
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Includes the changes from JDK-8233826 ( commit d1ad0ea ) on this file

Copy link
Member

@phohensee phohensee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why complicate a trivial backport by bringing in changes from JDK-8243945 and friends? You could instead just backport them. If a full backport of a particular JBS issue is infeasible due to prerequisites that are unlikely to be approved, you could file a JBS issue against 11u to backport just the test changes in the original commits.

@amosshi
Copy link
Contributor Author

amosshi commented Dec 30, 2023

There are 18 dependencies between this PR and the current jdk11u-dev base. So we give up this "dirty" PR for now, and looking for a better solution

dep

@amosshi amosshi closed this Dec 30, 2023
@amosshi amosshi deleted the backport-8272552 branch December 30, 2023 07:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport Port of a pull request already in a different code base rfr Pull request is ready for review

Development

Successfully merging this pull request may close these issues.

2 participants