On Mon, 12 May 2025 20:35:52 GMT, Daniel Gredler <dgred...@openjdk.org> wrote:
> `LineBreakMeasurer.nextLayout()` calls `nextOffset()` internally to calculate
> the layout limit. When this happens, a `GlyphVector` is created and the
> layout engine is invoked to shape the text. The `GlyphVector` is cached in
> the relevant `ExtendedTextSourceLabel` component.
>
> `LineBreakMeasurer.nextLayout()` then calls `TextMeasurer.getLayout()` which
> eventually asks that same `ExtendedTextSourceLabel` component for a subset
> component. This triggers the creation of a fresh `ExtendedTextSourceLabel`
> without the cached `GlyphVector`.
>
> However, this fresh `ExtendedTextSourceLabel` is not necessary if the subset
> requested perfectly matches the already-existing `ExtendedTextSourceLabel`
> component. This happens when the text is short enough that no line break is
> needed.
>
> I think we should change `ExtendedTextSourceLabel.getSubset()` to return
> `this` if the requested subset is identical to the existing instance. This
> will allow us to use the existing cached `GlyphVector`, and the call to
> `LineBreakMeasurer.nextLayout()` will trigger text shaping once, rather than
> twice.
>
> In local testing, the test program below ran in ~1250 ms before this
> optimization, and ran in ~960 ms after the change (a 23% reduction in run
> time).
>
> The following three existing test classes provide good regression test
> coverage for this change:
> - test/jdk/java/awt/font/LineBreakMeasurer/LineBreakWithTrackingAuto
> - test/jdk/java/awt/font/LineBreakMeasurer/TestLineBreakWithFontSub
> - test/jdk/java/awt/font/LineBreakMeasurer/FRCTest
>
>
> public class LineBreakMeasurerPerfTest {
>
> public static void main(String[] args) {
>
> float advance = 0;
> long start = System.currentTimeMillis();
> AttributedString string = new AttributedString("This is a test.");
> FontRenderContext frc = new FontRenderContext(new AffineTransform(),
> true, true);
>
> for (int i = 0; i < 100_000; i++) {
> LineBreakMeasurer measurer = new
> LineBreakMeasurer(string.getIterator(), frc);
> TextLayout layout = measurer.nextLayout(999); // large enough to
> not require break
> advance = Math.max(advance, layout.getAdvance());
> }
>
> long end = System.currentTimeMillis();
> System.out.println((end - start) + " ms elapsed (advance: " + advance
> + ")");
> }
>
> }
LGTM. Ran LineBreakMeasurer tests, in particular the manual test
LineBreakWithTracking.java to see if there was an effect of tracking + manual
window resize on the line break lengths. There were multiple instances when the
same object was returned so the fix looks beneficial in those case. Did not
observe any regressions.
With the POC, noticed ~1000ms difference without and with fix on some runs.
-------------
Marked as reviewed by honkar (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/25193#pullrequestreview-2862595417
