8351851: Update PmemTest to run on AMD64

[frkator]

Remove default OS platform linux x86_64, remove @ignores allowing amd64 as a viable OS platform selection

---

Progress

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

Issue

• JDK-8351851: Update PmemTest to run on AMD64 (Task - P3)

Reviewers

• Andrew Dinn (@adinn - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/24416/head:pull/24416
$ git checkout pull/24416

Update a local copy of the PR:
$ git checkout pull/24416
$ git pull https://git.openjdk.org/jdk.git pull/24416/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 24416

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/24416.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back isipka! 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]

@frkator This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8351851: Update PmemTest to run on AMD64

Reviewed-by: adinn

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 275 new commits pushed to the master branch:

• 0995b94: 8354815: RISC-V: Change type of bitwise rotation shift to iRegIorL2I
• 38f138b: 8354985: Add unit tests for Executor class from jpackage test lib
• ad7c475: 8353655: Clean up and open source KeyEvent related tests (Part 1)
• ... and 272 more: https://git.openjdk.org/jdk/compare/70e325004536eb9b79f4943f27eed4ac6cec8bd5...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@adinn) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

[openjdk]

@frkator The following label will be automatically applied to this pull request:

• nio

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 02: Full - Incremental (59209dd4)
• 01: Full - Incremental (eae29977)
• 00: Full (f6822b82)

[mahendrachhipa]

Update the copy right year.

[adinn]

I'm not sure if this change is correct. The original directive was ignored and replaced with an x86_64-only directive because of automated testing concerns documented in PR 12352. If this change is happening because those automated test restrictions no longer apply for all three of amd64 (by which I mean not intel x86_64) aarch64 and ppc then reverting to the ignored directive would be fine. If the only change is that automated testing on amd64 now works as well as x86_64 then I would recommend modifying the first directive and keeping the ignored directive as the latter helps to clarify to general users how non-automated testing can be performed on aarch64 and ppc.

[msheppar]

It would be preferable if we retained the current test structure and swap
the x86_64 with amd64, as amd64 is the os.arch on which the test will be run

Change

80 /* @test
81 * @summary Testing NVRAM mapped byte buffer support
82 * @run main/manual PmemTest
83 * @requires (os.family == "linux")
84 * @requires (os.arch == "x86_64")
85 /
86
87 / @test
88 * @summary Testing NVRAM mapped byte buffer support
89 * @run main/manual PmemTest
90 * @requires (os.family == "linux")
91 * @requires ((os.arch == "amd64")|(os.arch == "aarch64")|(os.arch == "ppc64le"))
92 * @ignore The test described here is currently disabled on systems that are not
93 * x64-based and lack an external NVRAM memory device. In order to re-enable the
94 * test, you will need to mount the NVRAM device, which will typically appear as
95 * /dev/pmem0, to the directory /mnt/pmem. Once that is done, you can follow the
96 * instructions above to create a test directory and remove the ignore tag.
97 */

To the preferred change i.e. to swap the x86_64 and amd64 and to use

80 /* @test
81 * @summary Testing NVRAM mapped byte buffer support
82 * @run main/manual PmemTest
83 * @requires (os.family == "linux")
84 * @requires (os.arch == “amd64”)
85 */

91 * @requires ((os.arch == “x86_64”)|(os.arch == "aarch64")|(os.arch == "ppc64le"))
92 * @ignore The test described here is currently disabled on systems that are not

[adinn]

It would be preferable if we retained the current test structure and swap
the x86_64 with amd64, as amd64 is the os.arch on which the test will be run

When I wrote my previous comment I was under the impression that @requires(os.arch == "x86_64") selects Intel x86 64-bit only while @requires(os.arch == "amd64") selects either Intel x86 64-bit or AMD x86 64-bit. However, having looked through the code in files:

test/jdk/jdk/internal/util/ArchTest.java
src/java.base/share/classes/jdk/internal/util/Architecture.java
src/java.base/share/classes/jdk/internal/util/PlatformProps.java
/make/modules/java.base/gensrc/GensrcMisc.gmk

it appears to me that these two @requires clauses select the same thing i.e. either Intel x86 64-bit or AMD x86 64-bit. In which case the current config should mean that this test will run on AMD CPUs. Is that not the case?

[msheppar]

