On Thu, 29 Apr 2021 02:53:18 GMT, Yi Yang <[email protected]> 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 incrementally with one additional commit
> since the last revision:
>
> remove extra newline
test/hotspot/jtreg/compiler/c1/TestCheckIndexC1Intrinsic.java line 60:
> 58: } catch (IndexOutOfBoundsException e) {
> 59: // got it!
> 60: }
In all places where `Precondition.checkIndex` is expected to throw, an
AssertionError should be generated if it doesn't throw:
try {
Preconditions.checkIndex(1, 1, null);
throw new AssertionError("Expected IndexOutOfBoundsException not
thrown");
} catch (IndexOutOfBoundsException e) {
// got it!
}
-------------
PR: https://git.openjdk.java.net/jdk/pull/3615