Re: RFR: 8283279: [Testbug] Improve TestGetSwapSpaceSize

2022-03-22 Thread Jie Fu
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]

2022-03-22 Thread Vladimir Kozlov
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]

2022-03-22 Thread Fei Yang
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]

2022-03-22 Thread Fei Yang
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]

2022-03-22 Thread Fei Yang
> 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]

2022-03-22 Thread Fei Yang
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

2022-03-22 Thread Claes Redestad
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

2022-03-22 Thread Mandy Chung
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

2022-03-22 Thread Kevin Walls
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]

2022-03-22 Thread Vladimir Kozlov
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]

2022-03-22 Thread Vladimir Kozlov
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]

2022-03-22 Thread Roger Riggs
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]

2022-03-22 Thread David Holmes
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]

2022-03-22 Thread David Holmes
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]

2022-03-22 Thread Fei Yang
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]

2022-03-22 Thread Fei Yang
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]

2022-03-22 Thread Fei Yang
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]

2022-03-22 Thread Fei Yang
> 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

2022-03-22 Thread Kevin Walls
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]

2022-03-22 Thread Kevin Walls
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