From fda5a68698f3d2fa11ef955b45b0a7650285eff6 Mon Sep 17 00:00:00 2001 From: Lalit Maganti Date: Mon, 6 May 2024 14:50:52 +0100 Subject: tp: cleanup RPC code in prep for future changes This CL just cleans up includes/codestyle in the httpd/rpc code to make future CLs more consistent. Change-Id: I44836b521ba53d0af1ec92e66d241a1f198b02e2 --- include/perfetto/ext/base/http/http_server.h | 8 ++-- src/trace_processor/rpc/httpd.cc | 72 ++++++++++++++-------------- src/trace_processor/rpc/httpd.h | 8 ++-- src/trace_processor/rpc/rpc.cc | 41 +++++++++------- src/trace_processor/rpc/rpc.h | 39 +++++++-------- src/trace_processor/rpc/stdiod.cc | 18 ++++--- src/trace_processor/rpc/wasm_bridge.cc | 9 ++-- 7 files changed, 97 insertions(+), 98 deletions(-) diff --git a/include/perfetto/ext/base/http/http_server.h b/include/perfetto/ext/base/http/http_server.h index 2fe563bde..3e6204234 100644 --- a/include/perfetto/ext/base/http/http_server.h +++ b/include/perfetto/ext/base/http/http_server.h @@ -18,6 +18,8 @@ #define INCLUDE_PERFETTO_EXT_BASE_HTTP_HTTP_SERVER_H_ #include +#include +#include #include #include #include @@ -29,8 +31,7 @@ #include "perfetto/ext/base/string_view.h" #include "perfetto/ext/base/unix_socket.h" -namespace perfetto { -namespace base { +namespace perfetto::base { class HttpServerConnection; @@ -183,7 +184,6 @@ class HttpServer : public UnixSocket::EventListener { bool origin_error_logged_ = false; }; -} // namespace base -} // namespace perfetto +} // namespace perfetto::base #endif // INCLUDE_PERFETTO_EXT_BASE_HTTP_HTTP_SERVER_H_ diff --git a/src/trace_processor/rpc/httpd.cc b/src/trace_processor/rpc/httpd.cc index 2c44b71b5..e63bdb499 100644 --- a/src/trace_processor/rpc/httpd.cc +++ b/src/trace_processor/rpc/httpd.cc @@ -14,26 +14,29 @@ * limitations under the License. */ -#include "perfetto/base/build_config.h" - -#if PERFETTO_BUILDFLAG(PERFETTO_TP_HTTPD) - -#include "src/trace_processor/rpc/httpd.h" - +#include +#include +#include +#include +#include +#include +#include +#include + +#include "perfetto/base/logging.h" +#include "perfetto/base/status.h" #include "perfetto/ext/base/http/http_server.h" #include "perfetto/ext/base/string_utils.h" #include "perfetto/ext/base/string_view.h" #include "perfetto/ext/base/unix_task_runner.h" -#include "perfetto/ext/base/utils.h" #include "perfetto/protozero/scattered_heap_buffer.h" #include "perfetto/trace_processor/trace_processor.h" +#include "src/trace_processor/rpc/httpd.h" #include "src/trace_processor/rpc/rpc.h" #include "protos/perfetto/trace_processor/trace_processor.pbzero.h" -namespace perfetto { -namespace trace_processor { - +namespace perfetto::trace_processor { namespace { constexpr int kBindPort = 9001; @@ -58,9 +61,9 @@ class Httpd : public base::HttpRequestHandler { void OnHttpRequest(const base::HttpRequest&) override; void OnWebsocketMessage(const base::WebsocketMessage&) override; - void ServeHelpPage(const base::HttpRequest&); + static void ServeHelpPage(const base::HttpRequest&); - Rpc trace_processor_rpc_; + Rpc global_trace_processor_rpc_; base::UnixTaskRunner task_runner_; base::HttpServer http_srv_; }; @@ -68,7 +71,7 @@ class Httpd : public base::HttpRequestHandler { base::HttpServerConnection* g_cur_conn; base::StringView Vec2Sv(const std::vector& v) { - return base::StringView(reinterpret_cast(v.data()), v.size()); + return {reinterpret_cast(v.data()), v.size()}; } // Used both by websockets and /rpc chunked HTTP endpoints. @@ -91,7 +94,7 @@ void SendRpcChunk(const void* data, uint32_t len) { } Httpd::Httpd(std::unique_ptr preloaded_instance) - : trace_processor_rpc_(std::move(preloaded_instance)), + : global_trace_processor_rpc_(std::move(preloaded_instance)), http_srv_(&task_runner_, this) {} Httpd::~Httpd() = default; @@ -103,8 +106,9 @@ void Httpd::Run(int port) { "or through the Python API (see " "https://perfetto.dev/docs/analysis/trace-processor#python-api)."); - for (size_t i = 0; i < base::ArraySize(kAllowedCORSOrigins); ++i) - http_srv_.AddAllowedOrigin(kAllowedCORSOrigins[i]); + for (const auto& kAllowedCORSOrigin : kAllowedCORSOrigins) { + http_srv_.AddAllowedOrigin(kAllowedCORSOrigin); + } http_srv_.Start(port); task_runner_.Run(); } @@ -139,7 +143,7 @@ void Httpd::OnHttpRequest(const base::HttpRequest& req) { }; if (req.uri == "/status") { - auto status = trace_processor_rpc_.GetStatus(); + auto status = global_trace_processor_rpc_.GetStatus(); return conn.SendResponse("200 OK", default_headers, Vec2Sv(status)); } @@ -163,10 +167,10 @@ void Httpd::OnHttpRequest(const base::HttpRequest& req) { base::HttpServerConnection::kOmitContentLength); PERFETTO_CHECK(g_cur_conn == nullptr); g_cur_conn = req.conn; - trace_processor_rpc_.SetRpcResponseFunction(SendRpcChunk); + global_trace_processor_rpc_.SetRpcResponseFunction(SendRpcChunk); // OnRpcRequest() will call SendRpcChunk() one or more times. - trace_processor_rpc_.OnRpcRequest(req.body.data(), req.body.size()); - trace_processor_rpc_.SetRpcResponseFunction(nullptr); + global_trace_processor_rpc_.OnRpcRequest(req.body.data(), req.body.size()); + global_trace_processor_rpc_.SetRpcResponseFunction(nullptr); g_cur_conn = nullptr; // Terminate chunked stream. @@ -175,7 +179,7 @@ void Httpd::OnHttpRequest(const base::HttpRequest& req) { } if (req.uri == "/parse") { - base::Status status = trace_processor_rpc_.Parse( + base::Status status = global_trace_processor_rpc_.Parse( reinterpret_cast(req.body.data()), req.body.size()); protozero::HeapBuffered result; if (!status.ok()) { @@ -186,12 +190,12 @@ void Httpd::OnHttpRequest(const base::HttpRequest& req) { } if (req.uri == "/notify_eof") { - trace_processor_rpc_.NotifyEndOfFile(); + global_trace_processor_rpc_.NotifyEndOfFile(); return conn.SendResponse("200 OK", default_headers); } if (req.uri == "/restore_initial_tables") { - trace_processor_rpc_.RestoreInitialTables(); + global_trace_processor_rpc_.RestoreInitialTables(); return conn.SendResponse("200 OK", default_headers); } @@ -217,26 +221,27 @@ void Httpd::OnHttpRequest(const base::HttpRequest& req) { if (!has_more) conn.SendResponseBody("0\r\n\r\n", 5); }; - trace_processor_rpc_.Query( + global_trace_processor_rpc_.Query( reinterpret_cast(req.body.data()), req.body.size(), on_result_chunk); return; } if (req.uri == "/compute_metric") { - std::vector res = trace_processor_rpc_.ComputeMetric( + std::vector res = global_trace_processor_rpc_.ComputeMetric( reinterpret_cast(req.body.data()), req.body.size()); return conn.SendResponse("200 OK", default_headers, Vec2Sv(res)); } if (req.uri == "/enable_metatrace") { - trace_processor_rpc_.EnableMetatrace( + global_trace_processor_rpc_.EnableMetatrace( reinterpret_cast(req.body.data()), req.body.size()); return conn.SendResponse("200 OK", default_headers); } if (req.uri == "/disable_and_read_metatrace") { - std::vector res = trace_processor_rpc_.DisableAndReadMetatrace(); + std::vector res = + global_trace_processor_rpc_.DisableAndReadMetatrace(); return conn.SendResponse("200 OK", default_headers, Vec2Sv(res)); } @@ -246,17 +251,17 @@ void Httpd::OnHttpRequest(const base::HttpRequest& req) { void Httpd::OnWebsocketMessage(const base::WebsocketMessage& msg) { PERFETTO_CHECK(g_cur_conn == nullptr); g_cur_conn = msg.conn; - trace_processor_rpc_.SetRpcResponseFunction(SendRpcChunk); + global_trace_processor_rpc_.SetRpcResponseFunction(SendRpcChunk); // OnRpcRequest() will call SendRpcChunk() one or more times. - trace_processor_rpc_.OnRpcRequest(msg.data.data(), msg.data.size()); - trace_processor_rpc_.SetRpcResponseFunction(nullptr); + global_trace_processor_rpc_.OnRpcRequest(msg.data.data(), msg.data.size()); + global_trace_processor_rpc_.SetRpcResponseFunction(nullptr); g_cur_conn = nullptr; } } // namespace void RunHttpRPCServer(std::unique_ptr preloaded_instance, - std::string port_number) { + const std::string& port_number) { Httpd srv(std::move(preloaded_instance)); std::optional port_opt = base::StringToInt32(port_number); int port = port_opt.has_value() ? *port_opt : kBindPort; @@ -291,7 +296,4 @@ https://perfetto.dev/docs/contributing/getting-started#community req.conn->SendResponse("200 OK", headers, kPage); } -} // namespace trace_processor -} // namespace perfetto - -#endif // PERFETTO_TP_HTTPD +} // namespace perfetto::trace_processor diff --git a/src/trace_processor/rpc/httpd.h b/src/trace_processor/rpc/httpd.h index 57e7ddb4b..f8d89c70f 100644 --- a/src/trace_processor/rpc/httpd.h +++ b/src/trace_processor/rpc/httpd.h @@ -20,8 +20,7 @@ #include #include -namespace perfetto { -namespace trace_processor { +namespace perfetto::trace_processor { class TraceProcessor; @@ -30,9 +29,8 @@ class TraceProcessor; // The unique_ptr argument is optional. If non-null, the HTTP server will adopt // an existing instance with a pre-loaded trace. If null, it will create a new // instance when pushing data into the /parse endpoint. -void RunHttpRPCServer(std::unique_ptr, std::string); +void RunHttpRPCServer(std::unique_ptr, const std::string&); -} // namespace trace_processor -} // namespace perfetto +} // namespace perfetto::trace_processor #endif // SRC_TRACE_PROCESSOR_RPC_HTTPD_H_ diff --git a/src/trace_processor/rpc/rpc.cc b/src/trace_processor/rpc/rpc.cc index b4552e5af..c715784f5 100644 --- a/src/trace_processor/rpc/rpc.cc +++ b/src/trace_processor/rpc/rpc.cc @@ -16,30 +16,36 @@ #include "src/trace_processor/rpc/rpc.h" -#include - +#include +#include +#include +#include +#include +#include +#include #include #include "perfetto/base/logging.h" +#include "perfetto/base/status.h" #include "perfetto/base/time.h" -#include "perfetto/ext/base/utils.h" #include "perfetto/ext/base/version.h" #include "perfetto/ext/protozero/proto_ring_buffer.h" #include "perfetto/ext/trace_processor/rpc/query_result_serializer.h" +#include "perfetto/protozero/field.h" #include "perfetto/protozero/scattered_heap_buffer.h" -#include "perfetto/protozero/scattered_stream_writer.h" #include "perfetto/trace_processor/basic_types.h" +#include "perfetto/trace_processor/metatrace_config.h" #include "perfetto/trace_processor/trace_processor.h" #include "src/trace_processor/tp_metatrace.h" +#include "protos/perfetto/trace_processor/metatrace_categories.pbzero.h" #include "protos/perfetto/trace_processor/trace_processor.pbzero.h" -namespace perfetto { -namespace trace_processor { +namespace perfetto::trace_processor { namespace { // Writes a "Loading trace ..." update every N bytes. -constexpr size_t kProgressUpdateBytes = 50 * 1000 * 1000; +constexpr size_t kProgressUpdateBytes = 50ul * 1000 * 1000; using TraceProcessorRpcStream = protos::pbzero::TraceProcessorRpcStream; using RpcProto = protos::pbzero::TraceProcessorRpc; @@ -194,7 +200,7 @@ void Rpc::ParseRpcRequest(const uint8_t* data, size_t len) { result->set_error(kErrFieldNotSet); } else { protozero::ConstBytes byte_range = req.append_trace_data(); - util::Status res = Parse(byte_range.data, byte_range.size); + base::Status res = Parse(byte_range.data, byte_range.size); if (!res.ok()) { result->set_error(res.message()); } @@ -295,7 +301,7 @@ void Rpc::ParseRpcRequest(const uint8_t* data, size_t len) { } // switch(req_type) } -util::Status Rpc::Parse(const uint8_t* data, size_t len) { +base::Status Rpc::Parse(const uint8_t* data, size_t len) { PERFETTO_TP_TRACE( metatrace::Category::API_TIMELINE, "RPC_PARSE", [&](metatrace::Record* r) { r->AddArg("length", std::to_string(len)); }); @@ -310,7 +316,7 @@ util::Status Rpc::Parse(const uint8_t* data, size_t len) { MaybePrintProgress(); if (len == 0) - return util::OkStatus(); + return base::OkStatus(); // TraceProcessor needs take ownership of the memory chunk. std::unique_ptr data_copy(new uint8_t[len]); @@ -372,7 +378,7 @@ void Rpc::MaybePrintProgress() { void Rpc::Query(const uint8_t* args, size_t len, - QueryResultBatchCallback result_callback) { + const QueryResultBatchCallback& result_callback) { auto it = QueryInternal(args, len); QueryResultSerializer serializer(std::move(it)); @@ -396,7 +402,7 @@ Iterator Rpc::QueryInternal(const uint8_t* args, size_t len) { } }); - return trace_processor_->ExecuteQuery(sql.c_str()); + return trace_processor_->ExecuteQuery(sql); } void Rpc::RestoreInitialTables() { @@ -432,7 +438,7 @@ void Rpc::ComputeMetricInternal(const uint8_t* data, switch (args.format()) { case protos::pbzero::ComputeMetricArgs::BINARY_PROTOBUF: { std::vector metrics_proto; - util::Status status = + base::Status status = trace_processor_->ComputeMetric(metric_names, &metrics_proto); if (status.ok()) { result->set_metrics(metrics_proto.data(), metrics_proto.size()); @@ -443,7 +449,7 @@ void Rpc::ComputeMetricInternal(const uint8_t* data, } case protos::pbzero::ComputeMetricArgs::TEXTPROTO: { std::string metrics_string; - util::Status status = trace_processor_->ComputeMetricText( + base::Status status = trace_processor_->ComputeMetricText( metric_names, TraceProcessor::MetricResultFormat::kProtoText, &metrics_string); if (status.ok()) { @@ -455,7 +461,7 @@ void Rpc::ComputeMetricInternal(const uint8_t* data, } case protos::pbzero::ComputeMetricArgs::JSON: { std::string metrics_string; - util::Status status = trace_processor_->ComputeMetricText( + base::Status status = trace_processor_->ComputeMetricText( metric_names, TraceProcessor::MetricResultFormat::kJson, &metrics_string); if (status.ok()) { @@ -486,7 +492,7 @@ std::vector Rpc::DisableAndReadMetatrace() { void Rpc::DisableAndReadMetatraceInternal( protos::pbzero::DisableAndReadMetatraceResult* result) { std::vector trace_proto; - util::Status status = trace_processor_->DisableAndReadMetatrace(&trace_proto); + base::Status status = trace_processor_->DisableAndReadMetatrace(&trace_proto); if (status.ok()) { result->set_metatrace(trace_proto.data(), trace_proto.size()); } else { @@ -506,5 +512,4 @@ std::vector Rpc::GetStatus() { return status.SerializeAsArray(); } -} // namespace trace_processor -} // namespace perfetto +} // namespace perfetto::trace_processor diff --git a/src/trace_processor/rpc/rpc.h b/src/trace_processor/rpc/rpc.h index c10d4bf8c..61cf2e636 100644 --- a/src/trace_processor/rpc/rpc.h +++ b/src/trace_processor/rpc/rpc.h @@ -17,25 +17,23 @@ #ifndef SRC_TRACE_PROCESSOR_RPC_RPC_H_ #define SRC_TRACE_PROCESSOR_RPC_RPC_H_ +#include +#include #include #include +#include #include -#include -#include - +#include "perfetto/base/status.h" #include "perfetto/ext/protozero/proto_ring_buffer.h" #include "perfetto/trace_processor/basic_types.h" -#include "perfetto/trace_processor/status.h" namespace perfetto { -namespace protos { -namespace pbzero { +namespace protos::pbzero { class ComputeMetricResult; class DisableAndReadMetatraceResult; -} // namespace pbzero -} // namespace protos +} // namespace protos::pbzero namespace trace_processor { @@ -100,12 +98,12 @@ class Rpc { // The methods of this class are mirrors (modulo {un,}marshalling of args) of // the corresponding names in trace_processor.h . See that header for docs. - util::Status Parse(const uint8_t* data, size_t len); + base::Status Parse(const uint8_t*, size_t); void NotifyEndOfFile(); - void ResetTraceProcessor(const uint8_t* args, size_t len); + void ResetTraceProcessor(const uint8_t*, size_t); std::string GetCurrentTraceName(); - std::vector ComputeMetric(const uint8_t* data, size_t len); - void EnableMetatrace(const uint8_t* data, size_t len); // EnableMetatraceArgs + std::vector ComputeMetric(const uint8_t*, size_t); + void EnableMetatrace(const uint8_t*, size_t); std::vector DisableAndReadMetatrace(); std::vector GetStatus(); @@ -122,22 +120,17 @@ class Rpc { // ... // callback(..., has_more=false) // (Query() returns at this point). - // TODO(primiano): long-term this API should change and be turned into a - // bidirectional streaming api (see go/imperative-metrics). The problem with - // the current design is that it holds the callstack until the query is done - // and makes nested query hard as they cause re-entrancy. It's okay for now - // but will change soon. using QueryResultBatchCallback = std::function< void(const uint8_t* /*buf*/, size_t /*len*/, bool /*has_more*/)>; - void Query(const uint8_t* args, size_t len, QueryResultBatchCallback); + void Query(const uint8_t*, size_t, const QueryResultBatchCallback&); private: - void ParseRpcRequest(const uint8_t* data, size_t len); - void ResetTraceProcessorInternal(const Config& config); + void ParseRpcRequest(const uint8_t*, size_t); + void ResetTraceProcessorInternal(const Config&); void MaybePrintProgress(); - Iterator QueryInternal(const uint8_t* args, size_t len); - void ComputeMetricInternal(const uint8_t* args, - size_t len, + Iterator QueryInternal(const uint8_t*, size_t); + void ComputeMetricInternal(const uint8_t*, + size_t, protos::pbzero::ComputeMetricResult*); void DisableAndReadMetatraceInternal( protos::pbzero::DisableAndReadMetatraceResult*); diff --git a/src/trace_processor/rpc/stdiod.cc b/src/trace_processor/rpc/stdiod.cc index ae49c874f..d6cf8de42 100644 --- a/src/trace_processor/rpc/stdiod.cc +++ b/src/trace_processor/rpc/stdiod.cc @@ -16,10 +16,12 @@ #include "src/trace_processor/rpc/stdiod.h" -#if !PERFETTO_BUILDFLAG(PERFETTO_OS_WIN) -#include -#endif +#include +#include +#include +#include +#include "perfetto/base/build_config.h" #include "perfetto/base/logging.h" #include "perfetto/base/status.h" #include "perfetto/ext/base/file_utils.h" @@ -27,13 +29,16 @@ #include "perfetto/trace_processor/trace_processor.h" #include "src/trace_processor/rpc/rpc.h" +#if !PERFETTO_BUILDFLAG(PERFETTO_OS_WIN) +#include +#endif + #if PERFETTO_BUILDFLAG(PERFETTO_OS_WIN) && !defined(STDIN_FILENO) #define STDIN_FILENO 0 #define STDOUT_FILENO 1 #endif -namespace perfetto { -namespace trace_processor { +namespace perfetto::trace_processor { base::Status RunStdioRpcServer(std::unique_ptr tp) { Rpc rpc(std::move(tp)); @@ -57,5 +62,4 @@ base::Status RunStdioRpcServer(std::unique_ptr tp) { } } -} // namespace trace_processor -} // namespace perfetto +} // namespace perfetto::trace_processor diff --git a/src/trace_processor/rpc/wasm_bridge.cc b/src/trace_processor/rpc/wasm_bridge.cc index 6e266b85d..a9ba607c5 100644 --- a/src/trace_processor/rpc/wasm_bridge.cc +++ b/src/trace_processor/rpc/wasm_bridge.cc @@ -15,13 +15,11 @@ */ #include +#include -#include "perfetto/base/logging.h" -#include "perfetto/trace_processor/trace_processor.h" #include "src/trace_processor/rpc/rpc.h" -namespace perfetto { -namespace trace_processor { +namespace perfetto::trace_processor { namespace { Rpc* g_trace_processor_rpc; @@ -60,8 +58,7 @@ void trace_processor_on_rpc_request(uint32_t size) { } } // extern "C" -} // namespace trace_processor -} // namespace perfetto +} // namespace perfetto::trace_processor int main(int, char**) { // This is unused but is needed for the following reasons: -- cgit v1.2.3