Bug 273793

Summary:

Optimize BreakLines/nextBreakablePosition()

Product:

WebKit

Reporter:

fantasai <fantasai.bugs>

Component:

Layout and Rendering

Assignee:

fantasai <fantasai.bugs>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bfulgham, simon.fraser, webkit-bug-importer, zalan

Priority:

Keywords:

InRadar

Version:

Safari Technology Preview

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch 1: Optimizing the control flow	none
Patch 1: Optimizing the control flow	none
Patch 2: Fast-forward alongside ICU	none
Patch 3: WIP	none

fantasai

Reported 2024-05-06 13:50:56 PDT

The inner loop of nextBreakablePosition is so hot, it's had multiple "performance" fixes that did nothing but drop a simple conditional by templating it in. Let's fix up its control flow which is both convoluted and inefficient.

Attachments
Patch 1: Optimizing the control flow (6.68 KB, patch) 2024-05-06 14:05 PDT, fantasai	no flags	Details Formatted Diff Diff
Patch 1: Optimizing the control flow (7.58 KB, patch) 2024-05-08 14:31 PDT, fantasai	no flags	Details Formatted Diff Diff
Patch 2: Fast-forward alongside ICU (3.18 KB, patch) 2024-05-08 14:32 PDT, fantasai	no flags	Details Formatted Diff Diff
Patch 3: WIP (11.90 KB, patch) 2024-05-19 21:14 PDT, fantasai	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

fantasai

Comment 1 2024-05-06 14:05:10 PDT

Created attachment 471297 [details] Patch 1: Optimizing the control flow First draft of control flow optimization.

fantasai

Comment 2 2024-05-06 18:03:44 PDT

Pull request: https://github.com/WebKit/WebKit/pull/28214

fantasai

Comment 3 2024-05-08 14:31:42 PDT

Created attachment 471326 [details] Patch 1: Optimizing the control flow

fantasai

Comment 4 2024-05-08 14:32:12 PDT

Created attachment 471327 [details] Patch 2: Fast-forward alongside ICU

EWS

Comment 5 2024-05-08 21:34:28 PDT

Committed 278548@main (2b27eb4841d2): <https://commits.webkit.org/278548@main> Reviewed commits have been landed. Closing PR #28214 and removing active labels.

Radar WebKit Bug Importer

Comment 6 2024-05-08 21:35:26 PDT

<rdar://problem/127787501>

fantasai

Comment 7 2024-05-09 21:14:07 PDT

(Still working on this patch set...)

fantasai

Comment 8 2024-05-19 21:14:15 PDT

Created attachment 471441 [details] Patch 3: WIP Just bookmarking the current state of the patch, and also taking notes on what to work on next: - Add more characters, including punctuation and whitespace, to the set this handles locally. - Handle combining marks. Follow-up fixes: - Handle word-break. - Handle line-break. - Handle paragraph and line separator characters correctly. Benchmarks to create/compare: - Dump a bunch of Wikipedia article texts into some kind of perf test. - Compare not using ICU at all to what we're doing (e.g. by returning dummy values for everything outside ASCII) to find the performance ceiling.

fantasai

Comment 9 2024-05-23 01:31:58 PDT

Pull request: https://github.com/WebKit/WebKit/pull/28978

EWS

Comment 10 2024-05-24 08:58:26 PDT

Committed 279271@main (a7f897f869aa): <https://commits.webkit.org/279271@main> Reviewed commits have been landed. Closing PR #28323 and removing active labels.

EWS

Comment 11 2024-05-25 07:36:18 PDT

Committed 279318@main (e59f7bd6c41b): <https://commits.webkit.org/279318@main> Reviewed commits have been landed. Closing PR #28978 and removing active labels.

Note You need to log in before you can comment on or make changes to this bug.