On Mon, 2 Jun 2025 20:11:20 GMT, Laurent Bourgès <lbour...@openjdk.org> wrote:
>> 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?
>
> It may depend on hardward arch ???
Based on some additional checking I did, there is no good reason to set the
byte order explicitly, so I'll just use `ValueLayout.JAVA_INT`.
>> 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.
>
> Yes 4-bytes min or more like 16 if it helps the compiler to use avx
> instructions for fill / copy ?
I'll set it to 16 then.
>> 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?
>
> True.
Thanks for confirming.
>> 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?
>
> Yes. (I skipped that code as edges were left dirty until the next init() call
> to reset it unconditionally, resize was just a realloc to trim array)
Thanks for confirming.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1814#discussion_r2132049582
PR Review Comment: https://git.openjdk.org/jfx/pull/1814#discussion_r2132050659
PR Review Comment: https://git.openjdk.org/jfx/pull/1814#discussion_r2132051163
PR Review Comment: https://git.openjdk.org/jfx/pull/1814#discussion_r2132051446
