On Tue, 29 Aug 2023 22:04:40 GMT, Phil Race <p...@openjdk.org> wrote:
> 8318364: Add an FFM-based implementation of harfbuzz OpenType layout
src/java.desktop/share/classes/sun/font/HBShaper.java line 425:
> 423: float startY = (float)startPt.getY();
> 424:
> 425: MemorySegment matrix = arena.allocateArray(JAVA_FLOAT,
> mat.length);
There should be an overload of `allocateArray` which takes a Java array
directly and then copies it off-heap after allocation. In Java 22 this method
is called `allocateFrom` and is much more optimized (as it avoids zeroing of
memory). But, even in 21, the call to copy seems redundant - you can just use
the correct overload of `SegmentAllocator::allocateArray`
src/java.desktop/share/classes/sun/font/HBShaper.java line 613:
> 611: for (int i=0; i<glyphCount; i++) {
> 612: int storei = i + initialCount;
> 613: int cluster = (int)clusterHandle.get(glyphInfoArr, i) -
> offset;
All the var handle calls in this loop are not exact - e.g. they use a `int`
offset instead of a `long` one. Because of this, the memory access cannot be
fully optimized. Adding a cast to `long` on all offsets coordinates yields a
significant performance boost.
To avoid issues like these, it is recommended to set up the var handle using
the `VarHandle::withInvokeExactBehavior` method, which will cause an exception
to be thrown in case there's a type mismatch (similar to
`MethodHandle::invokeExact`).
src/java.desktop/share/native/libfontmanager/HBShaper_Panama.c line 80:
> 78: int flags,
> 79: int slot,
> 80: hb_font_get_nominal_glyph_func_t nominal_fn,
It shouldn't be necessary to pass all the functions here. Note that the upcalls
are now effectively static (due to the use of scope values). It would be more
efficient to create the array of functions once and for all in Java code
(either directly, by creating a memory segment and storing all the function
pointers in there, or indirectly, by calling the `_hb_jdk_get_font_funcs`
native function). But, we don't need to create a function array each time we
call the `shape` function (as all the functions in the array are going to be
the same after all). If you do that, you can replace all the `_fn` parameters
in here with a single function pointer array parameter.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15476#discussion_r1342570582
PR Review Comment: https://git.openjdk.org/jdk/pull/15476#discussion_r1345600057
PR Review Comment: https://git.openjdk.org/jdk/pull/15476#discussion_r1342567487
