Merge "tp: Add RegisterSqlPackage() and start sunsetting RegisterSqlM…

…odule()" into main

CHANGELOG

@@ -6,9 +6,18 @@ Unreleased:
  * Increased watchdog timeout to 180s from 30s to make watchdog crashes
  much less likely when system is under heavy load.
  SQL Standard library:
- *
+ * Improved CPU cycles calculation in `linux.cpu.utilization` modules:
+ `process`, `system` and `thread` by fixing a bug responsible for too high
+ CPU cycles values.
+ * Introduces functions responsible for calculating CPU cycles with
+ breakdown by CPU, thread, process and slice for a given interval.
  Trace Processor:
- *
+ * Renamed Trace Processor's C++ method `RegisterSqlModule()` to
+ `RegisterSqlPackage()`, which better represents the module/package
+ relationship. Package is the top level grouping of modules, which are
+ objects being included with `INCLUDE PERFETTO MODULE`.
+ `RegisterSqlModule()` is still available and runs `RegisterSqlPackage()`.
+ `RegisterSqlModule()` will be deprecated in v50.0.
  UI:
  * Scheduling wakeup information now reflects whether the wakeup came
  from an interrupt context. The per-cpu scheduling tracks now show only

include/perfetto/trace_processor/basic_types.h

@@ -263,9 +263,9 @@ struct PERFETTO_EXPORT_COMPONENT SqlValue {
 };
 
 // Data used to register a new SQL package.
