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 |
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 |
In reply to this post by 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 |
In reply to this post by Severin Gehwolf-3
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 |
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 |
In reply to this post by Chris Plummer-2
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 |
In reply to this post by 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 |
In reply to this post by Severin Gehwolf-3
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 |
In reply to this post by Chris Plummer-2
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 |
In reply to this post by Chris Plummer-2
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 |
In reply to this post by 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 |
Free forum by Nabble | Edit this page |