On Tue, 18 Nov 2025 12:35:25 GMT, Igor Rudenko <[email protected]> wrote:
>> src/java.base/share/classes/jdk/internal/foreign/AbstractMemorySegmentImpl.java
>> line 413:
>>
>>> 411: private void checkBounds(long offset, long length, BoundPolicy
>>> policy) {
>>> 412: if (length > 0) {
>>> 413: Preconditions.checkIndex(offset, this.length - length + 1,
>>> new BoundsCheckHandler(this, policy));
>>
>> This is a very very hot path. Allocating here is gonna have negative effect,
>> especially when this code cannot be inlined (in which case the allocation
>> can't be scalar replaced).
>
> Thank you for pointing this out, I believe we can move on with this solution
> if and only if the escape analysis eliminates instance creation. I'll run
> benchmarks you mentioned in order to check if it's so.
The problem is that a JMH benchmark is typically not conclusive to evaluate
impact of allocation. JMH benchmarks are small, so they typically run quite
hot, and C2 can inline the entire benchmark code. This means escape analysis
will routinely work in this context.
To make an example -- in an early version of the FFM API, we used not to have
any static `MemorySegment::copy` method. The theory was that, instead of
providing methods with explicit offset/length values, we could just address
these use cases with a combination of `asSlice` + `copyFrom`. This seemed to be
backed up by good JMH results.
But later on, some real world testing (with Apache Lucene) revealed that the
cost for creating slices was visible, especially before the application was
fully warmed up. This is kind of what I'm worried about here -- in the happy
case I don't doubt that performance of this PR is (very) competitive (and I
like the approach of your code changes!). But in cases where the code calling
the bound check cannot be JIT-compiled (there might be many reasons for this),
I wonder whether performance will remain competitive.
Unfortunately I don't know a good way to test this -- perhaps try to put a
`DontInline ` in some of the JMH benchmarks and measure in that mode.
@JornVernee what do you think?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28124#discussion_r2538761884
