mojo: Remove RandUint64() from CreateMessagePipe and CreateDataPipe

The calls to the CSRNG take a lot of cycles on Android, and mojo
CreateMessagePipe()/CreateDataPipe() is a big contributor to the number
of calls to base::RandUint64(). Those calls are used to create a pipe_id
for the needs of logging and tracing.

Add a base::Feature(kMojoAvoidRandomPipeId) as disabled by default. When
enabled, avoids calling RandUint64() and keeps the |pipe_id| equal to 0.

There are a couple of trace events that rely on |pipe_id| having a
unique value for correctness. Move them under the
|extended_tracing_enabled| GN flag.  Under this flag the value of
|kMojoAvoidRandomPipeId| is ignored, and the pipe_id is delivered by
base::trace_event::GetNextGlobalTraceId().

Bug: 1351813
Change-Id: I8f2311831824c240651044795722151f58d1bee9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3825925
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Commit-Queue: Egor Pasko <pasko@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1034950}

mojo/core/BUILD.gn

@@ -116,6 +116,7 @@ template("core_impl_source_set") {
       "//mojo/core/embedder:features",
       "//mojo/core/ports",
       "//mojo/public/c/system:headers",
+      "//mojo/public/cpp/bindings:mojo_buildflags",
       "//mojo/public/cpp/platform",
       "//third_party/ipcz/src:ipcz_chromium",
     ]

mojo/core/core.cc

@@ -22,6 +22,7 @@
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/time/time.h"
 #include "base/trace_event/memory_dump_manager.h"
+#include "base/trace_event/trace_id_helper.h"
 #include "build/build_config.h"
 #include "mojo/core/channel.h"
 #include "mojo/core/configuration.h"
@@ -41,6 +42,7 @@
 #include "mojo/core/shared_buffer_dispatcher.h"
 #include "mojo/core/user_message_impl.h"
 #include "mojo/core/watcher_dispatcher.h"
