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
> + ")");
> }
>
> }
The conditions for the case where we'd return "as-is" makes sense to me. I
tested these with & without the changes as well and saw some reductions to the
total elapsed runtime. Although some are small, there's some variance and I
never saw an increase to the runtime total. Change LGTM.
-------------
Marked as reviewed by dnguyen (Committer).
PR Review: https://git.openjdk.org/jdk/pull/25193#pullrequestreview-2862472099
