Avoid calling `TextPainter.plainText` for simple static text #146084

LongCatIsLooong · 2024-04-01T18:03:03Z

InlineSpan.toPlainText is surprisingly expensive even for a simple TextSpan like TextSpan(text: 'AAA', children: [TextSpan(text: char * 10), TextSpan(text: char * 20)])

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
I followed the breaking change policy and added Data Driven Fixes where supported.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

LongCatIsLooong · 2024-04-01T18:32:11Z

I'm supposed to get a "please add a test" message from the bot right?

goderbauer · 2024-04-01T18:33:42Z

I'm supposed to get a "please add a test" message from the bot right?

Yeah, I am surprised the bot is silent on this one.

flutter-dashboard · 2024-04-01T19:56:01Z

This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

LongCatIsLooong · 2024-04-01T20:21:09Z

with

TextSpan getSpan(int seed) {
  final char = getString(seed);
  return TextSpan(text: 'AAA', children: [TextSpan(text: char * 10), TextSpan(text: char * 20)]);
}

  final spans = List.generate(iters, getSpan);
  final stopwatch = Stopwatch()..start();
  for (int i = 0; i < iters; i++) {
    painter.text = spans[i];
    painter.plainText;
  }
  stopwatch.stop();
  print('time elapsed: ${stopwatch.elapsedMilliseconds} ms. ${stopwatch.elapsedMicroseconds / iters} microseconds per iteration.');
  exit(0);

I get

1.32038 microseconds per iteration.

That doesn't seem bad? How does that take 20% of the TextPainter.layout time.

goderbauer

LGTM

…lutter/flutter#146084)