+#include "mojo/public/cpp/bindings/mojo_buildflags.h"
 #include "mojo/public/cpp/platform/platform_handle_internal.h"
 
 namespace mojo {
@@ -59,6 +61,9 @@ const uint64_t kUnknownPipeIdForDebug = 0x7f7f7f7f7f7f7f7fUL;
 // invitation.
 constexpr base::StringPiece kIsolatedInvitationPipeName = {"\0\0\0\0", 4};
 
+// Set according to the field trial "MojoAvoidRandomPipeId".
+bool g_avoid_random_pipe_id;
+
 void InvokeProcessErrorCallback(MojoProcessErrorHandler handler,
                                 uintptr_t context,
                                 const std::string& error,
@@ -109,6 +114,16 @@ void RunMojoProcessErrorHandler(
                              MOJO_PROCESS_ERROR_FLAG_NONE);
 }
 
+uint64_t MakePipeId() {
+#if BUILDFLAG(MOJO_TRACE_ENABLED)
+  return base::trace_event::GetNextGlobalTraceId();
+#else
+  if (g_avoid_random_pipe_id)
+    return 0;
+  return base::RandUint64();
+#endif
+}
+
 }  // namespace
 
 Core::Core() {
@@ -499,15 +514,15 @@ MojoResult Core::CreateMessagePipe(const MojoCreateMessagePipeOptions* options,
   DCHECK(message_pipe_handle0);
   DCHECK(message_pipe_handle1);
 
-  uint64_t pipe_id = base::RandUint64();
+  uint64_t pipe_id = MakePipeId();
 
-  *message_pipe_handle0 = AddDispatcher(
-      new MessagePipeDispatcher(GetNodeController(), port0, pipe_id, 0));
+  *message_pipe_handle0 = AddDispatcher(new MessagePipeDispatcher(
+      GetNodeController(), port0, pipe_id, /*endpoint=*/0));
   if (*message_pipe_handle0 == MOJO_HANDLE_INVALID)
     return MOJO_RESULT_RESOURCE_EXHAUSTED;
 
-  *message_pipe_handle1 = AddDispatcher(
-      new MessagePipeDispatcher(GetNodeController(), port1, pipe_id, 1));
+  *message_pipe_handle1 = AddDispatcher(new MessagePipeDispatcher(
+      GetNodeController(), port1, pipe_id, /*endpoint=*/1));
   if (*message_pipe_handle1 == MOJO_HANDLE_INVALID) {
     scoped_refptr<Dispatcher> dispatcher0;
     {
@@ -673,7 +688,7 @@ MojoResult Core::CreateDataPipe(const MojoCreateDataPipeOptions* options,
   DCHECK(data_pipe_consumer_handle);
 
   base::UnsafeSharedMemoryRegion consumer_region = producer_region.Duplicate();
-  uint64_t pipe_id = base::RandUint64();
+  uint64_t pipe_id = MakePipeId();
   scoped_refptr<Dispatcher> producer = DataPipeProducerDispatcher::Create(
       GetNodeController(), port0, std::move(producer_region), create_options,
       pipe_id);
@@ -1504,6 +1519,11 @@ void Core::GetActiveHandlesForTest(std::vector<MojoHandle>* handles) {
   handles_->GetActiveHandlesForTest(handles);
 }
 
+// static
+void Core::set_avoid_random_pipe_id(bool avoid_random_pipe_id) {
+  g_avoid_random_pipe_id = avoid_random_pipe_id;
+}
+
 // static
 void Core::PassNodeControllerToIOThread(
     std::unique_ptr<NodeController> node_controller) {

mojo/core/core.h

@@ -332,6 +332,8 @@ class MOJO_SYSTEM_IMPL_EXPORT Core {
 
   void GetActiveHandlesForTest(std::vector<MojoHandle>* handles);
 
+  static void set_avoid_random_pipe_id(bool avoid_random_pipe_id);
+
  private:
   // Used to pass ownership of our NodeController over to the IO thread in the
   // event that we're torn down before said thread.

mojo/core/embedder/embedder.cc

@@ -56,6 +56,9 @@ void InitFeatures() {
 
   Channel::set_use_trivial_messages(
       base::FeatureList::IsEnabled(kMojoInlineMessagePayloads));
+
+  Core::set_avoid_random_pipe_id(
+      base::FeatureList::IsEnabled(kMojoAvoidRandomPipeId));
 }
 
 void Init(const Configuration& configuration) {

mojo/core/embedder/features.cc

@@ -28,5 +28,8 @@ const base::Feature kMojoPosixUseWritev{"MojoPosixUseWritev",
 const base::Feature kMojoInlineMessagePayloads{
     "MojoInlineMessagePayloads", base::FEATURE_DISABLED_BY_DEFAULT};
 
+const base::Feature kMojoAvoidRandomPipeId{"MojoAvoidRandomPipeId",
+                                           base::FEATURE_DISABLED_BY_DEFAULT};
+
 }  // namespace core
 }  // namespace mojo

mojo/core/embedder/features.h

@@ -34,6 +34,9 @@ extern const base::Feature kMojoPosixUseWritev;
 COMPONENT_EXPORT(MOJO_CORE_EMBEDDER_FEATURES)
 extern const base::Feature kMojoInlineMessagePayloads;
 
+COMPONENT_EXPORT(MOJO_CORE_EMBEDDER_FEATURES)
+extern const base::Feature kMojoAvoidRandomPipeId;
+
 }  // namespace core
 }  // namespace mojo
 

mojo/core/message_pipe_dispatcher.cc

@@ -16,6 +16,7 @@
 #include "mojo/core/ports/message_filter.h"
 #include "mojo/core/request_context.h"
 #include "mojo/core/user_message_impl.h"
+#include "mojo/public/cpp/bindings/mojo_buildflags.h"
 
 namespace mojo {
 namespace core {
@@ -397,8 +398,11 @@ MojoResult MessagePipeDispatcher::CloseNoLock() {
     base::AutoUnlock unlock(signal_lock_);
     node_controller_->ClosePort(port_);
 
-    TRACE_EVENT_WITH_FLOW0("toplevel.flow", "MessagePipe closing",
-                           pipe_id_ + endpoint_, TRACE_EVENT_FLAG_FLOW_OUT);
+#if BUILDFLAG(MOJO_TRACE_ENABLED)
+    TRACE_EVENT_WITH_FLOW0(TRACE_DISABLED_BY_DEFAULT("mojom"),
+                           "MessagePipe closing", pipe_id_ + endpoint_,
+                           TRACE_EVENT_FLAG_FLOW_OUT);
+#endif
   }
 
   return MOJO_RESULT_OK;
@@ -442,17 +446,18 @@ HandleSignalsState MessagePipeDispatcher::GetHandleSignalsStateNoLock() const {
   }
   rv.satisfiable_signals |=
       MOJO_HANDLE_SIGNAL_PEER_CLOSED | MOJO_HANDLE_SIGNAL_QUOTA_EXCEEDED;
-
+#if BUILDFLAG(MOJO_TRACE_ENABLED)
   const bool was_peer_closed =
       last_known_satisfied_signals_ & MOJO_HANDLE_SIGNAL_PEER_CLOSED;
   const bool is_peer_closed =
       rv.satisfied_signals & MOJO_HANDLE_SIGNAL_PEER_CLOSED;
-  last_known_satisfied_signals_ = rv.satisfied_signals;
   if (is_peer_closed && !was_peer_closed) {
-    TRACE_EVENT_WITH_FLOW0("toplevel.flow", "MessagePipe peer closed",
-                           pipe_id_ + (1 - endpoint_),
-                           TRACE_EVENT_FLAG_FLOW_IN);
+    TRACE_EVENT_WITH_FLOW0(
+        TRACE_DISABLED_BY_DEFAULT("mojom"), "MessagePipe peer closed",
+        pipe_id_ + (1 - endpoint_), TRACE_EVENT_FLAG_FLOW_IN);
   }
+#endif
+  last_known_satisfied_signals_ = rv.satisfied_signals;
 
   return rv;
 }