Skip to content

Keep the line breaker's per-atom fast path free of mixed-style atom handling - #875

Draft
nicoburns wants to merge 1 commit into
linebender:mainfrom
DioxusLabs:devin/1790906099-fix-864-break
Draft

nicoburns wants to merge 1 commit into
linebender:mainfrom
DioxusLabs:devin/1790906099-fix-864-break

Conversation

@nicoburns

@nicoburns nicoburns commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator

LLM Contributions: Generated with Opus 5.5 High

Fixes a linebreaking performance regression from #864 ("Handle line heights changing within atoms").
My LLMs break_all_lines+align benchmark on a ~250KB uniform-style English document went 3.27ms → 3.98ms (+22%).

DRAFT because benchmarking is currently showing that #855 gains all the benefits of this change and more

Root Causes

  • BreakerState::append_atom_to_line stopped inlining into break_next and became an out-of-line call per atom
  • LineBoxMetrics::add_text did extra work per atom before the last_text early-return: an eager atom.characters() slice (with bounds check) and a data.runs[run_idx].has_mixed_style_atoms load.

Fixes

  • Pass the atom's characters as atom.char_range() and only slice ShapedText::characters() on the slow path.
  • Encode the has_mixed_style_atoms into (item_idx, style_index). add_text_boxes sets it to the NO_LAST_TEXT sentinel after adding an atom of a run with mixed-style atoms, so such runs never hit the early return. The fast path check becomes just last_text == (item_idx, style_index) again.

Benchmarked "break" phase:

  • 8af7c03 3.27–3.33ms,
  • main 3.98–4.04ms
  • This PR 3.26–3.31ms

Changelog: None

…andling

linebender#864 made append_atom_to_line no longer inline into break_next (it
became an out-of-line call per atom), and added work to the fast path of
LineBoxMetrics::add_text on every atom: an eager atom.characters()
slice (with bounds check) and a data.runs[run_idx].has_mixed_style_atoms
load.

Instead, fold the flag into last_text: add_text_boxes resets last_text
to a sentinel after adding an atom of a run with mixed-style atoms, so
such runs never take the early return. The atom's characters are passed
as a char range and only sliced on the slow path.

Add a test where a mixed-style ligature follows an atom of the same run
and first style.
@nicoburns nicoburns changed the title Keep the line breaker's per-atom fast path free of mixed-style atom h… Keep the line breaker's per-atom fast path free of mixed-style atom handling Oct 2, 2026
@tomcur

tomcur commented Oct 2, 2026 •

Copy link
Copy Markdown
Member

My LLMs break_all_lines+align benchmark on a ~250KB uniform-style English document went 3.27ms → 3.98ms (+22%).

Interesting! For me 5794c746 (#864) vs 8af7c030 (#863, that PR's parent, which was the performance side of this) still benches close to neutral.

Line Breaking - arabic 8000 characters, narrow     [ 206.7 us ... 205.7 us ]      -0.49%
Line Breaking - arabic 8000 characters, wider      [ 107.1 us ... 108.9 us ]      +1.64%*
Line Breaking - arabic 8000 characters, unwrapped  [  78.4 us ...  80.8 us ]      +3.08%*
Line Breaking - latin 8000 characters, narrow      [ 172.8 us ... 172.0 us ]      -0.46%
Line Breaking - latin 8000 characters, wider       [  82.4 us ...  83.3 us ]      +1.04%*
Line Breaking - latin 8000 characters, unwrapped   [  52.2 us ...  53.5 us ]      +2.58%*
Line Breaking - japanese 8000 characters, narrow   [ 215.1 us ... 214.7 us ]      -0.23%
Line Breaking - japanese 8000 characters, wider    [ 143.2 us ... 145.5 us ]      +1.58%*
Line Breaking - japanese 8000 characters, unwrapped [ 121.0 us ... 121.6 us ]      +0.53%
Line Breaking - arabic 8000 characters, wrapped + spacing [ 116.3 us ... 118.7 us ]      +2.01%*
Line Breaking - latin 8000 characters, wrapped + spacing [  92.3 us ...  93.2 us ]      +0.98%
Line Breaking - japanese 8000 characters, wrapped + spacing [ 148.6 us ... 150.0 us ]      +0.98%
Page Break - arabic 8000 characters                [  86.2 us ...  88.9 us ]      +3.17%*
Page Break - latin 8000 characters                 [  61.1 us ...  62.9 us ]      +3.01%*
Page Break - japanese 8000 characters              [ 133.6 us ... 133.2 us ]      -0.31%

And versus that PR's parent's parent 97beea38, quite positive:

Line Breaking - arabic 8000 characters, narrow     [ 230.1 us ... 210.9 us ]      -8.31%*
Line Breaking - arabic 8000 characters, wider      [ 123.9 us ... 112.1 us ]      -9.57%*
Line Breaking - arabic 8000 characters, unwrapped  [  92.7 us ...  82.4 us ]     -11.10%*
Line Breaking - latin 8000 characters, narrow      [ 183.5 us ... 176.1 us ]      -4.05%*
Line Breaking - latin 8000 characters, wider       [  87.3 us ...  84.4 us ]      -3.27%*
Line Breaking - latin 8000 characters, unwrapped   [  55.5 us ...  54.4 us ]      -1.99%*
Line Breaking - japanese 8000 characters, narrow   [ 234.6 us ... 218.6 us ]      -6.82%*
Line Breaking - japanese 8000 characters, wider    [ 154.2 us ... 145.2 us ]      -5.86%*
Line Breaking - japanese 8000 characters, unwrapped [ 129.8 us ... 122.7 us ]      -5.44%*
Line Breaking - arabic 8000 characters, wrapped + spacing [ 133.1 us ... 119.2 us ]     -10.46%*
Line Breaking - latin 8000 characters, wrapped + spacing [  95.4 us ...  93.0 us ]      -2.53%*
Line Breaking - japanese 8000 characters, wrapped + spacing [ 160.6 us ... 150.3 us ]      -6.44%*
Page Break - arabic 8000 characters                [  99.0 us ...  88.2 us ]     -10.88%*
Page Break - latin 8000 characters                 [  64.2 us ...  62.8 us ]      -2.05%*
Page Break - japanese 8000 characters              [ 143.6 us ... 135.0 us ]      -6.00%*

@tomcur

tomcur commented Oct 2, 2026

Copy link
Copy Markdown
Member

#855 that landed gives a further nice speed-up on our benches. What exactly is your break_all_lines+align doing?

@nicoburns

Copy link
Copy Markdown
Collaborator Author

My benchmarks are showing something like -8% from #863, then +18% from #864, then -25% from #855 (which for some reason seems to subsume the benefits of this PR). I think it's partly due to inlining decisions. So it may depend on compiler settings (notably codegen-units) and the phase of the moon....

@nicoburns

Copy link
Copy Markdown
Collaborator Author

(not necessarily intending to land this)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants