On Mon, 20 Jan 2025 17:27:40 GMT, Jorn Vernee <jver...@openjdk.org> wrote:
>> Matthias Ernst has updated the pull request incrementally with three >> additional commits since the last revision: >> >> - shift api boundary >> - move bench >> - revert formatting > > src/java.base/share/classes/jdk/internal/foreign/abi/CallBufferCache.java > line 41: > >> 39: // Storing addresses, not MemorySegments turns out to be >> slightly faster (write barrier?). >> 40: private long address1; >> 41: private long address2; > > Can we store just `MemorySegment`s here? It would avoid having to call > `ofAddress` in `acquireOrAllocate`, and would slightly reduce the footprint > when using compressed oops. That was my original version, but this proved to be faster (albeit very little, O(.5ns)). I can't really explain why, that's above my paygrade, but one thing that comes to mind when storing references is that there's might be a GC barrier involved? Footprint reduction would be 8 bytes * #carrier threads, and with scalar replacement, ofAddress should become a noop, right? Let me re-run the BM once more with pointers, whether I can reproduce. > src/java.base/share/classes/jdk/internal/foreign/abi/SharedUtils.java line > 393: > >> 391: MemorySegment unscoped = >> CallBufferCache.acquireOrAllocate(size); >> 392: Arena scope = Arena.ofConfined(); >> 393: MemorySegment source = unscoped.reinterpret(scope, null); > > I suggest passing the `scope` to `acquireOrAllocate` as well, so we only need > a single call to `reinterpret` (it has an overload that takes both a size and > arena. (And I think you can then also remove the `@SuppressWarnings` from > this method) I generally agree that would be the right thing to do. But again, this would require creating the scope before the CTL interaction and that breaks EA. Just swapping lines 391 and 392 leads to plenty of allocations. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/23142#discussion_r1922738090 PR Review Comment: https://git.openjdk.org/jdk/pull/23142#discussion_r1922734006
