Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[Impeller] trimmed 3 bytes off of each Glyph #44054

Merged
auto-submit merged 2 commits into from
Jul 27, 2023
Merged

[Impeller] trimmed 3 bytes off of each Glyph #44054

auto-submit merged 2 commits into from
Jul 27, 2023

Conversation

@gaaclarke
Copy link
Member

@gaaclarke gaaclarke commented Jul 26, 2023 •
edited
Loading

While profiling Impeller I saw that we were allocating ~80MB/s of GlyphPositions. This should shave off 13MB/s by making Glyphs 20 bytes instead of 24.

Screenshot 2023-07-26 at 4 51 24 PM

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 and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

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

@gaaclarke gaaclarke requested a review from dnfield July 26, 2023 23:52
dnfield
dnfield reviewed Jul 27, 2023
View reviewed changes
impeller/typographer/glyph.h
: index(p_index), type(p_type), bounds(p_bounds) {}
};

static_assert(sizeof(Glyph) == 20);
Copy link
Contributor

@dnfield dnfield Jul 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment about this?

Copy link
Member Author

@gaaclarke gaaclarke Jul 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@jonahwilliams
Copy link
Contributor

jonahwilliams commented Jul 27, 2023

Nice LGTM

@gaaclarke gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 27, 2023
@auto-submit auto-submit bot merged commit 32f929f into flutter:main Jul 27, 2023
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jul 27, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 27, 2023
@engine-flutter-autoroll
32f929f94 [Impeller] trimmed 3 bytes off of each Glyph (flutter/engin…
5c9ea67 
…e#44054)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jul 27, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 27, 2023
@engine-flutter-autoroll
32f929f94 [Impeller] trimmed 3 bytes off of each Glyph (flutter/engin…
561ead0 
…e#44054)
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Jul 27, 2023
@engine-flutter-autoroll
Roll Flutter Engine from 9b14c382eebf to 08050cdecf94 (22 revisions) (#…
7e7a0a8 
…131426)

flutter/engine@9b14c38...08050cd

2023-07-27 [email protected] Roll Skia from 90048a938eb5 to 26ec2772960b (2 revisions) (flutter/engine#44070)
2023-07-27 [email protected] Roll ANGLE from 8aa3ca9d177c to d5ddb710e5bd (1 revision) (flutter/engine#44069)
2023-07-27 [email protected] [Impeller] trimmed 3 bytes off of each Glyph (flutter/engine#44054)
2023-07-27 [email protected] Roll Skia from 6571d88db5b3 to 90048a938eb5 (1 revision) (flutter/engine#44066)
2023-07-27 [email protected] Roll ANGLE from af5bf5b8245e to 8aa3ca9d177c (1 revision) (flutter/engine#44065)
2023-07-27 [email protected] Roll Dart SDK from b6747a06be06 to dcbaffbd09dd (1 revision) (flutter/engine#44064)
2023-07-27 [email protected] Roll Fuchsia Mac SDK from pgJmG2-qZVMzlMuq8... to EpwAiaHKf6M6Yejtk... (flutter/engine#44063)
2023-07-27 [email protected] Roll Skia from 26ed0d2ab04c to 6571d88db5b3 (1 revision) (flutter/engine#44062)
2023-07-27 [email protected] Roll Skia from e85c64d63787 to 26ed0d2ab04c (3 revisions) (flutter/engine#44061)
2023-07-27 [email protected] Roll Fuchsia Linux SDK from DOxjABQZ-mA_9Lllw... to FwIA6BaHtZg2mRS-0... (flutter/engine#44060)
2023-07-27 [email protected] Ignore the NOTICE.fuchsia file in the new license script (flutter/engine#44021)
2023-07-27 [email protected] Embed a license readme in every zip archive (flutter/engine#43974)
2023-07-27 [email protected] Roll Skia from 750d7f8ed4d7 to e85c64d63787 (4 revisions) (flutter/engine#44056)
2023-07-27 [email protected] Roll Dart SDK from 8961d36eaa0c to b6747a06be06 (1 revision) (flutter/engine#44053)
2023-07-26 [email protected] Roll ANGLE from 451b3c31a1fa to af5bf5b8245e (1 revision) (flutter/engine#44050)
2023-07-26 [email protected] Roll Skia from fdf224be4f98 to 750d7f8ed4d7 (3 revisions) (flutter/engine#44049)
2023-07-26 [email protected] Roll Dart SDK from b5710782b91c to 8961d36eaa0c (1 revision) (flutter/engine#44048)
2023-07-26 [email protected] Roll Skia from 990aa6016ff6 to fdf224be4f98 (3 revisions) (flutter/engine#44046)
2023-07-26 [email protected] Roll Fuchsia Mac SDK from d6O9t74z-k2svOmvz... to pgJmG2-qZVMzlMuq8... (flutter/engine#44045)
2023-07-26 [email protected] Roll ANGLE from 58cb5c8396a3 to 451b3c31a1fa (1 revision) (flutter/engine#44044)
2023-07-26 [email protected] Roll Skia from d76a9c3bb3fd to 990aa6016ff6 (4 revisions) (flutter/engine#44043)
2023-07-26 [email protected] Roll Skia from ca48e45a0262 to d76a9c3bb3fd (1 revision) (flutter/engine#44042)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from DOxjABQZ-mA_ to FwIA6BaHtZg2
  fuchsia/sdk/core/mac-amd64 from d6O9t74z-k2s to EpwAiaHKf6M6

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
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 Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
LouiseHsu pushed a commit to LouiseHsu/flutter that referenced this pull request Jul 31, 2023
@engine-flutter-autoroll @LouiseHsu
Roll Flutter Engine from 9b14c382eebf to 08050cdecf94 (22 revisions) (f…
68379bf 
…lutter#131426)

flutter/engine@9b14c38...08050cd

2023-07-27 [email protected] Roll Skia from 90048a938eb5 to 26ec2772960b (2 revisions) (flutter/engine#44070)
2023-07-27 [email protected] Roll ANGLE from 8aa3ca9d177c to d5ddb710e5bd (1 revision) (flutter/engine#44069)
2023-07-27 [email protected] [Impeller] trimmed 3 bytes off of each Glyph (flutter/engine#44054)
2023-07-27 [email protected] Roll Skia from 6571d88db5b3 to 90048a938eb5 (1 revision) (flutter/engine#44066)
2023-07-27 [email protected] Roll ANGLE from af5bf5b8245e to 8aa3ca9d177c (1 revision) (flutter/engine#44065)
2023-07-27 [email protected] Roll Dart SDK from b6747a06be06 to dcbaffbd09dd (1 revision) (flutter/engine#44064)
2023-07-27 [email protected] Roll Fuchsia Mac SDK from pgJmG2-qZVMzlMuq8... to EpwAiaHKf6M6Yejtk... (flutter/engine#44063)
2023-07-27 [email protected] Roll Skia from 26ed0d2ab04c to 6571d88db5b3 (1 revision) (flutter/engine#44062)
2023-07-27 [email protected] Roll Skia from e85c64d63787 to 26ed0d2ab04c (3 revisions) (flutter/engine#44061)
2023-07-27 [email protected] Roll Fuchsia Linux SDK from DOxjABQZ-mA_9Lllw... to FwIA6BaHtZg2mRS-0... (flutter/engine#44060)
2023-07-27 [email protected] Ignore the NOTICE.fuchsia file in the new license script (flutter/engine#44021)
2023-07-27 [email protected] Embed a license readme in every zip archive (flutter/engine#43974)
2023-07-27 [email protected] Roll Skia from 750d7f8ed4d7 to e85c64d63787 (4 revisions) (flutter/engine#44056)
2023-07-27 [email protected] Roll Dart SDK from 8961d36eaa0c to b6747a06be06 (1 revision) (flutter/engine#44053)
2023-07-26 [email protected] Roll ANGLE from 451b3c31a1fa to af5bf5b8245e (1 revision) (flutter/engine#44050)
2023-07-26 [email protected] Roll Skia from fdf224be4f98 to 750d7f8ed4d7 (3 revisions) (flutter/engine#44049)
2023-07-26 [email protected] Roll Dart SDK from b5710782b91c to 8961d36eaa0c (1 revision) (flutter/engine#44048)
2023-07-26 [email protected] Roll Skia from 990aa6016ff6 to fdf224be4f98 (3 revisions) (flutter/engine#44046)
2023-07-26 [email protected] Roll Fuchsia Mac SDK from d6O9t74z-k2svOmvz... to pgJmG2-qZVMzlMuq8... (flutter/engine#44045)
2023-07-26 [email protected] Roll ANGLE from 58cb5c8396a3 to 451b3c31a1fa (1 revision) (flutter/engine#44044)
2023-07-26 [email protected] Roll Skia from d76a9c3bb3fd to 990aa6016ff6 (4 revisions) (flutter/engine#44043)
2023-07-26 [email protected] Roll Skia from ca48e45a0262 to d76a9c3bb3fd (1 revision) (flutter/engine#44042)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from DOxjABQZ-mA_ to FwIA6BaHtZg2
  fuchsia/sdk/core/mac-amd64 from d6O9t74z-k2s to EpwAiaHKf6M6

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
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 Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
vashworth pushed a commit to vashworth/flutter that referenced this pull request Aug 2, 2023
@engine-flutter-autoroll @vashworth
Roll Flutter Engine from 9b14c382eebf to 08050cdecf94 (22 revisions) (f…
fa6abce 
…lutter#131426)

flutter/engine@9b14c38...08050cd

2023-07-27 [email protected] Roll Skia from 90048a938eb5 to 26ec2772960b (2 revisions) (flutter/engine#44070)
2023-07-27 [email protected] Roll ANGLE from 8aa3ca9d177c to d5ddb710e5bd (1 revision) (flutter/engine#44069)
2023-07-27 [email protected] [Impeller] trimmed 3 bytes off of each Glyph (flutter/engine#44054)
2023-07-27 [email protected] Roll Skia from 6571d88db5b3 to 90048a938eb5 (1 revision) (flutter/engine#44066)
2023-07-27 [email protected] Roll ANGLE from af5bf5b8245e to 8aa3ca9d177c (1 revision) (flutter/engine#44065)
2023-07-27 [email protected] Roll Dart SDK from b6747a06be06 to dcbaffbd09dd (1 revision) (flutter/engine#44064)
2023-07-27 [email protected] Roll Fuchsia Mac SDK from pgJmG2-qZVMzlMuq8... to EpwAiaHKf6M6Yejtk... (flutter/engine#44063)
2023-07-27 [email protected] Roll Skia from 26ed0d2ab04c to 6571d88db5b3 (1 revision) (flutter/engine#44062)
2023-07-27 [email protected] Roll Skia from e85c64d63787 to 26ed0d2ab04c (3 revisions) (flutter/engine#44061)
2023-07-27 [email protected] Roll Fuchsia Linux SDK from DOxjABQZ-mA_9Lllw... to FwIA6BaHtZg2mRS-0... (flutter/engine#44060)
2023-07-27 [email protected] Ignore the NOTICE.fuchsia file in the new license script (flutter/engine#44021)
2023-07-27 [email protected] Embed a license readme in every zip archive (flutter/engine#43974)
2023-07-27 [email protected] Roll Skia from 750d7f8ed4d7 to e85c64d63787 (4 revisions) (flutter/engine#44056)
2023-07-27 [email protected] Roll Dart SDK from 8961d36eaa0c to b6747a06be06 (1 revision) (flutter/engine#44053)
2023-07-26 [email protected] Roll ANGLE from 451b3c31a1fa to af5bf5b8245e (1 revision) (flutter/engine#44050)
2023-07-26 [email protected] Roll Skia from fdf224be4f98 to 750d7f8ed4d7 (3 revisions) (flutter/engine#44049)
2023-07-26 [email protected] Roll Dart SDK from b5710782b91c to 8961d36eaa0c (1 revision) (flutter/engine#44048)
2023-07-26 [email protected] Roll Skia from 990aa6016ff6 to fdf224be4f98 (3 revisions) (flutter/engine#44046)
2023-07-26 [email protected] Roll Fuchsia Mac SDK from d6O9t74z-k2svOmvz... to pgJmG2-qZVMzlMuq8... (flutter/engine#44045)
2023-07-26 [email protected] Roll ANGLE from 58cb5c8396a3 to 451b3c31a1fa (1 revision) (flutter/engine#44044)
2023-07-26 [email protected] Roll Skia from d76a9c3bb3fd to 990aa6016ff6 (4 revisions) (flutter/engine#44043)
2023-07-26 [email protected] Roll Skia from ca48e45a0262 to d76a9c3bb3fd (1 revision) (flutter/engine#44042)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from DOxjABQZ-mA_ to FwIA6BaHtZg2
  fuchsia/sdk/core/mac-amd64 from d6O9t74z-k2s to EpwAiaHKf6M6

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
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 Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
gaaclarke added a commit to gaaclarke/engine that referenced this pull request Aug 30, 2023
@gaaclarke
[Impeller] trimmed 3 bytes off of each Glyph (flutter#44054)
e51038b 
While profiling Impeller I saw that we were allocating ~80MB/s of `GlyphPosition`s.  This should shave off 13MB/s by making Glyphs 20 bytes instead of 24.

<img width="1220" alt="Screenshot 2023-07-26 at 4 51 24 PM" src="https://github.com/flutter/engine/assets/30870216/9e65e8bf-9333-4dd1-aa0b-be771412d07b">

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@dnfield dnfield dnfield approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

autosubmit Merge PR when tree becomes green via auto submit App e: impeller

Projects

No open projects
Impeller
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@gaaclarke @jonahwilliams @dnfield