RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v9]

[Maurizio Cimadamore]

On Wed, 21 Oct 2020 16:23:16 GMT, Paul Sandoz <psandoz at openjdk.org> wrote:

>> Maurizio Cimadamore has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 27 commits:
>> 
>>  - Merge branch 'master' into 8254231_linker
>>  - Don't use JNI when generating native wrappers
>>    
>>  - Merge branch 'master' into 8254231_linker
>>  - Fix incorrect capitalization in one copyright header
>>  - Update copyright years, and add classpath exception to files that were missing it
>>  - Use separate constants for native invoker code size
>>  - Re-add file erroneously deleted (detected as rename)
>>  - Re-add erroneously removed files
>>  - Merge branch 'master' into 8254231_linker
>>    
>>    - Fix tests
>>  - Fix more whitespaces
>>  - ... and 17 more: https://git.openjdk.java.net/jdk/compare/da97ab5c...8c7b75da
>
> src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/AbstractNativeScope.java line 120:
> 
>> 118:                 }
>> 119:             }
>> 120:             throw new AssertionError("Cannot get here!");
> 
> This code is a little confusing, effectively using an exception for control flow, within a retry loop. I recommend performing an explicit bounds check to determine if a new segment of `BLOCK_SIZE` is required from which to slice into. It will also be faster than the exceptional case.

I hear you - that said, note that doing the bound check is not as trivial as it seems; you have to take into account the value of `sp` and add required alignment padding, and then perform a bound check against that - all logic which would need to be duplicated across the normal and the exceptional cases. Which is why the code settled the way it is. I'll see what I can do.

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

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