On Mon, 9 Jun 2025 15:13:49 GMT, Kevin Rushforth <k...@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.
>
> Kevin Rushforth has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Review comments
modules/javafx.graphics/src/main/java/com/sun/marlin/OffHeapArray.java line 47:
> 45: /* members */
> 46: private MemorySegment segment;
> 47: private long length;
just curious: why `long`? are we expecting the length to exceed 2 billion?
modules/javafx.graphics/src/main/java/com/sun/marlin/OffHeapArray.java line 79:
> 77: * @return number of used bytes
> 78: */
> 79: int getUsed() {
wait, length is `long`, but used is `int`?
modules/javafx.graphics/src/main/java/com/sun/marlin/OffHeapArray.java line 97:
> 95: */
> 96: void incrementUsed(int increment) {
> 97: this.used += increment;
if used > 2B, it will overflow, right?
modules/javafx.graphics/src/main/java/com/sun/marlin/OffHeapArray.java line 157:
> 155: return segment.get(INT_LAYOUT, offset);
> 156: }
> 157:
extra newline?
modules/javafx.graphics/src/main/java/com/sun/marlin/Renderer.java line 399:
> 397:
> 398: final long SIZE_INT = 4L;
> 399: long addr = edgePtr;
minor: extra spaces?
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1814#discussion_r2135938366
PR Review Comment: https://git.openjdk.org/jfx/pull/1814#discussion_r2135942265
PR Review Comment: https://git.openjdk.org/jfx/pull/1814#discussion_r2135943404
PR Review Comment: https://git.openjdk.org/jfx/pull/1814#discussion_r2135946319
PR Review Comment: https://git.openjdk.org/jfx/pull/1814#discussion_r2135947245
