On Tue, 19 Aug 2025 00:39:41 GMT, Sergey Bylokhov <s...@openjdk.org> wrote:
>> Addresses recent slight performance regressions in some J2DBench benchmarks
>> focused on text drawing.
>>
>> `CCharToGlyphMapper` and `CompositeGlyphMapper` cache glyph IDs, but after
>> JDK-8353230 they weren't caching glyph IDs for chars which might be affected
>> by the raw / non-raw glyph distinction, since the cached value may not be
>> correct if we ask for a raw glyph ID one time, but a non-raw glyph ID the
>> next time (or vice versa). This caching exception was the reason for the
>> slightly degraded performance (the `CCharToGlyphMapper` behavior was
>> affecting macOS, and the `CompositeGlyphMapper` behavior was affecting some
>> versions of Windows). This change splits the cache in each of these two
>> classes into two caches, one for raw glyph IDs and one for non-raw glyph
>> IDs, so that all glyphs can benefit from caching.
>>
>> All of the font tests (`make test TEST="jtreg:test/jdk/java/awt/font"`) pass
>> for me locally with this change on Linux, macOS and Windows.
>
> src/java.desktop/macosx/classes/sun/font/CCharToGlyphMapper.java line 287:
>
>> 285: } else if (isIgnorableWhitespace(code) ||
>> (isDefaultIgnorable(code) && !raw)) {
>> 286: values[i] = INVISIBLE_GLYPH_ID;
>> 287: put(code, INVISIBLE_GLYPH_ID);
>
> Why do we need to change these lines instead of changing "get()" method above
> to use "this.raw" instead of parameter?
I'm not sure which `get` method you're referring to, but within `Cache` all
`raw` parameters should indeed be gone now, and there should only be references
to `this.raw`.
The key to the performance improvement is to allow values to be retrieved from
the cache *before* any `isIgnorableWhitespace` or `isDefaultIgnorable` checks,
and to make sure that all values are cached for future use (even if these
checks determine that special handling is needed). That's why these extra lines
are moved out of `get(int)`, so that they run only if/after a cache lookup
fails.
Let me know if that makes sense, or if I misunderstood your question.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/26702#discussion_r2284884051