I'm not sure if this change is correct. The original directive was ignored and replaced with an x86_64-only directive because of automated testing concerns documented in PR 12352. If this change is happening because those automated test restrictions no longer apply for all three of amd64 (by which I mean not intel x86_64) aarch64 and ppc then reverting to the ignored directive would be fine. If the only change is that automated testing on amd64 now works as well as x86_64 then I would recommend modifying the first directive and keeping the ignored directive as the latter helps to clarify to general users how non-automated testing can be performed on aarch64 and ppc.

just to clarify that both instances of @test are manual and that current test environments are os.arch == "amd64", which based on the default os.arch == "x86_64" meant the test was not run. As such, the purpose of the ticket is to make the default case "amd64"

[msheppar]

It would be preferable if we retained the current test structure and swap
the x86_64 with amd64, as amd64 is the os.arch on which the test will be run

When I wrote my previous comment I was under the impression that @requires(os.arch == "x86_64") selects Intel x86 64-bit only while @requires(os.arch == "amd64") selects either Intel x86 64-bit or AMD x86 64-bit. However, having looked through the code in files:

test/jdk/jdk/internal/util/ArchTest.java
src/java.base/share/classes/jdk/internal/util/Architecture.java
src/java.base/share/classes/jdk/internal/util/PlatformProps.java
/make/modules/java.base/gensrc/GensrcMisc.gmk

it appears to me that these two @requires clauses select the same thing i.e. either Intel x86 64-bit or AMD x86 64-bit. In which case the current config should mean that this test will run on AMD CPUs. Is that not the case?

Thanks for the details above. Currently the main linux execution platforms are returning "amd64" for the OS arch, and that is not picked up by the current @requires (os.arch == "x86_64")
to run on amd64 necessitates the removal of the @ignores directive
so this is a pragmatic change to facilitate the manual execution of the most prevalent amd64 test environments.

It was observed that there might be a separate issue for jtreg on the @requires and
based on your feedback, and expectations there is a separate jtreg issue to investigate the consistency of @requires on the os.arch attribute.

[adinn]

@msheppar Thanks for clarifying what is at stake here. I am afraid don't know enough about the mechanics of the test infrastructure to explain why it is working the way you describe. However, given what you have observed I agree with your suggestion above that the right solution is to keep the two test declarations but switch "amd64" for "x86_64" and vice versa. That should make the tests work on amd64 CPUs while leaving it clear how anyone wanting to test on an alternative CPU would need to modify the test. I'll be happy to review the PR if it includes that change.

[frkator]

@adinn thank you for pointing out the additional relevant tests, the test/jdk/jdk/internal/util/ArchTest.java one is pure gold!

@msheppar the issue on @requires directive consistency was suspected until the following observation was made on a machine with Intel Core i9 CPU (which is clearly x86_64 ISA) :

 $dpkg --print-architecture
 amd64
 $gcc -dumpmachine
 x86_64-linux-gnu
 $uname -m
 x86_64
 $arch
 x86_64

To the best of my understanding the jtreg @requires directive is based on platform-dependent System.getProperty("os.arch") which is, I am speculating, populated via flags set in build time and by querying system calls during JVM startup.

Furthermore, Oracle JDK releases for Linux list only aarch64 and x64 ISA's.

Based on these facts (the architecture test and the releases) it is reasonable to conclude that x86_64 and amd64 are equivalent - as far as the JDK VM scope is concerned - researching the topic seems to point that there still are some exotic differences but namely regarding virtualization.

Hence, in the general case any test that depends on the architecture should list x86_64 and amd64 together as it is uncertain (platform implementation dependent) what the exact string will be, e.g.:

  @requires ((os.arch == "x86_64")|(os.arch == "amd64"))

If we are aiming specifically for mentioned test enviroment it can be amd64 as we know that is what is returned.

In the case we want to proceed with two test cases - I suggest to introduce human readable ids (jtreg @test id tag) so that we dont see automatically assigned #id0 and #id1.

Please advise.

[msheppar]

changing the @requires in the main test to
@requires ((os.arch == "x86_64")|(os.arch == "amd64"))

seems reasonable

and adding an id to each of the test cases is a good idea

[frkator]

changes:

• removed @ignores tag

and split to two test cases:

• @requires ((os.arch == "x86_64")|(os.arch == "amd64")) with id default_architecture_test_case
• @requires ((os.arch == "aarch64")|(os.arch == "ppc64le")) with id other_architectures_test_case

local test shows that requires directive resolves properly as well as the test id is applied

