On Fri, 30 Apr 2021 02:19:30 GMT, Yi Yang <yy...@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 incrementally with one additional commit 
> since the last revision:
> 
>   better check1-4

Thanks for taking this feedback into account and updating the test!
Note: I only reviewed the test.

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

Marked as reviewed by dfuchs (Reviewer).

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

Reply via email to