-struct SqlModule {
+struct SqlPackage {
  // Must be unique among modules, or can be used to override existing module if
- // |allow_module_override| is set.
+ // |allow_override| is set.
  std::string name;
 
  // Pairs of strings used for |INCLUDE| with the contents of SQL files being
@@ -276,10 +276,18 @@ struct SqlModule {
  // run, with slashes replaced by dots and without the SQL extension. For
  // example, 'android/camera/jank.sql' would be included by
  // 'android.camera.jank'.
- std::vector<std::pair<std::string, std::string>> files;
+ std::vector<std::pair<std::string, std::string>> modules;
 
  // If true, SqlPackage will override registered module with the same name. Can
  // only be set if enable_dev_features is true, otherwise will throw an error.
+ bool allow_override = false;
+};
+
+// Deprecated. Use only with |trace_processor->RegisterSqlModule()|. Alias of
+// |SqlPackage|.
+struct SqlModule {
+ std::string name;
+ std::vector<std::pair<std::string, std::string>> files;
  bool allow_module_override = false;
 };
 

include/perfetto/trace_processor/trace_processor.h

@@ -63,7 +63,7 @@ class PERFETTO_EXPORT_COMPONENT TraceProcessor : public TraceProcessorStorage {
  // PERFETTO MODULE camera.cpu.metrics". The first word of the string has to be
  // a package name and there can be only one package registered with a given
  // name.
- virtual base::Status RegisterSqlModule(SqlModule) = 0;
+ virtual base::Status RegisterSqlPackage(SqlPackage) = 0;
 
  // Registers a metric at the given path which will run the specified SQL.
  virtual base::Status RegisterMetric(const std::string& path,
@@ -138,6 +138,11 @@ class PERFETTO_EXPORT_COMPONENT TraceProcessor : public TraceProcessorStorage {
  // loaded by trace processor shell at runtime. The message is encoded as
  // DescriptorSet, defined in perfetto/trace_processor/trace_processor.proto.
  virtual std::vector<uint8_t> GetMetricDescriptors() = 0;
+
+ // Deprecated. Use |RegisterSqlPackage()| instead, which is identical in
+ // functionality to |RegisterSqlModule()| and the only difference is in
+ // the argument, which is directly translatable to |SqlPackage|.
+ virtual base::Status RegisterSqlModule(SqlModule) = 0;
 };
 
 } // namespace trace_processor

protos/perfetto/trace_processor/trace_processor.proto

@@ -88,21 +88,23 @@ message TraceProcessorRpc {
  optional string fatal_error = 5;
 
  enum TraceProcessorMethod {
+ reserved 4, 12;
+ reserved "TPM_QUERY_RAW_DEPRECATED";
+ reserved "TPM_REGISTER_SQL_MODULE";
+
  TPM_UNSPECIFIED = 0;
  TPM_APPEND_TRACE_DATA = 1;
  TPM_FINALIZE_TRACE_DATA = 2;
  TPM_QUERY_STREAMING = 3;
  // Previously: TPM_QUERY_RAW_DEPRECATED
- reserved 4;
- reserved "TPM_QUERY_RAW_DEPRECATED";
  TPM_COMPUTE_METRIC = 5;
  TPM_GET_METRIC_DESCRIPTORS = 6;
  TPM_RESTORE_INITIAL_TABLES = 7;
  TPM_ENABLE_METATRACE = 8;
  TPM_DISABLE_AND_READ_METATRACE = 9;
  TPM_GET_STATUS = 10;
  TPM_RESET_TRACE_PROCESSOR = 11;
- TPM_REGISTER_SQL_MODULE = 12;
+ TPM_REGISTER_SQL_PACKAGE = 13;
  }
 
  oneof type {
@@ -134,8 +136,8 @@ message TraceProcessorRpc {
  EnableMetatraceArgs enable_metatrace_args = 106;
  // For TPM_RESET_TRACE_PROCESSOR.
  ResetTraceProcessorArgs reset_trace_processor_args = 107;
- // For TPM_REGISTER_SQL_MODULE.
- RegisterSqlModuleArgs register_sql_module_args = 108;
+ // For TPM_REGISTER_SQL_PACKAGE.
+ RegisterSqlPackageArgs register_sql_package_args = 108;
 
  // TraceProcessorMethod response args.
  // For TPM_APPEND_TRACE_DATA.
@@ -150,8 +152,8 @@ message TraceProcessorRpc {
  DisableAndReadMetatraceResult metatrace = 209;
  // For TPM_GET_STATUS.
  StatusResult status = 210;
- // For TPM_REGISTER_SQL_MODULE.
- RegisterSqlModuleResult register_sql_module_result = 211;
+ // For TPM_REGISTER_SQL_PACKAGE.
+ RegisterSqlPackageResult register_sql_package_result = 211;
  }
 
  // Previously: RawQueryArgs for TPM_QUERY_RAW_DEPRECATED
@@ -335,15 +337,16 @@ message ResetTraceProcessorArgs {
  optional bool ftrace_drop_until_all_cpus_valid = 4;
 }
 
-message RegisterSqlModuleArgs {
+message RegisterSqlPackageArgs {
  message Module {
  optional string name = 1;
  optional string sql = 2;
  }
- optional string top_level_package_name = 1;
+ optional string package_name = 1;
  repeated Module modules = 2;
- optional bool allow_module_override = 3;
+ optional bool allow_override = 3;
 }
-message RegisterSqlModuleResult {
+
+message RegisterSqlPackageResult {
  optional string error = 1;
 }

src/trace_processor/rpc/rpc.cc

@@ -324,10 +324,10 @@ void Rpc::ParseRpcRequest(const uint8_t* data, size_t len) {
  resp.Send(rpc_response_fn_);
  break;
  }
- case RpcProto::TPM_REGISTER_SQL_MODULE: {
+ case RpcProto::TPM_REGISTER_SQL_PACKAGE: {
  Response resp(tx_seq_id_++, req_type);
- base::Status status = RegisterSqlModule(req.register_sql_module_args());
- auto* res = resp->set_register_sql_module_result();
+ base::Status status = RegisterSqlPackage(req.register_sql_package_args());
+ auto* res = resp->set_register_sql_package_result();
  if (!status.ok()) {
  res->set_error(status.message());
  }
@@ -410,16 +410,16 @@ void Rpc::ResetTraceProcessor(const uint8_t* args, size_t len) {
  ResetTraceProcessorInternal(config);
 }
 
-base::Status Rpc::RegisterSqlModule(protozero::ConstBytes bytes) {
- protos::pbzero::RegisterSqlModuleArgs::Decoder args(bytes);
- SqlModule package;
- package.name = args.top_level_package_name().ToStdString();
- package.allow_module_override = args.allow_module_override();
+base::Status Rpc::RegisterSqlPackage(protozero::ConstBytes bytes) {
+ protos::pbzero::RegisterSqlPackageArgs::Decoder args(bytes);
+ SqlPackage package;
+ package.name = args.package_name().ToStdString();
+ package.allow_override = args.allow_override();
  for (auto it = args.modules(); it; ++it) {
- protos::pbzero::RegisterSqlModuleArgs::Module::Decoder m(*it);
- package.files.emplace_back(m.name().ToStdString(), m.sql().ToStdString());
+ protos::pbzero::RegisterSqlPackageArgs::Module::Decoder m(*it);
+ package.modules.emplace_back(m.name().ToStdString(), m.sql().ToStdString());
  }
- return trace_processor_->RegisterSqlModule(package);
+ return trace_processor_->RegisterSqlPackage(package);
 }
 
 void Rpc::MaybePrintProgress() {

src/trace_processor/rpc/rpc.h

@@ -131,7 +131,7 @@ class Rpc {
  private:
  void ParseRpcRequest(const uint8_t*, size_t);
  void ResetTraceProcessor(const uint8_t*, size_t);
- base::Status RegisterSqlModule(protozero::ConstBytes);
+ base::Status RegisterSqlPackage(protozero::ConstBytes);
  void ResetTraceProcessorInternal(const Config&);
  void MaybePrintProgress();
  Iterator QueryInternal(const uint8_t*, size_t);

src/trace_processor/trace_database_integrationtest.cc

@@ -767,11 +767,11 @@ no such column: t)");
 }
 
 TEST_F(TraceProcessorIntegrationTest, ErrorMessageModule) {
- SqlModule module;
+ SqlPackage module;
  module.name = "foo";
- module.files.push_back(std::make_pair("foo.bar", "select t from slice"));
+ module.modules.push_back(std::make_pair("foo.bar", "select t from slice"));
 
- ASSERT_TRUE(Processor()->RegisterSqlModule(module).ok());
+ ASSERT_TRUE(Processor()->RegisterSqlPackage(module).ok());
 
  auto it = Query("include perfetto module foo.bar;");
  ASSERT_FALSE(it.Next());

src/trace_processor/trace_processor_impl.cc

@@ -593,10 +593,10 @@ bool TraceProcessorImpl::IsRootMetricField(const std::string& metric_name) {
  return field_idx != nullptr;
 }
 
-base::Status TraceProcessorImpl::RegisterSqlModule(SqlModule sql_package) {
+base::Status TraceProcessorImpl::RegisterSqlPackage(SqlPackage sql_package) {
  sql_modules::RegisteredPackage new_package;
  std::string name = sql_package.name;
- if (engine_->FindPackage(name) && !sql_package.allow_module_override) {
+ if (engine_->FindPackage(name) && !sql_package.allow_override) {
  return base::ErrStatus(
  "Package '%s' is already registered. Choose a different name.\n"
  "If you want to replace the existing package using trace processor "
@@ -605,7 +605,7 @@ base::Status TraceProcessorImpl::RegisterSqlModule(SqlModule sql_package) {
  "to pass the module path.",
  name.c_str());
  }
- for (auto const& module_name_and_sql : sql_package.files) {
+ for (auto const& module_name_and_sql : sql_package.modules) {
  if (sql_modules::GetPackageName(module_name_and_sql.first) != name) {
  return base::ErrStatus(
  "Module name doesn't match the package name. First part of module "
@@ -615,7 +615,7 @@ base::Status TraceProcessorImpl::RegisterSqlModule(SqlModule sql_package) {
  new_package.modules.Insert(module_name_and_sql.first,
  {module_name_and_sql.second, false});
  }
- manually_registered_sql_packages_.push_back(SqlModule(sql_package));
+ manually_registered_sql_packages_.push_back(SqlPackage(sql_package));
  engine_->RegisterPackage(name, std::move(new_package));
  return base::OkStatus();
 }
@@ -868,8 +868,8 @@ void TraceProcessorImpl::InitPerfettoSqlEngine() {
  auto packages = GetStdlibPackages();
  for (auto package = packages.GetIterator(); package; ++package) {
  base::Status status =
- RegisterSqlModule({/*name=*/package.key(), /*modules=*/package.value(),
- /*allow_package_override=*/false});
+ RegisterSqlPackage({/*name=*/package.key(), /*modules=*/package.value(),
+  /*allow_override=*/false});
  if (!status.ok())
  PERFETTO_ELOG("%s", status.c_message());
  }
@@ -1095,7 +1095,7 @@ void TraceProcessorImpl::InitPerfettoSqlEngine() {
 
  // Reregister manually added stdlib packages.
  for (const auto& package : manually_registered_sql_packages_) {
- RegisterSqlModule(package);
+ RegisterSqlPackage(package);
  }
 }
 

src/trace_processor/trace_processor_impl.h

@@ -67,7 +67,7 @@ class TraceProcessorImpl : public TraceProcessor,
  base::Status RegisterMetric(const std::string& path,
  const std::string& sql) override;
 
- base::Status RegisterSqlModule(SqlModule) override;
+ base::Status RegisterSqlPackage(SqlPackage) override;
 
  base::Status ExtendMetricsProto(const uint8_t* data, size_t size) override;
 
@@ -97,6 +97,15 @@ class TraceProcessorImpl : public TraceProcessor,
  base::Status DisableAndReadMetatrace(
  std::vector<uint8_t>* trace_proto) override;
 
+ base::Status RegisterSqlModule(SqlModule module) override {
+ SqlPackage package;
+ package.name = std::move(module.name);
+ package.modules = std::move(module.files);
+ package.allow_override = module.allow_module_override;
+
+ return RegisterSqlPackage(package);
+ }
+
  private:
  // Needed for iterators to be able to access the context.
  friend class IteratorImpl;
@@ -120,7 +129,7 @@ class TraceProcessorImpl : public TraceProcessor,
 
  // Manually registeres SQL packages are stored here, to be able to restore
  // them when running |RestoreInitialTables()|.
- std::vector<SqlModule> manually_registered_sql_packages_;
+ std::vector<SqlPackage> manually_registered_sql_packages_;
 
  std::unordered_map<std::string, std::string> proto_field_to_sql_metric_path_;
  std::unordered_map<std::string, std::string> proto_fn_name_to_path_;

src/trace_processor/trace_processor_shell.cc

@@ -1256,9 +1256,9 @@ base::Status IncludeSqlModule(std::string root, bool allow_override) {
  .first->push_back({import_key, file_contents});
  }
  for (auto module_it = modules.GetIterator(); module_it; ++module_it) {
- auto status = g_tp->RegisterSqlModule(
- {/*name=*/module_it.key(), /*files=*/module_it.value(),
- /*allow_module_override=*/allow_override});
+ auto status = g_tp->RegisterSqlPackage({/*name=*/module_it.key(),
+   /*files=*/module_it.value(),
+  /*allow_override=*/allow_override});
  if (!status.ok())
  return status;
  }
@@ -1294,9 +1294,9 @@ base::Status LoadOverridenStdlib(std::string root) {
  .first->push_back({module_name, module_file});
  }
  for (auto package = packages.GetIterator(); package; ++package) {
- g_tp->RegisterSqlModule({/*name=*/package.key(),
- /*files=*/package.value(),
- /*allow_module_override=*/true});
+ g_tp->RegisterSqlPackage({/*name=*/package.key(),
+  /*files=*/package.value(),
+  /*allow_override=*/true});
  }
 
  return base::OkStatus();

ui/src/controller/trace_controller.ts

@@ -298,7 +298,7 @@ async function loadTraceIntoEngine(
  await engine.restoreInitialTables();
  }
  for (const p of globals.extraSqlPackages) {
- await engine.registerSqlModules(p);
+ await engine.registerSqlPackages(p);
  }
 
  const traceDetails = await getTraceInfo(engine, traceSource);

ui/src/protos/index.ts

@@ -71,8 +71,8 @@ import QueryServiceStateRequest = protos.perfetto.protos.QueryServiceStateReques
 import QueryServiceStateResponse = protos.perfetto.protos.QueryServiceStateResponse;
 import ReadBuffersRequest = protos.perfetto.protos.ReadBuffersRequest;
 import ReadBuffersResponse = protos.perfetto.protos.ReadBuffersResponse;
-import RegisterSqlModuleArgs = protos.perfetto.protos.RegisterSqlModuleArgs;
-import RegisterSqlModuleResult = protos.perfetto.protos.RegisterSqlModuleResult;
+import RegisterSqlPackageArgs = protos.perfetto.protos.RegisterSqlPackageArgs;
+import RegisterSqlPackageResult = protos.perfetto.protos.RegisterSqlPackageResult;
 import ResetTraceProcessorArgs = protos.perfetto.protos.ResetTraceProcessorArgs;
 import StatCounters = protos.perfetto.protos.SysStatsConfig.StatCounters;
 import StatusResult = protos.perfetto.protos.StatusResult;
@@ -140,8 +140,8 @@ export {
  QueryServiceStateResponse,
  ReadBuffersRequest,
  ReadBuffersResponse,
- RegisterSqlModuleArgs,
- RegisterSqlModuleResult,
+ RegisterSqlPackageArgs,
+ RegisterSqlPackageResult,
  ResetTraceProcessorArgs,
  StatCounters,
  StatusResult,