Roll Flutter from a418568 to e868e2b (34 revisions) flutter/flutter@a418568...e868e2b 2024-04-03 [email protected] Add SegmentedButton expand feature (flutter/flutter#142804) 2024-04-03 [email protected] Roll Flutter Engine from e36b9b10c36f to 56fa2c33a5f7 (1 revision) (flutter/flutter#146205) 2024-04-03 [email protected] Roll Packages from 83f3842 to 0e848fa (3 revisions) (flutter/flutter#146201) 2024-04-03 [email protected] Enhance ColorScheme.fromSeed with a new `variant` parameter (flutter/flutter#144805) 2024-04-03 [email protected] Roll Flutter Engine from 0280de5be276 to e36b9b10c36f (1 revision) (flutter/flutter#146200) 2024-04-03 [email protected] Fix typos in bottom_sheet.dart (flutter/flutter#146188) 2024-04-03 [email protected] Roll Flutter Engine from 979030d81f8d to 0280de5be276 (2 revisions) (flutter/flutter#146199) 2024-04-03 [email protected] Roll Flutter Engine from bef3bbe3f74e to 979030d81f8d (1 revision) (flutter/flutter#146186) 2024-04-03 [email protected] Roll Flutter Engine from 5fc83bc24b2e to bef3bbe3f74e (1 revision) (flutter/flutter#146183) 2024-04-03 [email protected] Roll Flutter Engine from 0da1b2eb370a to 5fc83bc24b2e (1 revision) (flutter/flutter#146180) 2024-04-03 [email protected] Avoid calling `TextPainter.plainText` for simple static text (flutter/flutter#146084) 2024-04-03 [email protected] Roll Flutter Engine from e603f89844a9 to 0da1b2eb370a (2 revisions) (flutter/flutter#146179) 2024-04-03 [email protected] Roll Flutter Engine from ef60a95d78c1 to e603f89844a9 (3 revisions) (flutter/flutter#146177) 2024-04-03 [email protected] Fix chip baseline implementation (flutter/flutter#146162) 2024-04-03 [email protected] Roll Flutter Engine from bb4ec2d7eb39 to ef60a95d78c1 (1 revision) (flutter/flutter#146176) 2024-04-03 [email protected] Add tests for material_state_mouse_cursor.0.dart API example. (flutter/flutter#145987) 2024-04-03 [email protected] Update material_color_utilities package version to latest 0.11.1 (flutter/flutter#145959) 2024-04-03 [email protected] Roll Flutter Engine from 5f6dec8bd877 to bb4ec2d7eb39 (4 revisions) (flutter/flutter#146169) 2024-04-03 [email protected] Roll Flutter Engine from 5dbcfdc2a456 to 5f6dec8bd877 (1 revision) (flutter/flutter#146163) 2024-04-02 [email protected] Dispose FocusNode in tests. (flutter/flutter#146161) 2024-04-02 [email protected] Add `none` language strings to code blocks. (flutter/flutter#146154) 2024-04-02 [email protected] Refactor docs (flutter/flutter#145998) 2024-04-02 [email protected] Roll Flutter Engine from c60b00a20fc3 to 5dbcfdc2a456 (3 revisions) (flutter/flutter#146159) 2024-04-02 [email protected] Marks Linux_pixel_7pro complex_layout_scroll_perf_impeller__timeline_summary to be unflaky (flutter/flutter#140038) 2024-04-02 [email protected] Roll Flutter Engine from 5bf8b94505a4 to c60b00a20fc3 (2 revisions) (flutter/flutter#146157) 2024-04-02 [email protected] Refactor analyze (flutter/flutter#146138) 2024-04-02 [email protected] Implement SelectionArea triple click gestures (flutter/flutter#144563) 2024-04-02 [email protected] Roll Flutter Engine from 6883f7313da0 to 5bf8b94505a4 (2 revisions) (flutter/flutter#146152) 2024-04-02 [email protected] Fix border color is wrong for a focused and hovered TextField (flutter/flutter#146127) 2024-04-02 [email protected] Sync lints and enable `annotate_redeclares` (flutter/flutter#146144) 2024-04-02 [email protected] Implements `RenderBox.computeDryBaseline` for material render boxes (flutter/flutter#146027) 2024-04-02 [email protected] Roll Flutter Engine from 523fc953ebc8 to 6883f7313da0 (2 revisions) (flutter/flutter#146140) 2024-04-02 [email protected] Fix `MenuItemButton` overflow (flutter/flutter#143932) 2024-04-02 [email protected] Roll Packages from d5aff19 to 83f3842 (4 revisions) (flutter/flutter#146134) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 ...

Avoid calling TextPainter.plainText for simple text

e361523

github-actions bot added a: text input Entering text in a text field or keyboard related problems framework flutter/packages/flutter repository. See also f: labels. labels Apr 1, 2024

LongCatIsLooong requested a review from goderbauer April 1, 2024 18:30

LongCatIsLooong marked this pull request as ready for review April 1, 2024 18:30

LongCatIsLooong marked this pull request as draft April 1, 2024 19:55

LongCatIsLooong marked this pull request as ready for review April 1, 2024 23:39

goderbauer approved these changes Apr 2, 2024

View reviewed changes

LongCatIsLooong added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 3, 2024

auto-submit bot merged commit 3295a48 into flutter:master Apr 3, 2024

LongCatIsLooong deleted the no-plain-text branch April 3, 2024 06:36

engine-flutter-autoroll mentioned this pull request Apr 3, 2024

Roll Flutter from a418568a6498 to bd4c17155d02 (28 revisions) flutter/packages#6452

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 3, 2024

3295a48 Avoid calling TextPainter.plainText for simple static text (f…

97deeb6

…lutter/flutter#146084)

engine-flutter-autoroll mentioned this pull request Apr 3, 2024

Roll Flutter from a418568a6498 to 1326c8ee536e (29 revisions) flutter/packages#6454

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 3, 2024

3295a48 Avoid calling TextPainter.plainText for simple static text (f…

e1ee728

…lutter/flutter#146084)

engine-flutter-autoroll mentioned this pull request Apr 3, 2024

Roll Flutter from a418568a6498 to e868e2b3834f (34 revisions) flutter/packages#6455

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 3, 2024

3295a48 Avoid calling TextPainter.plainText for simple static text (f…

4d1b9f5

…lutter/flutter#146084)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Avoid calling `TextPainter.plainText` for simple static text #146084

Avoid calling `TextPainter.plainText` for simple static text #146084

Uh oh!

LongCatIsLooong commented Apr 1, 2024

Uh oh!

LongCatIsLooong commented Apr 1, 2024

Uh oh!

goderbauer commented Apr 1, 2024

Uh oh!

flutter-dashboard bot commented Apr 1, 2024

Uh oh!

LongCatIsLooong commented Apr 1, 2024

Uh oh!

goderbauer left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Avoid calling TextPainter.plainText for simple static text #146084

Avoid calling TextPainter.plainText for simple static text #146084

Uh oh!

Conversation

LongCatIsLooong commented Apr 1, 2024

Pre-launch Checklist

Uh oh!

LongCatIsLooong commented Apr 1, 2024

Uh oh!

goderbauer commented Apr 1, 2024

Uh oh!

flutter-dashboard bot commented Apr 1, 2024

Uh oh!

LongCatIsLooong commented Apr 1, 2024

Uh oh!

goderbauer left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Avoid calling `TextPainter.plainText` for simple static text #146084

Avoid calling `TextPainter.plainText` for simple static text #146084