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

Add trace events for creating minikin fonts - #8477

Merged
liyuqian merged 2 commits into
flutter-team-archive:masterfrom
liyuqian:trace_font
Apr 9, 2019
Merged

liyuqian merged 2 commits into
flutter-team-archive:masterfrom
liyuqian:trace_font

Conversation

@liyuqian

@liyuqian liyuqian commented Apr 7, 2019

Copy link
Copy Markdown
Contributor

Creating fonts can take a long time (50ms+) during frame build in the UI
thread. Trace them to help us understand when it's causing a jank.

For example:
Screen Shot 2019-04-07 at 11 37 11 AM

Creating fonts can take a long time (50ms+) during frame build in the UI
thread.  Trace them to help us understand when it's causing a jank.

@cbracken cbracken left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm_hanko

But you'll want to run clang-format on this.

std::shared_ptr<minikin::FontFamily> FontCollection::CreateMinikinFontFamily(
const sk_sp<SkFontMgr>& manager,
const std::string& family_name) {
TRACE_EVENT0("flutter", "FontCollection::CreateMinikinFontFamily");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Use the varargs variant FML_TRACE_EVENT to log the family name as well.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

How about the TRACE_EVENT1 in the new patch?

minikin_fonts.emplace_back(std::move(minikin_font));
}

TRACE_EVENT1("flutter", "minikin::FontFamily",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think a trace counter makes more sense here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I tried that in my first version and it's a little confusing to me because it no longer appears in the UI thread. Instead, if has its own grey box on the very top (similar to CPU/GPU clock counters). Is there a way to trace a counter, but show that as a normal box in the UI thread?

@liyuqian
liyuqian merged commit 9bfa96d into flutter-team-archive:master Apr 9, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 9, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 9, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 9, 2019
engine-flutter-autoroll added a commit to flutter/flutter that referenced this pull request Apr 9, 2019
flutter-team-archive/engine@6a0460c...eec74e5

git log 6a0460c..eec74e5 --no-merges --oneline
eec74e5 Rename the blink namespace to flutter. (flutter-team-archive/engine#8517)
fff80a0 Roll src/third_party/dart 06ed3f598c..06ed3f598c (0 commits)
000f8e3 Remove dart_utf_tag from DEPS
9bfa96d Add trace events for creating minikin fonts (flutter-team-archive/engine#8477)
5381e57 Roll src/third_party/skia 9a7c7be15969..b5d8c634f95b (11 commits) (flutter-team-archive/engine#8513)
df707c6 switch to newer APIs for shaders and filters (flutter-team-archive/engine#8511)

The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff ([email protected]), and stop
the roller if necessary.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Development

Successfully merging this pull request may close these issues.

4 participants