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

Conversation

@dnfield
Copy link
Contributor

@dnfield dnfield commented Jun 5, 2019

Fixes flutter/flutter#33789

This will allow things like flutter driver to work, along with https://dart-review.googlesource.com/c/sdk/+/104948

Sorry for the size of the PR - the vast majority of the changes here are find and replaces for TRACE_* to FML_TRACE_*.

Key changes are in trace_event.h and dart_vm.cc.

trace_event.h:

  • Prefix all our trace macros with FML_.
  • Remove special casing for Fuchsia for these macros. Let Dart take care of forwarding these events to systrace. This is tested and working locally (both the timeline and the systrace are getting trace events this way).

dart_vm.cc:

  • Don't force fuchsia to do systrace only - only if the VM has been told to do systrace specifically. This means that by default now the engine will foward things over to Dart for tracing, which will take care of forwarding things over to systrace.

The linked bug I'm removing a reference to (DNO-448) is marked as resolved in Jira.

@dnfield
Copy link
Contributor Author

dnfield commented Jun 5, 2019

From offline conversation: it would be nice to rework these macros so they take variadic arguments and we can get rid of TRACE_FOO1/TRACE_FOO2. etc.

@chinmaygarde
Copy link
Member

I don't think your editor is setup to format on save when doing a search and replace. In case you are using Sublime, Sublime Hooks with the Clang Format plugin works well for me.

Copy link
Member

@zanderso zanderso left a comment

Choose a reason for hiding this comment

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

lgtm w/ nit

PushBackAll(&args, kDartTraceStreamsArgs, fml::size(kDartTraceStreamsArgs));
#endif

// PushBackAll(&args, kDartTraceStreamsArgs,
Copy link
Member

Choose a reason for hiding this comment

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

rm commented out code.

Copy link
Member

Choose a reason for hiding this comment

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

replace with a prose comment or TODO if needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@dnfield
Copy link
Contributor Author

dnfield commented Jun 5, 2019

I'm going to wait to land this until the related change in Dart is confirmed as good.

@dnfield dnfield merged commit 7826548 into flutter:master Jun 5, 2019
@dnfield dnfield deleted the fml_trace branch June 5, 2019 22:14
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 5, 2019
engine-flutter-autoroll added a commit to flutter/flutter that referenced this pull request Jun 6, 2019
flutter/engine@afb9d51...7826548

git log afb9d51..7826548 --no-merges --oneline
7826548 Align fuchsia and non-fuchsia tracing (flutter/engine#9199)
5ea125e Switch PlatformViewsController from Activity ref to Application ref. (flutter/engine#9193)
fd4368c Skip golden tests on non-Linux OSes (flutter/engine#9198)

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 (bmparr@google.com), and stop
the roller if necessary.
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 6, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 6, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 6, 2019
huqiuser pushed a commit to huqiuser/engine that referenced this pull request Jun 12, 2019
zanderso added a commit to zanderso/engine that referenced this pull request Jun 12, 2019
zanderso added a commit that referenced this pull request Jun 12, 2019
* Revert "[fuchsia] Fix alignment of Fuchsia/non-Fuchsia tracing (#9289)"

This reverts commit f80ac5f.

* Revert "Align fuchsia and non-fuchsia tracing (#9199)"

This reverts commit 7826548.
huqiuser pushed a commit to huqiuser/engine that referenced this pull request Jun 13, 2019
* Revert "[fuchsia] Fix alignment of Fuchsia/non-Fuchsia tracing (flutter#9289)"

This reverts commit f80ac5f.

* Revert "Align fuchsia and non-fuchsia tracing (flutter#9199)"

This reverts commit 7826548.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

flutter drive Fails to get durations

4 participants