RFR: 8359955: Regressions ~7% in several J2DBench in 25-b26

[Daniel Gredler]

On Tue, 19 Aug 2025 00:39:41 GMT, Sergey Bylokhov <serb at 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