RFR: 8334137: Marlin: replace sun.misc.Unsafe memory access methods with FFM

[Kevin Rushforth]

On Wed, 21 May 2025 13:24:32 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

> PR to replace the use of sun.misc.Unsafe memory access methods in the Marlin rasterizer with FFM.
> 
> I broke this up into the following commits. The bulk of the work is in the first two:
> 
> 1. Encapsulate all off-heap access in OffHeapArray -- All of the memory allocation and management was already done in the OffHeapArray class, but the users of OffHeapArray, primarily Renderer and RendererNoAA, called Unsafe methods directly. I encapsulated all of the calls to Unsafe in OffHeapArray. The main impact on calling code is that the base address is no longer accessible or needed. Instead, the `(put|get)(Byte|Int)` methods take only an offset. This commit was straight refactoring with no behavioral changes.
> 2. Initial FFM implementation -- I changed the memory management and access methods to use FFM. Each OffHeap array uses a shared Arena to manage the single memory segment allocated at construction time. The resize method creates a new Arena and memory segment, copying the data from the old and then closing it
> 3. Set `used` to 0 in `dispose()` -- While testing and instrumenting the code, I discovered that the Renderer dispose methods resize the edges array back to the default size without clearing the "used" field. The used field will be cleared before the next time it is accessed, but clearing it in dispose allows optimizing resize to not copy any data.
> 4. Remove '--sun-misc-unsafe-memory-access=allow' from test and app execution, since it is no longer needed. This also enables `-Werror` for the `javafx.graphics` module.
> 5. ~~Temporary debug prints that will be removed before making this "rfr"~~
> 
> Additional commits address review comments.

@bourgesl @prrace @nlisker @minborg Can you take an initial look at this? It isn't finished (there are a few open questions), but it is working.

modules/javafx.graphics/src/main/java/com/sun/marlin/OffHeapArray.java line 36:

> 34: // KCR: BEGIN DEBUG
> 35: import java.util.concurrent.atomic.AtomicInteger;
> 36: // KCR: END DEBUG

I'll revert all of the debugging code.

modules/javafx.graphics/src/main/java/com/sun/marlin/OffHeapArray.java line 46:

> 44:     // FFM stuff
> 45:     private static final ValueLayout.OfByte BYTE_LAYOUT = ValueLayout.JAVA_BYTE;
> 46:     private static final ValueLayout.OfInt INT_LAYOUT = ValueLayout.JAVA_INT.withOrder(ByteOrder.BIG_ENDIAN);

@minborg Is this the best layout to use? I chose it thinking that, since we don't need to access this memory from native code, it might perform better to use Java's big endian byte order. Is this correct?

modules/javafx.graphics/src/main/java/com/sun/marlin/OffHeapArray.java line 50:

> 48:     static {
> 49:         // KCR FIXME: get this from FFM
> 50:         SIZE_INT = 4;

I will address this.

modules/javafx.graphics/src/main/java/com/sun/marlin/OffHeapArray.java line 55:

> 53:     /* members */
> 54:     private MemorySegment segment;
> 55: //    private long address;

I'll remove this (it's a leftover).

modules/javafx.graphics/src/main/java/com/sun/marlin/OffHeapArray.java line 78:

> 76:         // note: may throw OOME:
> 77:         // KCR FIXME: Set a MemoryLayout
> 78:         this.segment = arena.allocate(len);

I'll choose a memory layout and also specify 4-byte alignment.

modules/javafx.graphics/src/main/java/com/sun/marlin/OffHeapArray.java line 145:

> 143:         if (this.used > 0) {
> 144:             MemorySegment.copy(segment, 0, newSegment, 0, Math.min(this.used, len));
> 145:         }

@bourgesl I presume that there is never a need to copy more than `used` bytes?

modules/javafx.graphics/src/main/java/com/sun/marlin/Renderer.java line 650:

> 648:         // KCR: Double-check this
> 649:         // Clear used bytes in edges array
> 650:         edges.setUsed(0);

@bourgesl I presume that clearing `used` is correct here?

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

PR Comment: https://git.openjdk.org/jfx/pull/1814#issuecomment-2897972083
PR Review Comment: https://git.openjdk.org/jfx/pull/1814#discussion_r2100387719
PR Review Comment: https://git.openjdk.org/jfx/pull/1814#discussion_r2100395691
PR Review Comment: https://git.openjdk.org/jfx/pull/1814#discussion_r2100385901
PR Review Comment: https://git.openjdk.org/jfx/pull/1814#discussion_r2100386573
PR Review Comment: https://git.openjdk.org/jfx/pull/1814#discussion_r2100397246
PR Review Comment: https://git.openjdk.org/jfx/pull/1814#discussion_r2100399348
PR Review Comment: https://git.openjdk.org/jfx/pull/1814#discussion_r2100402618