On Wed, 21 Oct 2020 16:23:16 GMT, Paul Sandoz <psan...@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
