RFR: 8261607: SA attach is exceeding JNI Local Refs capacity

[Chris Plummer-2]

Delete some localrefs to avoid -Xcheck:jni warnings. See CR for details.

-------------

Commit messages:
 - Fix SA -Xcheck:jni warnings and add some testing for the warnings.

Changes: https://git.openjdk.java.net/jdk/pull/2568/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2568&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8261607
  Stats: 23 lines in 4 files changed: 16 ins; 3 del; 4 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2568.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2568/head:pull/2568

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

[Severin Gehwolf-3]

On Mon, 15 Feb 2021 06:00:29 GMT, Chris Plummer <[hidden email]> wrote:

> Delete some localrefs to avoid -Xcheck:jni warnings. See CR for details.

test/hotspot/jtreg/serviceability/sa/ClhsdbLauncher.java line 149:

> 147:         // Make sure no -Xcheck:jni failures. WARNING: JNI local refs
> 148:         oa.shouldNotMatch("^WARNING: JNI local refs:.*$");
> 149:         oa.shouldNotMatch("^WARNING in native method:.*$");

It's not clear to me why this matcher is being done without any tests using `-Xcheck:jni`. Is this intentional?

-------------

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

[Chris Plummer-2]

> Delete some localrefs to avoid -Xcheck:jni warnings. See CR for details.

Chris Plummer has updated the pull request incrementally with one additional commit since the last revision:

  Fix comment

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2568/files
  - new: https://git.openjdk.java.net/jdk/pull/2568/files/775374e4..21ff3296

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2568&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2568&range=00-01

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2568.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2568/head:pull/2568

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

[Chris Plummer-2]

On Mon, 15 Feb 2021 13:21:39 GMT, Severin Gehwolf <[hidden email]> wrote:

>> Chris Plummer has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   Fix comment
>
> test/hotspot/jtreg/serviceability/sa/ClhsdbLauncher.java line 149:
>
>> 147:         // Make sure no -Xcheck:jni failures. WARNING: JNI local refs
>> 148:         oa.shouldNotMatch("^WARNING: JNI local refs:.*$");
>> 149:         oa.shouldNotMatch("^WARNING in native method:.*$");
>
> It's not clear to me why this matcher is being done without any tests using `-Xcheck:jni`. Is this intentional?

Although no Clhsdb test is explicitly always runs with `-Xcheck:jni`, the option can be added to any test run. For example:

`$ make test  TEST=open/test/hotspot/jtreg/serviceability/sa/ TEST_VM_OPTS="-Xcheck:jni"`

This matching is done to make sure there are no warnings when the test is run in this manner. The matching could be conditional on the test being run with  `-Xcheck:jni`, but I don't really see to point in cluttering up the logic with such a check since the warnings should never appear, whether or not you are using `-Xcheck:jni`.

-------------

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

[Severin Gehwolf-3]

On Mon, 15 Feb 2021 17:47:47 GMT, Chris Plummer <[hidden email]> wrote:

>> test/hotspot/jtreg/serviceability/sa/ClhsdbLauncher.java line 149:
>>
>>> 147:         // Make sure no -Xcheck:jni failures. WARNING: JNI local refs
>>> 148:         oa.shouldNotMatch("^WARNING: JNI local refs:.*$");
>>> 149:         oa.shouldNotMatch("^WARNING in native method:.*$");
>>
>> It's not clear to me why this matcher is being done without any tests using `-Xcheck:jni`. Is this intentional?
>
> Although no Clhsdb test is explicitly always runs with `-Xcheck:jni`, the option can be added to any test run. For example:
>
> `$ make test  TEST=open/test/hotspot/jtreg/serviceability/sa/ TEST_VM_OPTS="-Xcheck:jni"`
>
> This matching is done to make sure there are no warnings when the test is run in this manner. The matching could be conditional on the test being run with  `-Xcheck:jni`, but I don't really see to point in cluttering up the logic with such a check since the warnings should never appear, whether or not you are using `-Xcheck:jni`.

OK, I thought so. Could you expand on the comment saying so, please? Suggestion: `Make sure there are no -Xcheck:jni warnings as -Xcheck:jni might be set via TEST_VM_OPTS on test runs`.

-------------

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

[Severin Gehwolf-3]

On Mon, 15 Feb 2021 17:53:58 GMT, Chris Plummer <[hidden email]> wrote:

>> Delete some localrefs to avoid -Xcheck:jni warnings. See CR for details.
>
> Chris Plummer has updated the pull request incrementally with one additional commit since the last revision:
>
>   Fix comment

Other than the comment suggestion this looks reasonable to me.

-------------

Marked as reviewed by sgehwolf (Reviewer).

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

[Chris Plummer-2]

> Delete some localrefs to avoid -Xcheck:jni warnings. See CR for details.

Chris Plummer has updated the pull request incrementally with one additional commit since the last revision:

  Fix comment

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2568/files
  - new: https://git.openjdk.java.net/jdk/pull/2568/files/21ff3296..e018e781

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2568&range=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2568&range=01-02

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2568.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2568/head:pull/2568

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

[Chris Plummer-2]

On Mon, 15 Feb 2021 18:09:45 GMT, Severin Gehwolf <[hidden email]> wrote:

>> Although no Clhsdb test is explicitly always runs with `-Xcheck:jni`, the option can be added to any test run. For example:
>>
>> `$ make test  TEST=open/test/hotspot/jtreg/serviceability/sa/ TEST_VM_OPTS="-Xcheck:jni"`
>>
>> This matching is done to make sure there are no warnings when the test is run in this manner. The matching could be conditional on the test being run with  `-Xcheck:jni`, but I don't really see to point in cluttering up the logic with such a check since the warnings should never appear, whether or not you are using `-Xcheck:jni`.
>
> OK, I thought so. Could you expand on the comment saying so, please? Suggestion: `Make sure there are no -Xcheck:jni warnings as -Xcheck:jni might be set via TEST_VM_OPTS on test runs`.

Ok, the comment has been updated. Thanks!

-------------

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

[Alex Menkov-3]

On Mon, 15 Feb 2021 20:10:02 GMT, Chris Plummer <[hidden email]> wrote:

>> Delete some localrefs to avoid -Xcheck:jni warnings. See CR for details.
>
> Chris Plummer has updated the pull request incrementally with one additional commit since the last revision:
>
>   Fix comment

Marked as reviewed by amenkov (Reviewer).

-------------

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

[Severin Gehwolf-3]

On Mon, 15 Feb 2021 20:10:02 GMT, Chris Plummer <[hidden email]> wrote:

>> Delete some localrefs to avoid -Xcheck:jni warnings. See CR for details.
>
> Chris Plummer has updated the pull request incrementally with one additional commit since the last revision:
>
>   Fix comment

Marked as reviewed by sgehwolf (Reviewer).

-------------

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

[Chris Plummer-2]

On Mon, 15 Feb 2021 06:00:29 GMT, Chris Plummer <[hidden email]> wrote:

> Delete some localrefs to avoid -Xcheck:jni warnings. See CR for details.

This pull request has now been integrated.

Changeset: 55d7bbce
Author:    Chris Plummer <[hidden email]>
URL:       https://git.openjdk.java.net/jdk/commit/55d7bbce
Stats:     23 lines in 4 files changed: 16 ins; 3 del; 4 mod

8261607: SA attach is exceeding JNI Local Refs capacity

Reviewed-by: sgehwolf, amenkov

-------------

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