Re: RFR: 8283279: [Testbug] Improve TestGetSwapSpaceSize
On Thu, 17 Mar 2022 13:40:53 GMT, Severin Gehwolf wrote: > Please review this container test improvement. The test in question only > makes sense iff the total swap space size as reported by the container aware > OperatingSystemMXBean is `0`. If that's not the case, then something else > might be amiss, e.g. OperatingSystemMXBean reporting the host swap limits. In > such a case a passing test tells us nothing. Certainly not if the > fix from [JDK-8242480](https://bugs.openjdk.java.net/browse/JDK-8242480) is > present or not. > > Testing: > - [x] Manual with and without the code fix of JDK-8242480. Still passes with > the fix, and fails without. Tested the test on cgroups v1 and cgroups v2. Please also update the copyright year. Thanks. test/jdk/jdk/internal/platform/docker/GetFreeSwapSpaceSize.java line 28: > 26: > 27: // Usage: > 28: // GetFreeSwapSpaceSize > I would suggest // GetFreeSwapSpaceSize test/jdk/jdk/internal/platform/docker/GetFreeSwapSpaceSize.java line 32: > 30: public static void main(String[] args) { > 31: if (args.length != 4) { > 32: throw new RuntimeException("Unexpected arguments. Expected 2, > got " + args.length); Shouldn't be `Expected 4` ? - PR: https://git.openjdk.java.net/jdk/pull/7854
Re: RFR: 8276799: Implementation of JEP 422: Linux/RISC-V Port [v4]
On Wed, 23 Mar 2022 02:03:26 GMT, Fei Yang wrote: >> This PR implements JEP 422: Linux/RISC-V Port [1]. >> The PR starts as a squashed merge of the >> https://openjdk.java.net/projects/riscv-port branch. >> >> This has been tested with jtreg tier{1,2,3,4} and jcstress on HiFive >> Unmatched board. Dacapo, SPECjbb2015 and SPECjvm2008 benchmark tests are >> also carried out regularly. So it should be good enough to run most Java >> programs. >> >> [1] https://openjdk.java.net/jeps/422 > > Fei Yang has updated the pull request incrementally with one additional > commit since the last revision: > > Fix copyright header Update looks good. Testing results are also good. - Marked as reviewed by kvn (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6294
Re: RFR: 8276799: Implementation of JEP 422: Linux/RISC-V Port [v3]
On Wed, 23 Mar 2022 01:57:25 GMT, Fei Yang wrote: >> src/hotspot/cpu/riscv/disassembler_riscv.hpp line 18: >> >>> 16: * >>> 17: * You should have received a copy of the GNU General Public License >>> version >>> 18: * 2 along with this work; if not, write to the Free Software >>> Foundation, * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. >> >> These 2 lines merged into 1 accidentally causing failure in copyright >> headers verification. > >> I looked on C1/C2 changes and compiler tests. Seems reasonable. But before >> approval I would need to run changes through our testing. > > That's great to hear :-) Thanks for the efforts. I have fixed the copyright headers verification problem. Please take another look. - PR: https://git.openjdk.java.net/jdk/pull/6294
Re: RFR: 8276799: Implementation of JEP 422: Linux/RISC-V Port [v3]
On Tue, 22 Mar 2022 17:34:18 GMT, Vladimir Kozlov wrote: >> Fei Yang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Address review comments > > src/hotspot/cpu/riscv/disassembler_riscv.hpp line 18: > >> 16: * >> 17: * You should have received a copy of the GNU General Public License >> version >> 18: * 2 along with this work; if not, write to the Free Software >> Foundation, * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. > > These 2 lines merged into 1 accidentally causing failure in copyright > headers verification. > I looked on C1/C2 changes and compiler tests. Seems reasonable. But before > approval I would need to run changes through our testing. That's great to hear :-) Thanks for the efforts. - PR: https://git.openjdk.java.net/jdk/pull/6294
Re: RFR: 8276799: Implementation of JEP 422: Linux/RISC-V Port [v4]
> This PR implements JEP 422: Linux/RISC-V Port [1]. > The PR starts as a squashed merge of the > https://openjdk.java.net/projects/riscv-port branch. > > This has been tested with jtreg tier{1,2,3,4} and jcstress on HiFive > Unmatched board. Dacapo, SPECjbb2015 and SPECjvm2008 benchmark tests are also > carried out regularly. So it should be good enough to run most Java programs. > > [1] https://openjdk.java.net/jeps/422 Fei Yang has updated the pull request incrementally with one additional commit since the last revision: Fix copyright header - Changes: - all: https://git.openjdk.java.net/jdk/pull/6294/files - new: https://git.openjdk.java.net/jdk/pull/6294/files/b7a31729..d8bef7fa Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6294&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6294&range=02-03 Stats: 28 lines in 13 files changed: 14 ins; 0 del; 14 mod Patch: https://git.openjdk.java.net/jdk/pull/6294.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6294/head:pull/6294 PR: https://git.openjdk.java.net/jdk/pull/6294
Re: RFR: 8276799: Implementation of JEP 422: Linux/RISC-V Port [v3]
On Tue, 22 Mar 2022 14:01:28 GMT, Roger Riggs wrote: > The test/jdk files look ok. (I didn't look at the rest) Thank you for looking at that part. - PR: https://git.openjdk.java.net/jdk/pull/6294
Re: RFR: 8283254: Remove redundant class jdk/internal/agent/spi/AgentProvider
On Tue, 22 Mar 2022 21:19:20 GMT, Kevin Walls wrote: > There are no uses of jdk/internal/agent/spi/AgentProvider, since the SNMP > agent was removed ( 8071367 ): this class should be removed. It is not a > public interface. > > Remove > src/jdk.management.agent/share/classes/jdk/internal/agent/spi/AgentProvider.java > Remove import from > src/jdk.management.agent/share/classes/jdk/internal/agent/Agent.java > Remove uses from src/jdk.management.agent/share/classes/module-info.java > > Testing with the test/jdk/sun/management tests looks good. Marked as reviewed by redestad (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7912
Re: RFR: 8283254: Remove redundant class jdk/internal/agent/spi/AgentProvider
On Tue, 22 Mar 2022 21:19:20 GMT, Kevin Walls wrote: > There are no uses of jdk/internal/agent/spi/AgentProvider, since the SNMP > agent was removed ( 8071367 ): this class should be removed. It is not a > public interface. > > Remove > src/jdk.management.agent/share/classes/jdk/internal/agent/spi/AgentProvider.java > Remove import from > src/jdk.management.agent/share/classes/jdk/internal/agent/Agent.java > Remove uses from src/jdk.management.agent/share/classes/module-info.java > > Testing with the test/jdk/sun/management tests looks good. Marked as reviewed by mchung (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7912
RFR: 8283254: Remove redundant class jdk/internal/agent/spi/AgentProvider
There are no uses of jdk/internal/agent/spi/AgentProvider, since the SNMP agent was removed ( 8071367 ): this class should be removed. It is not a public interface. Remove src/jdk.management.agent/share/classes/jdk/internal/agent/spi/AgentProvider.java Remove import from src/jdk.management.agent/share/classes/jdk/internal/agent/Agent.java Remove uses from src/jdk.management.agent/share/classes/module-info.java Testing with the test/jdk/sun/management tests looks good. - Commit messages: - 8283254: Remove redundant class jdk/internal/agent/spi/AgentProvider Changes: https://git.openjdk.java.net/jdk/pull/7912/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=7912&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8283254 Stats: 80 lines in 3 files changed: 0 ins; 79 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/7912.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7912/head:pull/7912 PR: https://git.openjdk.java.net/jdk/pull/7912
Re: RFR: 8276799: Implementation of JEP 422: Linux/RISC-V Port [v3]
On Tue, 22 Mar 2022 11:50:13 GMT, Fei Yang wrote: >> This PR implements JEP 422: Linux/RISC-V Port [1]. >> The PR starts as a squashed merge of the >> https://openjdk.java.net/projects/riscv-port branch. >> >> This has been tested with jtreg tier{1,2,3,4} and jcstress on HiFive >> Unmatched board. Dacapo, SPECjbb2015 and SPECjvm2008 benchmark tests are >> also carried out regularly. So it should be good enough to run most Java >> programs. >> >> [1] https://openjdk.java.net/jeps/422 > > Fei Yang has updated the pull request incrementally with one additional > commit since the last revision: > > Address review comments src/hotspot/cpu/riscv/disassembler_riscv.hpp line 18: > 16: * > 17: * You should have received a copy of the GNU General Public License > version > 18: * 2 along with this work; if not, write to the Free Software Foundation, > * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. These 2 lines merged into 1 accidentally causing failure in copyright headers verification. - PR: https://git.openjdk.java.net/jdk/pull/6294
Re: RFR: 8276799: Implementation of JEP 422: Linux/RISC-V Port [v3]
On Tue, 22 Mar 2022 11:50:13 GMT, Fei Yang wrote: >> This PR implements JEP 422: Linux/RISC-V Port [1]. >> The PR starts as a squashed merge of the >> https://openjdk.java.net/projects/riscv-port branch. >> >> This has been tested with jtreg tier{1,2,3,4} and jcstress on HiFive >> Unmatched board. Dacapo, SPECjbb2015 and SPECjvm2008 benchmark tests are >> also carried out regularly. So it should be good enough to run most Java >> programs. >> >> [1] https://openjdk.java.net/jeps/422 > > Fei Yang has updated the pull request incrementally with one additional > commit since the last revision: > > Address review comments I looked on C1/C2 changes and compiler tests. Seems reasonable. But before approval I would need to run changes through our testing. - PR: https://git.openjdk.java.net/jdk/pull/6294
Re: RFR: 8276799: Implementation of JEP 422: Linux/RISC-V Port [v3]
On Tue, 22 Mar 2022 11:50:13 GMT, Fei Yang wrote: >> This PR implements JEP 422: Linux/RISC-V Port [1]. >> The PR starts as a squashed merge of the >> https://openjdk.java.net/projects/riscv-port branch. >> >> This has been tested with jtreg tier{1,2,3,4} and jcstress on HiFive >> Unmatched board. Dacapo, SPECjbb2015 and SPECjvm2008 benchmark tests are >> also carried out regularly. So it should be good enough to run most Java >> programs. >> >> [1] https://openjdk.java.net/jeps/422 > > Fei Yang has updated the pull request incrementally with one additional > commit since the last revision: > > Address review comments The test/jdk files look ok. (I didn't look at the rest) - Marked as reviewed by rriggs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6294
Re: RFR: 8276799: Implementation of JEP 422: Linux/RISC-V Port [v2]
On Tue, 22 Mar 2022 12:08:01 GMT, Fei Yang wrote: >> make/autoconf/libraries.m4 line 152: >> >>> 150: fi >>> 151: >>> 152: # Programs which use C11 or C++11 atomics, like #include , >> >> Use of C++ atomics is not allowed in hotspot code base. See the style guide: >> https://github.com/openjdk/jdk/blob/master/doc/hotspot-style.md >> >> That said, I don't see any actual use of C++ atomics. ?? > > I think the old code comment here is a bit too general. It does not mean we > introduce any use of C++ atomics here. > The fact is that RISC-V only has word-sized atomics, it requries libatomic > where other common architectures do not [1]. > So atomic support would require explicit linking against -latomic on RISC-V. > Otherwise we got build errors like: New comment looks good - thanks for clarifying. - PR: https://git.openjdk.java.net/jdk/pull/6294
Re: RFR: 8276799: Implementation of JEP 422: Linux/RISC-V Port [v3]
On Tue, 22 Mar 2022 11:50:13 GMT, Fei Yang wrote: >> This PR implements JEP 422: Linux/RISC-V Port [1]. >> The PR starts as a squashed merge of the >> https://openjdk.java.net/projects/riscv-port branch. >> >> This has been tested with jtreg tier{1,2,3,4} and jcstress on HiFive >> Unmatched board. Dacapo, SPECjbb2015 and SPECjvm2008 benchmark tests are >> also carried out regularly. So it should be good enough to run most Java >> programs. >> >> [1] https://openjdk.java.net/jeps/422 > > Fei Yang has updated the pull request incrementally with one additional > commit since the last revision: > > Address review comments Marked as reviewed by dholmes (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6294
Re: RFR: 8276799: Implementation of JEP 422: Linux/RISC-V Port [v2]
On Tue, 22 Mar 2022 04:13:17 GMT, David Holmes wrote: >> Fei Yang has updated the pull request with a new target base due to a merge >> or a rebase. The incremental webrev excludes the unrelated changes brought >> in by the merge/rebase. The pull request contains two additional commits >> since the last revision: >> >> - Merge remote-tracking branch 'upstream/master' into JDK-8276799 >> - 8276799: Implementation of JEP 422: Linux/RISC-V Port > > make/autoconf/libraries.m4 line 152: > >> 150: fi >> 151: >> 152: # Programs which use C11 or C++11 atomics, like #include , > > Use of C++ atomics is not allowed in hotspot code base. See the style guide: > https://github.com/openjdk/jdk/blob/master/doc/hotspot-style.md > > That said, I don't see any actual use of C++ atomics. ?? I think the old code comment here is a bit too general. It does not mean we introduce any use of C++ atomics here. The fact is that RISC-V only has word-sized atomics, it requries libatomic where other common architectures do not [1]. So atomic support would require explicit linking against -latomic on RISC-V. Otherwise we got build errors like: - PR: https://git.openjdk.java.net/jdk/pull/6294
Re: RFR: 8276799: Implementation of JEP 422: Linux/RISC-V Port [v2]
On Tue, 22 Mar 2022 05:12:46 GMT, David Holmes wrote: > Hi, > > I've looked at everything that is not a RISC-V specific file, except for the > C1 changes as the compiler folk will need to approve those. > > Some copyrights will need updating to 2022 on the Oracle copyright line > please. Hi David, I have pushed one more commit updating the Oralce copyright line for existing files touched. Thanks for looking at this. - PR: https://git.openjdk.java.net/jdk/pull/6294
Re: RFR: 8276799: Implementation of JEP 422: Linux/RISC-V Port [v2]
On Tue, 22 Mar 2022 03:31:16 GMT, Fei Yang wrote: >> This PR implements JEP 422: Linux/RISC-V Port [1]. >> The PR starts as a squashed merge of the >> https://openjdk.java.net/projects/riscv-port branch. >> >> This has been tested with jtreg tier{1,2,3,4} and jcstress on HiFive >> Unmatched board. Dacapo, SPECjbb2015 and SPECjvm2008 benchmark tests are >> also carried out regularly. So it should be good enough to run most Java >> programs. >> >> [1] https://openjdk.java.net/jeps/422 > > Fei Yang has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains two additional commits since > the last revision: > > - Merge remote-tracking branch 'upstream/master' into JDK-8276799 > - 8276799: Implementation of JEP 422: Linux/RISC-V Port > Build changes look good. I can't say anything about the rest of the code. > > /reviewers 3 Thanks again for looking at the build changes :-) - PR: https://git.openjdk.java.net/jdk/pull/6294
Re: RFR: 8276799: Implementation of JEP 422: Linux/RISC-V Port [v3]
> This PR implements JEP 422: Linux/RISC-V Port [1]. > The PR starts as a squashed merge of the > https://openjdk.java.net/projects/riscv-port branch. > > This has been tested with jtreg tier{1,2,3,4} and jcstress on HiFive > Unmatched board. Dacapo, SPECjbb2015 and SPECjvm2008 benchmark tests are also > carried out regularly. So it should be good enough to run most Java programs. > > [1] https://openjdk.java.net/jeps/422 Fei Yang has updated the pull request incrementally with one additional commit since the last revision: Address review comments - Changes: - all: https://git.openjdk.java.net/jdk/pull/6294/files - new: https://git.openjdk.java.net/jdk/pull/6294/files/a144cd20..b7a31729 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6294&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6294&range=01-02 Stats: 44 lines in 41 files changed: 0 ins; 1 del; 43 mod Patch: https://git.openjdk.java.net/jdk/pull/6294.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6294/head:pull/6294 PR: https://git.openjdk.java.net/jdk/pull/6294
Integrated: 8283092: JMX subclass permission check redundant with strong encapsulation
On Tue, 15 Mar 2022 20:22:16 GMT, Kevin Walls wrote: > Removing permission checks which, in the presence of a Security Manager, > would check for a RuntimePermission "className.subclass". This was to > prevent subclassing these classes, but is no longer necessary with strong > encapsulation from modules. This pull request has now been integrated. Changeset: 37fc77ef Author:Kevin Walls URL: https://git.openjdk.java.net/jdk/commit/37fc77ef60dd97c4acc468ecfeb6753132974720 Stats: 108 lines in 4 files changed: 28 ins; 45 del; 35 mod 8283092: JMX subclass permission check redundant with strong encapsulation Reviewed-by: dfuchs, mchung - PR: https://git.openjdk.java.net/jdk/pull/7827
Re: RFR: 8283092: JMX subclass permission check redundant with strong encapsulation [v3]
On Mon, 21 Mar 2022 20:19:03 GMT, Kevin Walls wrote: >> Removing permission checks which, in the presence of a Security Manager, >> would check for a RuntimePermission "className.subclass". This was to >> prevent subclassing these classes, but is no longer necessary with strong >> encapsulation from modules. > > Kevin Walls has updated the pull request incrementally with one additional > commit since the last revision: > > remove redundant constructors Thanks all for the comments and reviews! - PR: https://git.openjdk.java.net/jdk/pull/7827