Re: RFR: 8265518: C1: Intrinsic support for Preconditions.checkIndex [v11]
The JDK codebase re-created many variants of checkIndex(`grep -I -r 'cehckIndex' jdk/`). A notable variant is java.nio.Buffer.checkIndex, which annotated with @IntrinsicCandidate and it only has a corresponding C1 intrinsic version.
In fact, there is an utility method `jdk.internal.util.Preconditions.checkIndex`(wrapped by java.lang.Objects.checkIndex) that behaves the same as these variants of checkIndex, we can replace these re-created variants of checkIndex by Objects.checkIndex, it would significantly reduce duplicated code and enjoys performance improvement because Preconditions.checkIndex is @IntrinsicCandidate and it has a corresponding intrinsic method in HotSpot.
But, the problem is currently HotSpot only implements the C2 version of Preconditions.checkIndex. To reuse it global-widely in JDK code, I think we can firstly implement its C1 counterpart. There are also a few kinds of stuff we can do later:
1. Replace all variants of checkIndex by Objects.checkIndex in the whole JDK codebase. 2. Remove Buffer.checkIndex and obsolete/deprecate InlineNIOCheckIndex flag
Testing: cds, compiler and jdk
Yi Yang has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains eight commits: - x86_32 fails - build failed - cmp clobbers its left argument on x86_32 - better check1-4 - AssertionError when expected exception was not thrown - remove InlineNIOCheckIndex flag - remove java_nio_Buffer in javaClasses.hpp - consolidate ------------- Changes: https://git.openjdk.java.net/jdk/pull/3615/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3615&range=10 Stats: 338 lines in 11 files changed: 242 ins; 78 del; 18 mod Patch: https://git.openjdk.java.net/jdk/pull/3615.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3615/head:pull/3615 PR: https://git.openjdk.java.net/jdk/pull/3615
On Wed, 2 Jun 2021 02:32:54 GMT, Yi Yang <yyang@openjdk.org> wrote:
The JDK codebase re-created many variants of checkIndex(`grep -I -r 'cehckIndex' jdk/`). A notable variant is java.nio.Buffer.checkIndex, which annotated with @IntrinsicCandidate and it only has a corresponding C1 intrinsic version.
In fact, there is an utility method `jdk.internal.util.Preconditions.checkIndex`(wrapped by java.lang.Objects.checkIndex) that behaves the same as these variants of checkIndex, we can replace these re-created variants of checkIndex by Objects.checkIndex, it would significantly reduce duplicated code and enjoys performance improvement because Preconditions.checkIndex is @IntrinsicCandidate and it has a corresponding intrinsic method in HotSpot.
But, the problem is currently HotSpot only implements the C2 version of Preconditions.checkIndex. To reuse it global-widely in JDK code, I think we can firstly implement its C1 counterpart. There are also a few kinds of stuff we can do later:
1. Replace all variants of checkIndex by Objects.checkIndex in the whole JDK codebase. 2. Remove Buffer.checkIndex and obsolete/deprecate InlineNIOCheckIndex flag
Testing: cds, compiler and jdk
Yi Yang has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains eight commits:
- x86_32 fails - build failed - cmp clobbers its left argument on x86_32 - better check1-4 - AssertionError when expected exception was not thrown - remove InlineNIOCheckIndex flag - remove java_nio_Buffer in javaClasses.hpp - consolidate
`test/jdk/java/util/Objects/CheckIndex.java` and `test/jdk/java/util/Objects/CheckLongIndex.java` started failing. Please take a look. test CheckIndex.testCheckIndex(0, -2147483647, false): failure java.lang.AssertionError: Index 0 is out of bounds of [0, -2147483647), but was reported to be within bounds at org.testng.Assert.fail(Assert.java:94) at CheckIndex.lambda$testCheckIndex$3(CheckIndex.java:103) at CheckIndex.testCheckIndex(CheckIndex.java:117) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:568) at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:85) at org.testng.internal.Invoker.invokeMethod(Invoker.java:639) at org.testng.internal.Invoker.invokeTestMethod(Invoker.java:821) at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1131) at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:124) at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:108) at org.testng.TestRunner.privateRun(TestRunner.java:773) at org.testng.TestRunner.run(TestRunner.java:623) at org.testng.SuiteRunner.runTest(SuiteRunner.java:357) at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:352) at org.testng.SuiteRunner.privateRun(SuiteRunner.java:310) at org.testng.SuiteRunner.run(SuiteRunner.java:259) at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52) at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:86) at org.testng.TestNG.runSuitesSequentially(TestNG.java:1185) at org.testng.TestNG.runSuitesLocally(TestNG.java:1110) at org.testng.TestNG.run(TestNG.java:1018) at com.sun.javatest.regtest.agent.TestNGRunner.main(TestNGRunner.java:94) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:568) at com.sun.javatest.regtest.agent.MainActionHelper$AgentVMRunnable.run(MainActionHelper.java:298) at java.base/java.lang.Thread.run(Thread.java:831) test CheckIndex.testCheckIndex(0, -2147483648, false): failure java.lang.AssertionError: Index 0 is out of bounds of [0, -2147483648), but was reported to be within bounds at org.testng.Assert.fail(Assert.java:94) at CheckIndex.lambda$testCheckIndex$3(CheckIndex.java:103) at CheckIndex.testCheckIndex(CheckIndex.java:120) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:568) at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:85) at org.testng.internal.Invoker.invokeMethod(Invoker.java:639) at org.testng.internal.Invoker.invokeTestMethod(Invoker.java:821) at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1131) at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:124) at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:108) at org.testng.TestRunner.privateRun(TestRunner.java:773) at org.testng.TestRunner.run(TestRunner.java:623) at org.testng.SuiteRunner.runTest(SuiteRunner.java:357) at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:352) at org.testng.SuiteRunner.privateRun(SuiteRunner.java:310) at org.testng.SuiteRunner.run(SuiteRunner.java:259) at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52) at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:86) at org.testng.TestNG.runSuitesSequentially(TestNG.java:1185) at org.testng.TestNG.runSuitesLocally(TestNG.java:1110) at org.testng.TestNG.run(TestNG.java:1018) at com.sun.javatest.regtest.agent.TestNGRunner.main(TestNGRunner.java:94) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:568) at com.sun.javatest.regtest.agent.MainActionHelper$AgentVMRunnable.run(MainActionHelper.java:298) at java.base/java.lang.Thread.run(Thread.java:831) test CheckIndex.testCheckIndex(1, -1, false): failure java.lang.AssertionError: Index 1 is out of bounds of [0, -1), but was reported to be within bounds at org.testng.Assert.fail(Assert.java:94) at CheckIndex.lambda$testCheckIndex$3(CheckIndex.java:103) at CheckIndex.testCheckIndex(CheckIndex.java:130) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:568) at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:85) at org.testng.internal.Invoker.invokeMethod(Invoker.java:639) at org.testng.internal.Invoker.invokeTestMethod(Invoker.java:821) at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1131) at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:124) at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:108) at org.testng.TestRunner.privateRun(TestRunner.java:773) at org.testng.TestRunner.run(TestRunner.java:623) at org.testng.SuiteRunner.runTest(SuiteRunner.java:357) at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:352) at org.testng.SuiteRunner.privateRun(SuiteRunner.java:310) at org.testng.SuiteRunner.run(SuiteRunner.java:259) at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52) at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:86) at org.testng.TestNG.runSuitesSequentially(TestNG.java:1185) at org.testng.TestNG.runSuitesLocally(TestNG.java:1110) at org.testng.TestNG.run(TestNG.java:1018) at com.sun.javatest.regtest.agent.TestNGRunner.main(TestNGRunner.java:94) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:568) at com.sun.javatest.regtest.agent.MainActionHelper$AgentVMRunnable.run(MainActionHelper.java:298) at java.base/java.lang.Thread.run(Thread.java:831) ------------- PR: https://git.openjdk.java.net/jdk/pull/3615
participants (2)
-
Igor Veresov
-
Yi Yang