jtreg -version
jtreg 7.6+1-12
Installed in /home/isipka/apps/jtreg/jtreg-7.6+1/lib/jtreg.jar
Running on platform version 25-ea from /home/isipka/.sdkman/candidates/java/25.ea.9-open.
Built with Java(TM) 2 SDK, Version 11.0.16+11-LTS-199 on February 06, 2025.
Copyright (c) 1999, 2022, Oracle and/or its affiliates. All rights reserved.
Use is subject to license terms.
JT Harness, version 6.0 ea b24 (September 27, 2022)
Java Assembler Tools, version 7.0 ea b09 (June 04, 2020)
TestNG: testng-7.3.0.jar, jcommander-1.82.jar, guice-5.1.0.jar
  JUnit: junit-platform-console-standalone-1.11.0.jar
  
  jtreg -verbose:esa java/nio/MappedByteBuffer/PmemTest.java
  failed to get value for jdk.static
  java.lang.UnsatisfiedLinkError: 'boolean jdk.test.whitebox.WhiteBox.isStatic()'
      at jdk.test.whitebox.WhiteBox.isStatic(Native Method)
      at requires.VMProps.isStatic(VMProps.java:830)
      at requires.VMProps$SafeMap.put(VMProps.java:82)
      at requires.VMProps.call(VMProps.java:145)
      at requires.VMProps.call(VMProps.java:67)
      at com.sun.javatest.regtest.agent.GetJDKProperties.run(GetJDKProperties.java:80)
      at com.sun.javatest.regtest.agent.GetJDKProperties.main(GetJDKProperties.java:54)
  runner starting test: java/nio/MappedByteBuffer/PmemTest.java#default_architecture_test_case
  runner finished test: java/nio/MappedByteBuffer/PmemTest.java#default_architecture_test_case
  Failed. Execution failed: `main' threw exception: java.nio.file.NoSuchFileException: /mnt/pmem/test/pmemtest
  Test results: failed: 1; did not meet platform requirements: 1
  Report written to /home/isipka/workspace/jdk/test/jdk/JTreport/html/report.html
  Results written to /home/isipka/workspace/jdk/test/jdk/JTwork
  Error: Some tests failed or other problems occurred.

[frkator]

 jtreg -ignore:quiet -verbose:esa java/nio/MappedByteBuffer/PmemTest.java
  failed to get value for jdk.static
  java.lang.UnsatisfiedLinkError: 'boolean jdk.test.whitebox.WhiteBox.isStatic()'
      at jdk.test.whitebox.WhiteBox.isStatic(Native Method)
      at requires.VMProps.isStatic(VMProps.java:830)
      at requires.VMProps$SafeMap.put(VMProps.java:82)
      at requires.VMProps.call(VMProps.java:145)
      at requires.VMProps.call(VMProps.java:67)
      at com.sun.javatest.regtest.agent.GetJDKProperties.run(GetJDKProperties.java:80)
      at com.sun.javatest.regtest.agent.GetJDKProperties.main(GetJDKProperties.java:54)
  runner starting test: java/nio/MappedByteBuffer/PmemTest.java#default_architecture_test_case
  runner finished test: java/nio/MappedByteBuffer/PmemTest.java#default_architecture_test_case
  Failed. Execution failed: `main' threw exception: java.nio.file.NoSuchFileException: /mnt/pmem/test/pmemtest
  Test results: failed: 1; did not match keywords: 1
  Report written to /home/isipka/workspace/jdk/test/jdk/JTreport/html/report.html
  Results written to /home/isipka/workspace/jdk/test/jdk/JTwork
  Error: Some tests failed or other problems occurred.

[frkator]

/integrate

[openjdk]

@frkator This pull request has not yet been marked as ready for integration.

[adinn]

Ok,that looks like a good solution

[adinn]

@frkator Try to integrate again and I will sponsor.

[frkator]

/integrate

[openjdk]

@frkator
Your change (at version 59209dd) is now ready to be sponsored by a Committer.

[adinn]

/sponsor

[openjdk]

Going to push as commit 0feecb7.
Since your change was applied there have been 275 commits pushed to the master branch:

• 0995b94: 8354815: RISC-V: Change type of bitwise rotation shift to iRegIorL2I
• 38f138b: 8354985: Add unit tests for Executor class from jpackage test lib
• ad7c475: 8353655: Clean up and open source KeyEvent related tests (Part 1)
• ... and 272 more: https://git.openjdk.org/jdk/compare/70e325004536eb9b79f4943f27eed4ac6cec8bd5...master

Your commit was automatically rebased without conflicts.

[openjdk]

@adinn @frkator Pushed as commit 0feecb7.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.