From f173e72e069bebc9c6d6d162e90229437fc5fd65 Mon Sep 17 00:00:00 2001 From: Chris Bracken Date: Tue, 6 Aug 2019 11:26:34 -0700 Subject: [PATCH] Migrate Fuchsia runners to SDK tracing API (flutter/engine#10478) Migrates the Fuchsia Flutter and Dart runners off the internal tracing APIs and onto the public SDK. --- engine/src/flutter/fml/trace_event.h | 14 ++++++++++++-- .../flutter/shell/common/skia_event_tracer_impl.cc | 2 +- .../shell/platform/fuchsia/dart_runner/BUILD.gn | 2 ++ .../shell/platform/fuchsia/dart_runner/main.cc | 4 ++-- .../fuchsia/dart_runner/mapped_resource.cc | 7 +------ .../shell/platform/fuchsia/flutter/BUILD.gn | 3 +++ .../flutter/shell/platform/fuchsia/flutter/main.cc | 9 ++------- .../platform/fuchsia/flutter/platform_view.cc | 4 ---- .../shell/platform/fuchsia/flutter/runner.cc | 11 +---------- .../shell/platform/fuchsia/flutter/runner.h | 9 ++------- engine/src/flutter/shell/testing/BUILD.gn | 6 +----- 11 files changed, 27 insertions(+), 44 deletions(-) diff --git a/engine/src/flutter/fml/trace_event.h b/engine/src/flutter/fml/trace_event.h index d9c13367bfa..a5b478ffc15 100644 --- a/engine/src/flutter/fml/trace_event.h +++ b/engine/src/flutter/fml/trace_event.h @@ -15,7 +15,9 @@ // TODO(DNO-448): This is disabled because the Fuchsia counter id json parsing // only handles ints whereas this can produce ints or strings. -#define FML_TRACE_COUNTER(a, b, c, args...) +#define FML_TRACE_COUNTER(a, b, c, arg1, ...) \ + ::fml::tracing::TraceCounterNopHACK((a), (b), (c), (arg1), __VA_ARGS__); + #define FML_TRACE_EVENT(a, b, args...) TRACE_DURATION(a, b) #define TRACE_EVENT0(a, b) TRACE_DURATION(a, b) @@ -49,7 +51,7 @@ __LINE__)(name); // This macro has the FML_ prefix so that it does not collide with the macros -// from trace/event.h on Fuchsia. +// from lib/trace/event.h on Fuchsia. // // TODO(chinmaygarde): All macros here should have the FML prefix. #define FML_TRACE_COUNTER(category_group, name, counter_id, arg1, ...) \ @@ -174,6 +176,14 @@ void TraceCounter(TraceArg category, split.first, split.second); } +// HACK: Used to NOP FML_TRACE_COUNTER macro without triggering unused var +// warnings at usage sites. +template +void TraceCounterNopHACK(TraceArg category, + TraceArg name, + TraceIDArg identifier, + Args... args) {} + template void TraceEvent(TraceArg category, TraceArg name, Args... args) { auto split = SplitArguments(args...); diff --git a/engine/src/flutter/shell/common/skia_event_tracer_impl.cc b/engine/src/flutter/shell/common/skia_event_tracer_impl.cc index 43b103209c8..a44a39b214e 100644 --- a/engine/src/flutter/shell/common/skia_event_tracer_impl.cc +++ b/engine/src/flutter/shell/common/skia_event_tracer_impl.cc @@ -85,7 +85,7 @@ class FlutterEventTracer : public SkEventTracer { SkEventTracer::Handle handle) override { // This is only ever called from a scoped trace event so we will just end // the section. -#if defined(OS_FUCHSIA) && !defined(FUCHSIA_SDK) +#if defined(OS_FUCHSIA) TRACE_DURATION_END(kSkiaTag, name); #else fml::tracing::TraceEventEnd(name); diff --git a/engine/src/flutter/shell/platform/fuchsia/dart_runner/BUILD.gn b/engine/src/flutter/shell/platform/fuchsia/dart_runner/BUILD.gn index 80e4b7f1b7a..b67f50aff82 100644 --- a/engine/src/flutter/shell/platform/fuchsia/dart_runner/BUILD.gn +++ b/engine/src/flutter/shell/platform/fuchsia/dart_runner/BUILD.gn @@ -58,6 +58,8 @@ template("runner") { "$fuchsia_sdk_root/pkg:async-loop-cpp", "$fuchsia_sdk_root/pkg:fidl_cpp", "$fuchsia_sdk_root/pkg:syslog", + "$fuchsia_sdk_root/pkg:trace", + "$fuchsia_sdk_root/pkg:trace-provider-so", "$fuchsia_sdk_root/pkg/lib/sys/cpp", "$fuchsia_sdk_root/pkg/lib/vfs/cpp", "//third_party/tonic", diff --git a/engine/src/flutter/shell/platform/fuchsia/dart_runner/main.cc b/engine/src/flutter/shell/platform/fuchsia/dart_runner/main.cc index 6390507e5da..05388888e4a 100644 --- a/engine/src/flutter/shell/platform/fuchsia/dart_runner/main.cc +++ b/engine/src/flutter/shell/platform/fuchsia/dart_runner/main.cc @@ -3,6 +3,8 @@ // found in the LICENSE file. #include +#include +#include #include "dart_runner.h" #include "flutter/fml/logging.h" @@ -14,8 +16,6 @@ #if !defined(FUCHSIA_SDK) #include -#include -#include #endif // !defined(FUCHSIA_SDK) #if !defined(DART_PRODUCT) diff --git a/engine/src/flutter/shell/platform/fuchsia/dart_runner/mapped_resource.cc b/engine/src/flutter/shell/platform/fuchsia/dart_runner/mapped_resource.cc index 6b9ba3ed21b..cd17bb177a0 100644 --- a/engine/src/flutter/shell/platform/fuchsia/dart_runner/mapped_resource.cc +++ b/engine/src/flutter/shell/platform/fuchsia/dart_runner/mapped_resource.cc @@ -5,14 +5,11 @@ #include "mapped_resource.h" #include +#include #include #include #include -#if !defined(FUCHSIA_SDK) -#include -#endif // !defined(FUCHSIA_SDK) - #include "flutter/fml/logging.h" #include "logging.h" #include "runtime/dart/utils/inlines.h" @@ -24,9 +21,7 @@ bool MappedResource::LoadFromNamespace(fdio_ns_t* namespc, const std::string& path, MappedResource& resource, bool executable) { -#if !defined(FUCHSIA_SDK) TRACE_DURATION("dart", "LoadFromNamespace", "path", path); -#endif // !defined(FUCHSIA_SDK) // openat of a path with a leading '/' ignores the namespace fd. dart_utils::Check(path[0] != '/', LOG_TAG); diff --git a/engine/src/flutter/shell/platform/fuchsia/flutter/BUILD.gn b/engine/src/flutter/shell/platform/fuchsia/flutter/BUILD.gn index c0ee0e0ab5b..5771db5fc5e 100644 --- a/engine/src/flutter/shell/platform/fuchsia/flutter/BUILD.gn +++ b/engine/src/flutter/shell/platform/fuchsia/flutter/BUILD.gn @@ -122,6 +122,9 @@ template("flutter_runner") { "$fuchsia_sdk_root/pkg:fidl_cpp", "$fuchsia_sdk_root/pkg:scenic_cpp", "$fuchsia_sdk_root/pkg:syslog", + "$fuchsia_sdk_root/pkg:trace", + "$fuchsia_sdk_root/pkg:trace-engine", + "$fuchsia_sdk_root/pkg:trace-provider-so", "$fuchsia_sdk_root/pkg:zx", "$fuchsia_sdk_root/pkg/lib/sys/cpp", "$fuchsia_sdk_root/pkg/lib/vfs/cpp", diff --git a/engine/src/flutter/shell/platform/fuchsia/flutter/main.cc b/engine/src/flutter/shell/platform/fuchsia/flutter/main.cc index 354e869d4af..28853d36280 100644 --- a/engine/src/flutter/shell/platform/fuchsia/flutter/main.cc +++ b/engine/src/flutter/shell/platform/fuchsia/flutter/main.cc @@ -3,11 +3,8 @@ // found in the LICENSE file. #include - -#if !defined(FUCHSIA_SDK) -#include -#include -#endif +#include +#include #include @@ -18,7 +15,6 @@ int main(int argc, char const* argv[]) { std::unique_ptr loop(flutter_runner::MakeObservableLoop(true)); -#if !defined(FUCHSIA_SDK) std::unique_ptr provider; { TRACE_DURATION("flutter", "CreateTraceProvider"); @@ -27,7 +23,6 @@ int main(int argc, char const* argv[]) { trace::TraceProviderWithFdio::CreateSynchronously( loop->dispatcher(), "flutter_runner", &provider, &already_started); } -#endif // Set up the process-wide /tmp memfs. dart_utils::SetupRunnerTemp(); diff --git a/engine/src/flutter/shell/platform/fuchsia/flutter/platform_view.cc b/engine/src/flutter/shell/platform/fuchsia/flutter/platform_view.cc index 0a024e2b745..ddfedd30dc1 100644 --- a/engine/src/flutter/shell/platform/fuchsia/flutter/platform_view.cc +++ b/engine/src/flutter/shell/platform/fuchsia/flutter/platform_view.cc @@ -413,7 +413,6 @@ static flutter::PointerData::DeviceKind GetKindFromPointerType( } } -#if !defined(FUCHSIA_SDK) // TODO(SCN-1278): Remove this. // Turns two floats (high bits, low bits) into a 64-bit uint. static trace_flow_id_t PointerTraceHACK(float fa, float fb) { @@ -422,18 +421,15 @@ static trace_flow_id_t PointerTraceHACK(float fa, float fb) { memcpy(&ib, &fb, sizeof(uint32_t)); return (((uint64_t)ia) << 32) | ib; } -#endif // !defined(FUCHSIA_SDK) bool PlatformView::OnHandlePointerEvent( const fuchsia::ui::input::PointerEvent& pointer) { TRACE_EVENT0("flutter", "PlatformView::OnHandlePointerEvent"); -#if !defined(FUCHSIA_SDK) // TODO(SCN-1278): Use proper trace_id for tracing flow. trace_flow_id_t trace_id = PointerTraceHACK(pointer.radius_major, pointer.radius_minor); TRACE_FLOW_END("input", "dispatch_event_to_client", trace_id); -#endif // !defined(FUCHSIA_SDK) flutter::PointerData pointer_data; pointer_data.Clear(); diff --git a/engine/src/flutter/shell/platform/fuchsia/flutter/runner.cc b/engine/src/flutter/shell/platform/fuchsia/flutter/runner.cc index 8ad10e72e52..97b9b8c7d04 100644 --- a/engine/src/flutter/shell/platform/fuchsia/flutter/runner.cc +++ b/engine/src/flutter/shell/platform/fuchsia/flutter/runner.cc @@ -6,13 +6,10 @@ #include #include +#include #include #include -#if !defined(FUCHSIA_SDK) -#include -#endif // !defined(FUCHSIA_SDK) - #include #include @@ -101,9 +98,7 @@ Runner::Runner(async::Loop* loop) dart_utils::VMServiceObject::kPortDirName, std::make_unique()); -#if !defined(FUCHSIA_SDK) SetupTraceObserver(); -#endif // !defined(FUCHSIA_SDK) #endif // !defined(DART_PRODUCT) SkGraphics::Init(); @@ -122,9 +117,7 @@ Runner::~Runner() { runner_context_->RemovePublicService(); #if !defined(DART_PRODUCT) -#if !defined(FUCHSIA_SDK) trace_observer_->Stop(); -#endif // !defined(FUCHSIA_SDK) #endif // !defined(DART_PRODUCT) } @@ -213,7 +206,6 @@ void Runner::SetupICU() { } #if !defined(DART_PRODUCT) -#if !defined(FUCHSIA_SDK) void Runner::SetupTraceObserver() { trace_observer_ = std::make_unique(); trace_observer_->Start(loop_->dispatcher(), [runner = this]() { @@ -237,7 +229,6 @@ void Runner::SetupTraceObserver() { } }); } -#endif // !defined(FUCHSIA_SDK) #endif // !defined(DART_PRODUCT) } // namespace flutter_runner diff --git a/engine/src/flutter/shell/platform/fuchsia/flutter/runner.h b/engine/src/flutter/shell/platform/fuchsia/flutter/runner.h index 0f29a39f444..bb5573e4422 100644 --- a/engine/src/flutter/shell/platform/fuchsia/flutter/runner.h +++ b/engine/src/flutter/shell/platform/fuchsia/flutter/runner.h @@ -11,11 +11,8 @@ #include #include #include - -#if !defined(FUCHSIA_SDK) -#include -#include -#endif // !defined(FUCHSIA_SDK) +#include +#include #include "component.h" #include "flutter/fml/macros.h" @@ -57,10 +54,8 @@ class Runner final : public fuchsia::sys::Runner { // The connection between the Dart VM service and The Hub. std::unique_ptr vmservice_object_; -#if !defined(FUCHSIA_SDK) std::unique_ptr trace_observer_; trace_prolonged_context_t* prolonged_context_; -#endif // !defined(FUCHSIA_SDK) #endif // !defined(DART_PRODUCT) // |fuchsia::sys::Runner| diff --git a/engine/src/flutter/shell/testing/BUILD.gn b/engine/src/flutter/shell/testing/BUILD.gn index 9e741ef165d..9daca896faf 100644 --- a/engine/src/flutter/shell/testing/BUILD.gn +++ b/engine/src/flutter/shell/testing/BUILD.gn @@ -31,10 +31,6 @@ executable("testing") { ] if (is_fuchsia && !is_fuchsia_sdk) { - deps += [ - "//garnet/public/lib/ui/scenic:client", - "//zircon/public/lib/trace", - "//zircon/public/lib/trace-provider", - ] + deps += [ "//garnet/public/lib/ui/scenic:client" ] } }