On Mon, 24 Jun 2024 22:25:22 GMT, Phil Race <p...@openjdk.org> wrote:

>> Migrate font code from jdk.internal.misc.Unsafe to using FFM.
>> This reduces the coupling between the java.desktop module and the internals 
>> of the java.base module.
>> 
>> The code being changed here is not particularly performance sensitive, and 
>> it is not executed in the most common cases.
>> The main impact performance-wise is a total of around 37ms in initialisation 
>> costs on my x64 macbook.
>> A minimal program that just draws a string to an image - does not even put 
>> up a window - runs at around 690-700ms.
>> There's variability in that number and the overall time for a JDK without 
>> the change is around (660-670ms)
>> In the small test, this is the first and only use of FFM, so the one-off 
>> part cost should move elsewhere when FFM starts
>> to be used earlier in the JDK itself.
>
> Phil Race has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8334495

Just some suggestions on comments

src/java.desktop/share/classes/sun/font/GlyphList.java line 364:

> 362:          * a native call which does a getPrimitiveArrayCritical and a
> 363:          * memcpy for the typical amount of image data (30-150 bytes)
> 364:          * Consider a native method if there is a performance problem 
> (which

should this comment be removed now that we aren't using the unsafe loop anymore?

src/java.desktop/unix/classes/sun/font/XRGlyphCacheEntry.java line 86:

> 84:         // 'void*' (see field 'cellInfo' of struct 'GlyphInfo'
> 85:         // in src/share/native/sun/font/fontscalerdefs.h).
> 86:         // On 64-bit Big-endian architectures it would be wrong to access 
> this

"wrong to write this field as an int"?

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

Marked as reviewed by achung (Committer).

PR Review: https://git.openjdk.org/jdk/pull/19777#pullrequestreview-2142946771
PR Review Comment: https://git.openjdk.org/jdk/pull/19777#discussion_r1655510996
PR Review Comment: https://git.openjdk.org/jdk/pull/19777#discussion_r1655526018

Reply via email to