On Mon, 11 Aug 2025 16:31:17 GMT, Per Minborg <pminb...@openjdk.org> wrote:
>> This PR proposes to use overlapping memory areas in >> `SegmentBulkOperations::copy`, similar to what is proposed for >> `SegmentBulkOperations::fill` in https://github.com/openjdk/jdk/pull/25383. >> >> This PR passes `tier1`, `tier2`, and `tier3`testing on multiple platforms. > > Per Minborg has updated the pull request incrementally with one additional > commit since the last revision: > > Fix comment src/java.base/share/classes/jdk/internal/foreign/AbstractMemorySegmentImpl.java line 274: > 272: // A long value that is less than zero has it's 63:th bit > set and so, > 273: // we can just AND the expressions (and the sign bit 63) > 274: return (int) (((thisStart - thatEnd) & (thatStart - > thisEnd))); // overlap occurs -> negative value Why not move the branch to here? Is there really a good reason why you move it to the call-site? That way, you could save the comment: `// Returns a negative value if the regions overlap, otherwise a non-negative value.` Also: I see that your conditions at the call-site are not consistent with the current semantics. You have: `if (overlaps(that) >= 0) {` and `src.overlaps(dst) == 0` The second seems wrong to me - do you agree? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/26672#discussion_r2270199905
