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=6294=03
- incr: https://webrevs.openjdk.java.net/?repo=jdk=6294=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=7912=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=6294=02
- incr: https://webrevs.openjdk.java.net/?repo=jdk=6294=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
