Re: RFR: JDK-8274930: sun/tools/jps/TestJps.java can fail with long VM arguments string

2021-10-28 Thread Alex Menkov
On Wed, 27 Oct 2021 23:09:09 GMT, Chris Plummer  wrote:

> Actually revoking my review for the moment. Do we have any tests that 
> currently test the default PerfMaxStringConstLength, and this change could be 
> subverting the test by making it so the length is never exceeded?

We don't have any test for PerfMaxStringConstLength.
I think we can fix the issue by using LingeredApp.startAppExactJvmOpts instead 
of LingeredApp.startApp.
I the case test VM options are not added to target app and command line 
contains only specified options.
It should be ok as we test only jps functionality.
I don't have strong opinion which way is better.

-

PR: https://git.openjdk.java.net/jdk/pull/5858


Re: RFR: 8274621: NullPointerException because listenAddress[0] is null

2021-10-28 Thread Chris Plummer
On Tue, 5 Oct 2021 22:34:38 GMT, Alex Menkov  wrote:

> The change fixes ProcessTools.startProcess "warmup predicate" synchronization 
> issue.
> Initially the predicate was called only for STDOUT;
> From jdk8 it's called for STDERR too (but ProcessTools javadoc was not 
> updated).
> The fix keeps existing functionality as is (as we have this behavior for a 
> long time and we have tests which expect STDERR output), but adds 
> synchronization to avoid calling predicate after previous call returned 
> "true".
> Also updated javadoc to reflect actual behavior.

Marked as reviewed by cjplummer (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/5830


Re: kludgeMantis is no longer needed?

2021-10-28 Thread Chris Plummer

  
I think it is safe to assume that
  jvmstat no longer needs to support querying 1.4.2 JVMs. The issue
  was fixed in Java 5.
  
  Chris
  
  On 10/28/21 12:48 AM, Magnus Ihse Bursie wrote:


  
  I noticed the following piece of code in
src/jdk.internal.jvmstat/share/classes/sun/jvmstat/perfdata/monitor/v1_0/PerfDataBuffer.java:
  
  if (jvm_name.stringValue().indexOf("HotSpot") >= 0) {if (jvm_version.stringValue().startsWith("1.4.2")) {kludgeMantis(map, args);}}
  
  It looks like there's some room for cleaning out old cruft here...
  
  /Magnus


  



Re: RFR: 8275735: [linux] Remove deprecated Metrics api (kernel memory limit)

2021-10-28 Thread Mandy Chung
On Thu, 28 Oct 2021 13:03:56 GMT, Severin Gehwolf  wrote:

> Please review this change to remove some API which no longer works as 
> expected as recent OCI runtimes start to drop support for `--kernel-memory` 
> switch. See the bug for references. This part of the API is not present in 
> hotspot code.
> 
> Testing: Container tests (cgroup v1) on Linux x86_64 (all pass)

Marked as reviewed by mchung (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/6156


Re: RFR: 8271356: Modify jdb to treat an empty command as a repeat of the previous command [v6]

2021-10-28 Thread Chris Plummer
On Mon, 25 Oct 2021 19:50:54 GMT, Chris Plummer  wrote:

>> Based on my local testing it appears the localization system falls back to 
>> the English values when entries are missing, so it appears the current 
>> behavior does already match what you've suggested.  However, I don't 
>> understand the mechanism that implements this fallback handling, so I'm not 
>> sure the behavior would be consistent across environments.  At least for now 
>> I'll add those entries back with the English translations copied, since that 
>> seems better.
>
>> Based on my local testing it appears the localization system falls back to 
>> the English values when entries are missing, so it appears the current 
>> behavior does already match what you've suggested. However, I don't 
>> understand the mechanism that implements this fallback handling, so I'm not 
>> sure the behavior would be consistent across environments. At least for now 
>> I'll add those entries back with the English translations copied, since that 
>> seems better.
> 
> Ok. That change looks good, but you need to update the copyright in those two 
> files.

> @plummercj Looks like this is all ready to go then. Would you be willing to 
> sponsor it for integration?

Yes. I'm doing some testing with our CI now. I'll integrate once that is done.

-

PR: https://git.openjdk.java.net/jdk/pull/5290


Re: RFR: 8275735: [linux] Remove deprecated Metrics api (kernel memory limit)

2021-10-28 Thread Severin Gehwolf
On Thu, 28 Oct 2021 14:04:15 GMT, Harold Seigel  wrote:

> The changes look good. Thanks for doing this. Harold

Thanks for the review!

-

PR: https://git.openjdk.java.net/jdk/pull/6156


Re: RFR: 8275735: [linux] Remove deprecated Metrics api (kernel memory limit)

2021-10-28 Thread Harold Seigel
On Thu, 28 Oct 2021 13:03:56 GMT, Severin Gehwolf  wrote:

> Please review this change to remove some API which no longer works as 
> expected as recent OCI runtimes start to drop support for `--kernel-memory` 
> switch. See the bug for references. This part of the API is not present in 
> hotspot code.
> 
> Testing: Container tests (cgroup v1) on Linux x86_64 (all pass)

The changes look good.  Thanks for doing this.
Harold

-

Marked as reviewed by hseigel (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/6156


RFR: 8275735: [linux] Remove deprecated Metrics api (kernel memory limit)

2021-10-28 Thread Severin Gehwolf
Please review this change to remove some API which no longer works as expected 
as recent OCI runtimes start to drop support for `--kernel-memory` switch. See 
the bug for references. This part of the API is not present in hotspot code.

Testing: Container tests (cgroup v1) on Linux x86_64 (all pass)

-

Commit messages:
 - 8275735: [linux] Remove deprecated Metrics api (kernel memory limit)

Changes: https://git.openjdk.java.net/jdk/pull/6156/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6156=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8275735
  Stats: 104 lines in 6 files changed: 0 ins; 100 del; 4 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6156.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6156/head:pull/6156

PR: https://git.openjdk.java.net/jdk/pull/6156


kludgeMantis is no longer needed?

2021-10-28 Thread Magnus Ihse Bursie
I noticed the following piece of code in 
src/jdk.internal.jvmstat/share/classes/sun/jvmstat/perfdata/monitor/v1_0/PerfDataBuffer.java:


if(jvm_name.stringValue().indexOf("HotSpot") >= 0) {
if(jvm_version.stringValue().startsWith("1.4.2")) {
kludgeMantis(map, args);
}
}

It looks like there's some room for cleaning out old cruft here...

/Magnus