Merge "TP: add enable_perfetto_trace_processor_mac_instruments" into …

…main
google · Sep 2, 2024 · 2aa4908 · 2aa4908
2 parents 5744572 + 1e42eca
commit 2aa4908
Show file tree

Hide file tree

Showing 12 changed files with 125 additions and 50 deletions.
diff --git a/BUILD b/BUILD
@@ -1719,6 +1719,7 @@ perfetto_filegroup(
 perfetto_filegroup(
     name = "src_trace_processor_importers_instruments_instruments",
     srcs = [
+        "src/trace_processor/importers/instruments/instruments_utils.h",
         "src/trace_processor/importers/instruments/instruments_xml_tokenizer.cc",
         "src/trace_processor/importers/instruments/instruments_xml_tokenizer.h",
         "src/trace_processor/importers/instruments/row_data_tracker.cc",

diff --git a/buildtools/BUILD.gn b/buildtools/BUILD.gn
@@ -1402,39 +1402,41 @@ source_set("jsoncpp") {
   deps = [ "//gn:default_deps" ]
 }
 
-config("expat_public_config") {
-  defines = [ "XML_STATIC" ]
-  cflags = [
-    # Using -isystem instead of include_dirs (-I), so we don't need to suppress
-    # warnings coming from third-party headers. Doing so would mask warnings in
-    # our own code.
-    perfetto_isystem_cflag,
-    rebase_path("expat/src/expat/lib", root_build_dir),
-    perfetto_isystem_cflag,
-    rebase_path("expat/include", root_build_dir),
-  ]
-}
-
 config("no_format_warning") {
   cflags = [ "-Wno-format" ]
 }
 
-source_set("expat") {
-  sources = [
-    "expat/src/expat/lib/expat.h",
-    "expat/src/expat/lib/xmlparse.c",
-    "expat/src/expat/lib/xmlrole.c",
-    "expat/src/expat/lib/xmltok.c",
-  ]
+if (enable_perfetto_trace_processor_mac_instruments) {
+  config("expat_public_config") {
+    defines = [ "XML_STATIC" ]
+    cflags = [
+      # Using -isystem instead of include_dirs (-I), so we don't need to
+      # suppress warnings coming from third-party headers. Doing so would mask
+      # warnings in our own code.
+      perfetto_isystem_cflag,
+      rebase_path("expat/src/expat/lib", root_build_dir),
+      perfetto_isystem_cflag,
+      rebase_path("expat/include", root_build_dir),
+    ]
+  }
 
-  public_configs = [ ":expat_public_config" ]
-  configs -= [ "//gn/standalone:extra_warnings" ]
-  configs += [ ":no_format_warning" ]
+  source_set("expat") {
+    sources = [
+      "expat/src/expat/lib/expat.h",
+      "expat/src/expat/lib/xmlparse.c",
+      "expat/src/expat/lib/xmlrole.c",
+      "expat/src/expat/lib/xmltok.c",
+    ]
 
-  defines = [
-    "_LIB",
-    "HAVE_EXPAT_CONFIG_H",
-  ]
+    public_configs = [ ":expat_public_config" ]
+    configs -= [ "//gn/standalone:extra_warnings" ]
+    configs += [ ":no_format_warning" ]
+
+    defines = [
+      "_LIB",
+      "HAVE_EXPAT_CONFIG_H",
+    ]
+  }
 }
 
 config("linenoise_config") {

diff --git a/gn/BUILD.gn b/gn/BUILD.gn
@@ -87,6 +87,7 @@ action("gen_buildflags") {
     "PERFETTO_TP_LINENOISE=$enable_perfetto_trace_processor_linenoise",
     "PERFETTO_TP_HTTPD=$enable_perfetto_trace_processor_httpd",
     "PERFETTO_TP_JSON=$enable_perfetto_trace_processor_json",
+    "PERFETTO_TP_INSTRUMENTS=$enable_perfetto_trace_processor_mac_instruments",
     "PERFETTO_LOCAL_SYMBOLIZER=$perfetto_local_symbolizer",
     "PERFETTO_ZLIB=$enable_perfetto_zlib",
     "PERFETTO_TRACED_PERF=$enable_perfetto_traced_perf",
@@ -363,11 +364,13 @@ config("sqlite_third_party_include_path") {
   }
 }
 
-group("expat") {
-  if (perfetto_root_path == "//") {
-    public_deps = [ "//buildtools:expat" ]
-  } else {
-    public_deps = [ "//third_party/expat:expat" ]
+if (enable_perfetto_trace_processor_mac_instruments) {
+  group("expat") {
+    if (perfetto_root_path == "//") {
+      public_deps = [ "//buildtools:expat" ]
+    } else {
+      public_deps = [ "//third_party/expat:expat" ]
+    }
   }
 }
 

diff --git a/gn/perfetto.gni b/gn/perfetto.gni
@@ -298,6 +298,14 @@ declare_args() {
   enable_perfetto_trace_processor_json =
       enable_perfetto_trace_processor && !perfetto_build_with_android
 
+  # Enables the support for importing profiles from the MacOS Instruments app.
+  # Requires a dependency on libexpat for XML parsing.
+  # Disabled in chromium due to some fuzzer related build failure (b/363347029).
+  enable_perfetto_trace_processor_mac_instruments =
+      enable_perfetto_trace_processor &&
+      (perfetto_build_standalone || perfetto_build_with_android ||
+       is_perfetto_build_generator)
+
   # Enables httpd RPC support in the trace processor.
   # Further per-OS conditionals are applied in gn/BUILD.gn.
   # Chromium+Win: httpd support depends on enable_perfetto_ipc, which is not

diff --git a/include/perfetto/base/build_configs/android_tree/perfetto_build_flags.h b/include/perfetto/base/build_configs/android_tree/perfetto_build_flags.h
@@ -38,6 +38,7 @@
 #define PERFETTO_BUILDFLAG_DEFINE_PERFETTO_TP_LINENOISE() (0)
 #define PERFETTO_BUILDFLAG_DEFINE_PERFETTO_TP_HTTPD() (1)
 #define PERFETTO_BUILDFLAG_DEFINE_PERFETTO_TP_JSON() (0)
+#define PERFETTO_BUILDFLAG_DEFINE_PERFETTO_TP_INSTRUMENTS() (1)
 #define PERFETTO_BUILDFLAG_DEFINE_PERFETTO_LOCAL_SYMBOLIZER() (PERFETTO_BUILDFLAG_DEFINE_PERFETTO_OS_LINUX() || PERFETTO_BUILDFLAG_DEFINE_PERFETTO_OS_MAC() ||PERFETTO_BUILDFLAG_DEFINE_PERFETTO_OS_WIN())
 #define PERFETTO_BUILDFLAG_DEFINE_PERFETTO_ZLIB() (1)
 #define PERFETTO_BUILDFLAG_DEFINE_PERFETTO_TRACED_PERF() (1)

diff --git a/include/perfetto/base/build_configs/bazel/perfetto_build_flags.h b/include/perfetto/base/build_configs/bazel/perfetto_build_flags.h
@@ -38,6 +38,7 @@
 #define PERFETTO_BUILDFLAG_DEFINE_PERFETTO_TP_LINENOISE() (1)
 #define PERFETTO_BUILDFLAG_DEFINE_PERFETTO_TP_HTTPD() (1)
 #define PERFETTO_BUILDFLAG_DEFINE_PERFETTO_TP_JSON() (1)
+#define PERFETTO_BUILDFLAG_DEFINE_PERFETTO_TP_INSTRUMENTS() (1)
 #define PERFETTO_BUILDFLAG_DEFINE_PERFETTO_LOCAL_SYMBOLIZER() (PERFETTO_BUILDFLAG_DEFINE_PERFETTO_OS_LINUX() || PERFETTO_BUILDFLAG_DEFINE_PERFETTO_OS_MAC() ||PERFETTO_BUILDFLAG_DEFINE_PERFETTO_OS_WIN())
 #define PERFETTO_BUILDFLAG_DEFINE_PERFETTO_ZLIB() (1)
 #define PERFETTO_BUILDFLAG_DEFINE_PERFETTO_TRACED_PERF() (0)

diff --git a/src/trace_processor/importers/instruments/BUILD.gn b/src/trace_processor/importers/instruments/BUILD.gn
@@ -25,24 +25,31 @@ source_set("row") {
 
 source_set("instruments") {
   sources = [
-    "instruments_xml_tokenizer.cc",
+    "instruments_utils.h",
     "instruments_xml_tokenizer.h",
-    "row_data_tracker.cc",
-    "row_data_tracker.h",
-    "row_parser.cc",
     "row_parser.h",
   ]
-  public_deps = [ ":row" ]
   deps = [
     "../../../../gn:default_deps",
-    "../../../../gn:expat",
     "../../../../include/perfetto/ext/base:base",
-    "../../../../include/perfetto/public",
-    "../../../../include/perfetto/trace_processor:trace_processor",
-    "../../../../protos/perfetto/trace:zero",
-    "../../sorter",
-    "../../storage",
-    "../../types",
-    "../common:common",
   ]
+  if (enable_perfetto_trace_processor_mac_instruments) {
+    public_deps = [ ":row" ]
+    sources += [
+      "instruments_xml_tokenizer.cc",
+      "row_data_tracker.cc",
+      "row_data_tracker.h",
+      "row_parser.cc",
+    ]
+    deps += [
+      "../../../../gn:expat",
+      "../../../../include/perfetto/public",
+      "../../../../include/perfetto/trace_processor:trace_processor",
+      "../../../../protos/perfetto/trace:zero",
+      "../../sorter",
+      "../../storage",
+      "../../types",
+      "../common:common",
+    ]
+  }
 }
diff --git a/src/trace_processor/importers/instruments/instruments_utils.h b/src/trace_processor/importers/instruments/instruments_utils.h
@@ -0,0 +1,34 @@
+/*
+ * Copyright (C) 2024 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef SRC_TRACE_PROCESSOR_IMPORTERS_INSTRUMENTS_INSTRUMENTS_UTILS_H_
+#define SRC_TRACE_PROCESSOR_IMPORTERS_INSTRUMENTS_INSTRUMENTS_UTILS_H_
+
+#include "perfetto/base/build_config.h"
+
+namespace perfetto::trace_processor::instruments_importer {
+
+inline bool IsInstrumentsSupported() {
+#if PERFETTO_BUILDFLAG(PERFETTO_TP_INSTRUMENTS)
+  return true;
+#else
+  return false;
+#endif
+}
+
+}  // namespace perfetto::trace_processor::instruments_importer
+
+#endif  // SRC_TRACE_PROCESSOR_IMPORTERS_INSTRUMENTS_INSTRUMENTS_UTILS_H_
diff --git a/src/trace_processor/importers/instruments/instruments_xml_tokenizer.cc b/src/trace_processor/importers/instruments/instruments_xml_tokenizer.cc
@@ -31,6 +31,11 @@
 #include "src/trace_processor/importers/instruments/row_data_tracker.h"
 #include "src/trace_processor/sorter/trace_sorter.h"
 
+#if !PERFETTO_BUILDFLAG(PERFETTO_TP_INSTRUMENTS)
+#error \
+    "This file should not be built when enable_perfetto_trace_processor_mac_instruments=false"
+#endif
+
 namespace perfetto::trace_processor::instruments_importer {
 
 namespace {

diff --git a/src/trace_processor/importers/instruments/row_data_tracker.cc b/src/trace_processor/importers/instruments/row_data_tracker.cc
@@ -18,6 +18,11 @@
 
 #include "perfetto/base/status.h"
 
+#if !PERFETTO_BUILDFLAG(PERFETTO_TP_INSTRUMENTS)
+#error \
+    "This file should not be built when enable_perfetto_trace_processor_mac_instruments=false"
+#endif
+
 namespace perfetto::trace_processor::instruments_importer {
 
 RowDataTracker::RowDataTracker() {}

diff --git a/src/trace_processor/importers/instruments/row_parser.cc b/src/trace_processor/importers/instruments/row_parser.cc
@@ -24,6 +24,11 @@
 #include "src/trace_processor/importers/instruments/row.h"
 #include "src/trace_processor/importers/instruments/row_data_tracker.h"
 
+#if !PERFETTO_BUILDFLAG(PERFETTO_TP_INSTRUMENTS)
+#error \
+    "This file should not be built when enable_perfetto_trace_processor_mac_instruments=false"
+#endif
+
 namespace perfetto::trace_processor::instruments_importer {
 
 RowParser::RowParser(TraceProcessorContext* context)

diff --git a/src/trace_processor/trace_processor_impl.cc b/src/trace_processor/trace_processor_impl.cc
@@ -51,6 +51,7 @@
 #include "src/trace_processor/importers/fuchsia/fuchsia_trace_parser.h"
 #include "src/trace_processor/importers/fuchsia/fuchsia_trace_tokenizer.h"
 #include "src/trace_processor/importers/gzip/gzip_trace_parser.h"
+#include "src/trace_processor/importers/instruments/instruments_utils.h"
 #include "src/trace_processor/importers/instruments/instruments_xml_tokenizer.h"
 #include "src/trace_processor/importers/instruments/row_parser.h"
 #include "src/trace_processor/importers/json/json_trace_parser_impl.h"
@@ -400,11 +401,13 @@ TraceProcessorImpl::TraceProcessorImpl(const Config& cfg)
   context_.perf_record_parser =
       std::make_unique<perf_importer::RecordParser>(&context_);
 
-  context_.reader_registry
-      ->RegisterTraceReader<instruments_importer::InstrumentsXmlTokenizer>(
-          kInstrumentsXmlTraceType);
-  context_.instruments_row_parser =
-      std::make_unique<instruments_importer::RowParser>(&context_);
+  if (instruments_importer::IsInstrumentsSupported()) {
+    context_.reader_registry
+        ->RegisterTraceReader<instruments_importer::InstrumentsXmlTokenizer>(
+            kInstrumentsXmlTraceType);
+    context_.instruments_row_parser =
+        std::make_unique<instruments_importer::RowParser>(&context_);
+  }
 
   if (util::IsGzipSupported()) {
     context_.reader_registry->RegisterTraceReader<GzipTraceParser>(