From 293ef51fc635e9b3f44de6ff7d25e6397b9bbee1 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Fri, 8 Oct 2021 15:55:50 -0700 Subject: [PATCH] Reland Android systrace (flutter/engine#29080) * Reland "Use the systrace recorder if systracing is enabled at startup, and enable systracing in release mode on Android (#28903)" (#29071)" This reverts commit a7660964b41f64991369341364a43c93317a4a51. * More logcat * more logs * Remove wait * Avoid plugin registrar exception * DEFAULT instead of LAUNCHER * use am instead of monkey * Update android_systrace_test.py --- DEPS | 11 ++ engine/src/flutter/fml/BUILD.gn | 6 +- engine/src/flutter/fml/native_library.h | 22 +++- .../platform/posix/native_library_posix.cc | 8 +- .../fml/platform/win/native_library_win.cc | 4 +- engine/src/flutter/fml/trace_event.h | 2 +- engine/src/flutter/runtime/dart_vm.cc | 10 +- .../shell/platform/android/flutter_main.cc | 30 +++++ .../flutter/testing/android_systrace_test.py | 113 ++++++++++++++++++ engine/src/flutter/testing/run_tests.py | 9 ++ .../scenarios/TestableFlutterActivity.java | 3 +- 11 files changed, 197 insertions(+), 21 deletions(-) create mode 100755 engine/src/flutter/testing/android_systrace_test.py diff --git a/DEPS b/DEPS index 21bedc808ad..02eea527425 100644 --- a/DEPS +++ b/DEPS @@ -468,6 +468,17 @@ deps = { 'dep_type': 'cipd' }, + 'src/third_party/android_tools/trace_to_text': { + 'packages': [ + { + 'version': 'git_tag:v20.1', + 'package': 'perfetto/trace_to_text/${{platform}}' + } + ], + 'condition': 'download_android_deps', + 'dep_type': 'cipd' + }, + 'src/third_party/android_tools/ndk': { 'packages': [ { diff --git a/engine/src/flutter/fml/BUILD.gn b/engine/src/flutter/fml/BUILD.gn index ceb484d7201..430bba11010 100644 --- a/engine/src/flutter/fml/BUILD.gn +++ b/engine/src/flutter/fml/BUILD.gn @@ -217,10 +217,8 @@ source_set("fml") { "platform/win/wstring_conversion.h", ] - if (is_win) { - # For wstring_conversion. See issue #50053. - defines = [ "_SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING" ] - } + # For wstring_conversion. See issue #50053. + defines = [ "_SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING" ] } else { sources += [ "platform/posix/file_posix.cc", diff --git a/engine/src/flutter/fml/native_library.h b/engine/src/flutter/fml/native_library.h index 793be5f5add..a6174b9eb60 100644 --- a/engine/src/flutter/fml/native_library.h +++ b/engine/src/flutter/fml/native_library.h @@ -5,6 +5,8 @@ #ifndef FLUTTER_FML_NATIVE_LIBRARY_H_ #define FLUTTER_FML_NATIVE_LIBRARY_H_ +#include + #include "flutter/fml/build_config.h" #include "flutter/fml/macros.h" #include "flutter/fml/memory/ref_counted.h" @@ -19,8 +21,10 @@ class NativeLibrary : public fml::RefCountedThreadSafe { public: #if OS_WIN using Handle = HMODULE; + using SymbolHandle = FARPROC; #else // OS_WIN using Handle = void*; + using SymbolHandle = void*; #endif // OS_WIN static fml::RefPtr Create(const char* path); @@ -31,7 +35,22 @@ class NativeLibrary : public fml::RefCountedThreadSafe { static fml::RefPtr CreateForCurrentProcess(); - const uint8_t* ResolveSymbol(const char* symbol); + template + const std::optional ResolveFunction(const char* symbol) { + auto* resolved_symbol = Resolve(symbol); + if (!resolved_symbol) { + return std::nullopt; + } + return std::optional(reinterpret_cast(resolved_symbol)); + } + + const uint8_t* ResolveSymbol(const char* symbol) { + auto* resolved_symbol = reinterpret_cast(Resolve(symbol)); + if (resolved_symbol == nullptr) { + FML_DLOG(INFO) << "Could not resolve symbol in library: " << symbol; + } + return resolved_symbol; + } private: Handle handle_ = nullptr; @@ -44,6 +63,7 @@ class NativeLibrary : public fml::RefCountedThreadSafe { ~NativeLibrary(); Handle GetHandle() const; + SymbolHandle Resolve(const char* symbol) const; FML_DISALLOW_COPY_AND_ASSIGN(NativeLibrary); FML_FRIEND_REF_COUNTED_THREAD_SAFE(NativeLibrary); diff --git a/engine/src/flutter/fml/platform/posix/native_library_posix.cc b/engine/src/flutter/fml/platform/posix/native_library_posix.cc index 693cc1c5cbf..43406bc21b5 100644 --- a/engine/src/flutter/fml/platform/posix/native_library_posix.cc +++ b/engine/src/flutter/fml/platform/posix/native_library_posix.cc @@ -57,12 +57,8 @@ fml::RefPtr NativeLibrary::CreateForCurrentProcess() { return fml::AdoptRef(new NativeLibrary(RTLD_DEFAULT, false)); } -const uint8_t* NativeLibrary::ResolveSymbol(const char* symbol) { - auto* resolved_symbol = static_cast(::dlsym(handle_, symbol)); - if (resolved_symbol == nullptr) { - FML_DLOG(INFO) << "Could not resolve symbol in library: " << symbol; - } - return resolved_symbol; +NativeLibrary::SymbolHandle NativeLibrary::Resolve(const char* symbol) const { + return ::dlsym(handle_, symbol); } } // namespace fml diff --git a/engine/src/flutter/fml/platform/win/native_library_win.cc b/engine/src/flutter/fml/platform/win/native_library_win.cc index 046c3237a91..27a88006611 100644 --- a/engine/src/flutter/fml/platform/win/native_library_win.cc +++ b/engine/src/flutter/fml/platform/win/native_library_win.cc @@ -49,11 +49,11 @@ fml::RefPtr NativeLibrary::CreateForCurrentProcess() { return fml::AdoptRef(new NativeLibrary(::GetModuleHandle(nullptr), false)); } -const uint8_t* NativeLibrary::ResolveSymbol(const char* symbol) { +NativeLibrary::SymbolHandle NativeLibrary::Resolve(const char* symbol) const { if (symbol == nullptr || handle_ == nullptr) { return nullptr; } - return reinterpret_cast(::GetProcAddress(handle_, symbol)); + return ::GetProcAddress(handle_, symbol); } } // namespace fml diff --git a/engine/src/flutter/fml/trace_event.h b/engine/src/flutter/fml/trace_event.h index a030cce117e..9e1d0609e7c 100644 --- a/engine/src/flutter/fml/trace_event.h +++ b/engine/src/flutter/fml/trace_event.h @@ -47,7 +47,7 @@ #include "flutter/fml/time/time_point.h" #include "third_party/dart/runtime/include/dart_tools_api.h" -#if (FLUTTER_RELEASE && !defined(OS_FUCHSIA)) +#if (FLUTTER_RELEASE && !defined(OS_FUCHSIA) && !defined(OS_ANDROID)) #define FLUTTER_TIMELINE_ENABLED 0 #else #define FLUTTER_TIMELINE_ENABLED 1 diff --git a/engine/src/flutter/runtime/dart_vm.cc b/engine/src/flutter/runtime/dart_vm.cc index 574b0eae652..a2f400bb81e 100644 --- a/engine/src/flutter/runtime/dart_vm.cc +++ b/engine/src/flutter/runtime/dart_vm.cc @@ -97,11 +97,8 @@ static const char* kDartEndlessTraceBufferArgs[]{ "--timeline_recorder=endless", }; -static const char* kDartSystraceTraceBufferArgs[]{ - "--timeline_recorder=systrace", -}; - -static const char* kDartFuchsiaTraceArgs[] FML_ALLOW_UNUSED_TYPE = { +// This is the same as --timeline_recorder=systrace. +static const char* kDartSystraceTraceBufferArgs[] = { "--systrace_timeline", }; @@ -386,7 +383,8 @@ DartVM::DartVM(std::shared_ptr vm_data, } #if defined(OS_FUCHSIA) - PushBackAll(&args, kDartFuchsiaTraceArgs, fml::size(kDartFuchsiaTraceArgs)); + PushBackAll(&args, kDartSystraceTraceBufferArgs, + fml::size(kDartSystraceTraceBufferArgs)); PushBackAll(&args, kDartSystraceTraceStreamsArgs, fml::size(kDartSystraceTraceStreamsArgs)); #else diff --git a/engine/src/flutter/shell/platform/android/flutter_main.cc b/engine/src/flutter/shell/platform/android/flutter_main.cc index 35cf6594881..6bfc7c81fbb 100644 --- a/engine/src/flutter/shell/platform/android/flutter_main.cc +++ b/engine/src/flutter/shell/platform/android/flutter_main.cc @@ -8,12 +8,14 @@ #include +#include #include #include "flutter/fml/command_line.h" #include "flutter/fml/file.h" #include "flutter/fml/macros.h" #include "flutter/fml/message_loop.h" +#include "flutter/fml/native_library.h" #include "flutter/fml/paths.h" #include "flutter/fml/platform/android/jni_util.h" #include "flutter/fml/platform/android/paths_android.h" @@ -37,6 +39,21 @@ extern const intptr_t kPlatformStrongDillSize; namespace { +// This is only available on API 23+, so dynamically look it up. +// This method is only called once at shell creation. +// Do this in C++ because the API is available at level 23 here, but only 29+ in +// Java. +bool IsATraceEnabled() { + auto libandroid = fml::NativeLibrary::Create("libandroid.so"); + FML_CHECK(libandroid); + auto atrace_fn = + libandroid->ResolveFunction("ATrace_isEnabled"); + if (atrace_fn) { + return atrace_fn.value()(); + } + return false; +} + fml::jni::ScopedJavaGlobalRef* g_flutter_jni_class = nullptr; } // anonymous namespace @@ -75,6 +92,19 @@ void FlutterMain::Init(JNIEnv* env, auto settings = SettingsFromCommandLine(command_line); + // Turn systracing on if ATrace_isEnabled is true and the user did not already + // request systracing + if (!settings.trace_systrace) { + settings.trace_systrace = IsATraceEnabled(); + if (settings.trace_systrace) { + __android_log_print( + ANDROID_LOG_INFO, "Flutter", + "ATrace was enabled at startup. Flutter and Dart " + "tracing will be forwarded to systrace and will not show up in the " + "Observatory timeline or Dart DevTools."); + } + } + int64_t init_time_micros = initTimeMillis * 1000; settings.engine_start_timestamp = std::chrono::microseconds(Dart_TimelineGetMicros() - init_time_micros); diff --git a/engine/src/flutter/testing/android_systrace_test.py b/engine/src/flutter/testing/android_systrace_test.py new file mode 100755 index 00000000000..040de9ede28 --- /dev/null +++ b/engine/src/flutter/testing/android_systrace_test.py @@ -0,0 +1,113 @@ +#!/usr/bin/env python3 +# +# Copyright 2013 The Flutter Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +import argparse +import os +import subprocess +import sys + +BUILDROOT_DIR = os.path.abspath(os.path.join(os.path.realpath(__file__), '..', '..', '..')) + +PERFETTO_SESSION_KEY = 'session1' +PERFETTO_TRACE_FILE = '/data/misc/perfetto-traces/trace' +PERFETTO_CONFIG = ''' +write_into_file: true +file_write_period_ms: 1000000000 +flush_period_ms: 1000 + +buffers: { + size_kb: 129024 +} +data_sources: { + config { + name: "linux.ftrace" + ftrace_config { + ftrace_events: "ftrace/print" + atrace_apps: "%s" + } + } +} +''' + +def InstallApk(apk_path, package_name, adb_path='adb'): + print('Installing APK') + subprocess.check_output([adb_path, 'shell', 'am', 'force-stop', package_name]) + # Allowed to fail if APK was never installed. + subprocess.call([adb_path, 'uninstall', package_name], stdout=subprocess.DEVNULL) + subprocess.check_output([adb_path, 'install', apk_path]) + + +def StartPerfetto(package_name, adb_path='adb'): + print('Starting trace') + cmd = [adb_path, 'shell', 'echo' , "'" + PERFETTO_CONFIG % package_name + "'", '|', + 'perfetto', '-c', '-', '--txt', '-o', PERFETTO_TRACE_FILE, + '--detach', PERFETTO_SESSION_KEY] + + subprocess.check_output(cmd, stderr=subprocess.STDOUT) + + +def LaunchPackage(package_name, activity_name, adb_path='adb'): + print('Scanning logcat') + subprocess.check_output([adb_path, 'logcat', '-c'], stderr=subprocess.STDOUT) + logcat = subprocess.Popen([adb_path, 'logcat'], stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, universal_newlines=True) + + print('Launching %s (%s)' % (package_name, activity_name)) + subprocess.check_output( + [adb_path, 'shell', 'am ', 'start', '-n', + '%s/%s' % (package_name, activity_name)], stderr=subprocess.STDOUT) + for line in logcat.stdout: + print('>>>>>>>> ' + line.strip()) + if 'Observatory listening' in line: + logcat.kill() + break + + +def CollectAndValidateTrace(adb_path = 'adb'): + print('Fetching trace') + subprocess.check_output([adb_path, 'shell', 'perfetto', '--attach', + PERFETTO_SESSION_KEY, '--stop'], stderr=subprocess.STDOUT) + subprocess.check_output([adb_path, 'pull', PERFETTO_TRACE_FILE, 'trace.pb'], stderr=subprocess.STDOUT) + + print('Validating trace') + traceconv = os.path.join(BUILDROOT_DIR, 'third_party', + 'android_tools', 'trace_to_text', 'trace_to_text') + traceconv_output = subprocess.check_output([traceconv, 'systrace', 'trace.pb'], stderr=subprocess.STDOUT, universal_newlines=True) + + print('Trace output:') + print(traceconv_output) + + if 'ShellSetupUISubsystem' in traceconv_output: + return 0 + + print('Trace did not contain ShellSetupUISubsystem, failing.') + return 1 + + +def main(): + parser = argparse.ArgumentParser() + + parser.add_argument('--apk-path', dest='apk_path', action='store', + help='Provide the path to the APK to install') + parser.add_argument('--package-name', dest='package_name', action='store', + help='The package name of the APK, e.g. dev.flutter.scenarios') + parser.add_argument('--activity-name', dest='activity_name', action='store', + help='The activity to launch as it appears in AndroidManifest.xml, ' + 'e.g. .TextPlatformViewActivity') + parser.add_argument('--adb-path', dest='adb_path', action='store', + default='adb', help='Provide the path of adb used for android tests. ' + 'By default it looks on $PATH.') + + args = parser.parse_args() + + InstallApk(args.apk_path, args.package_name, args.adb_path) + StartPerfetto(args.package_name, args.adb_path) + LaunchPackage(args.package_name, args.activity_name, args.adb_path) + return CollectAndValidateTrace(args.adb_path) + + +if __name__ == '__main__': + sys.exit(main()) diff --git a/engine/src/flutter/testing/run_tests.py b/engine/src/flutter/testing/run_tests.py index 721e7bff445..1c12894c0de 100755 --- a/engine/src/flutter/testing/run_tests.py +++ b/engine/src/flutter/testing/run_tests.py @@ -361,6 +361,15 @@ def RunAndroidTests(android_variant='android_debug_unopt', adb_path=None): RunCmd([adb_path, 'push', tests_path, remote_path], cwd=buildroot_dir) RunCmd([adb_path, 'shell', remote_tests_path]) + systrace_test = os.path.join(buildroot_dir, 'flutter', 'testing', + 'android_systrace_test.py') + scenario_apk = os.path.join(out_dir, android_variant, 'firebase_apks', + 'scenario_app.apk') + RunCmd([systrace_test, '--adb-path', adb_path, '--apk-path', scenario_apk, + '--package-name', 'dev.flutter.scenarios', + '--activity-name', '.TextPlatformViewActivity']) + + def RunObjcTests(ios_variant='ios_debug_sim_unopt', test_filter=None): """Runs Objective-C XCTest unit tests for the iOS embedding""" AssertExpectedXcodeVersion() diff --git a/engine/src/flutter/testing/scenario_app/android/app/src/main/java/dev/flutter/scenarios/TestableFlutterActivity.java b/engine/src/flutter/testing/scenario_app/android/app/src/main/java/dev/flutter/scenarios/TestableFlutterActivity.java index ac149637227..82ba2d95c5a 100644 --- a/engine/src/flutter/testing/scenario_app/android/app/src/main/java/dev/flutter/scenarios/TestableFlutterActivity.java +++ b/engine/src/flutter/testing/scenario_app/android/app/src/main/java/dev/flutter/scenarios/TestableFlutterActivity.java @@ -15,7 +15,8 @@ public abstract class TestableFlutterActivity extends FlutterActivity { @Override public void configureFlutterEngine(@NonNull FlutterEngine flutterEngine) { - super.configureFlutterEngine(flutterEngine); + // Do not call super. We have no plugins to register, and the automatic + // registration will fail and print a scary exception in the logs. flutterEngine .getDartExecutor() .setMessageHandler("take_screenshot", (byteBuffer, binaryReply) -> notifyFlutterRendered());