cogl/trace: Make global start/stop more explicit

Don't try to handle things by threads enabling/disabling the main trace
context on-demand, just have a clear start/stop API. For the D-Bus API,
it becomes more straight forward, and for the persistent variant too, as
it avoids having to pass garbage input when it's known that arguments
will be discarded.

Part-of: <https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/2998>

cogl/cogl/cogl-trace.c

@@ -127,14 +127,24 @@ cogl_trace_context_ref (CoglTraceContext *trace_context)
   return trace_context;
 }
 
-static void
-ensure_trace_context (int         fd,
-                      const char *filename)
+static gboolean
+setup_trace_context (int          fd,
+                     const char  *filename,
+                     GError     **error)
 {
-  g_mutex_lock (&cogl_trace_mutex);
-  if (!cogl_trace_context)
+  g_autoptr (GMutexLocker) locker = NULL;
+
+  locker = g_mutex_locker_new (&cogl_trace_mutex);
+  if (cogl_trace_context)
+    {
+      g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
+                   "Trace context already setup");
+      return FALSE;
+    }
+
   cogl_trace_context = cogl_trace_context_new (fd, filename);
-  g_mutex_unlock (&cogl_trace_mutex);
+
+  return TRUE;
 }
 
 static CoglTraceThreadContext *
@@ -205,15 +215,35 @@ disable_tracing_idle_callback (gpointer user_data)
   return G_SOURCE_REMOVE;
 }
 
-static void
-set_tracing_enabled_on_thread (GMainContext *main_context,
-                               const char   *group,
-                               int           fd,
-                               const char   *filename)
+gboolean
+cogl_start_tracing_with_path (const char  *filename,
+                              GError     **error)
+{
+  return setup_trace_context (-1, filename, error);
+}
+
+gboolean
+cogl_start_tracing_with_fd (int      fd,
+                            GError **error)
+{
+  return setup_trace_context (fd, NULL, error);
+}
+
+void
+cogl_stop_tracing (void)
+{
+  g_mutex_lock (&cogl_trace_mutex);
+  g_clear_pointer (&cogl_trace_context, cogl_trace_context_unref);
+  g_mutex_unlock (&cogl_trace_mutex);
+}
+
+void
+cogl_set_tracing_enabled_on_thread (GMainContext *main_context,
+                                    const char   *group)
 {
   TraceData *data;
 
-  ensure_trace_context (fd, filename);
+  g_return_if_fail (cogl_trace_context);
 
   data = g_new0 (TraceData, 1);
   data->group = group ? strdup (group) : NULL;
@@ -239,29 +269,9 @@ set_tracing_enabled_on_thread (GMainContext *main_context,
     }
 }
 
-void
-cogl_set_tracing_enabled_on_thread_with_fd (GMainContext *main_context,
-                                            const char   *group,
-                                            int           fd)
-{
-  set_tracing_enabled_on_thread (main_context, group, fd, NULL);
-}
-
-void
-cogl_set_tracing_enabled_on_thread (GMainContext *main_context,
-                                    const char   *group,
-                                    const char   *filename)
-{
-  set_tracing_enabled_on_thread (main_context, group, -1, filename);
-}
-
 void
 cogl_set_tracing_disabled_on_thread (GMainContext *main_context)
 {
-  g_mutex_lock (&cogl_trace_mutex);
-  g_clear_pointer (&cogl_trace_context, cogl_trace_context_unref);
-  g_mutex_unlock (&cogl_trace_mutex);
-
   if (g_main_context_get_thread_default () == main_context)
     {
       disable_tracing_idle_callback (NULL);
@@ -330,18 +340,33 @@ cogl_trace_describe (CoglTraceHead *head,
 #include <string.h>
 #include <stdio.h>
 
+gboolean
+cogl_start_tracing_with_path (const char  *filename,
+                              GError     **error)
+{
+  g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_SUPPORTED,
+               "Tracing disabled at build time");
+  return FALSE;
+}
+
+gboolean
+cogl_start_tracing_with_fd (int      fd,
+                            GError **error)
+{
+  g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_SUPPORTED,
+               "Tracing disabled at build time");
+  return FALSE;
+}
+
 void
-cogl_set_tracing_enabled_on_thread_with_fd (void       *data,
-                                            const char *group,
-                                            int         fd)
+cogl_stop_tracing (void)
 {
   fprintf (stderr, "Tracing not enabled");
 }
 
 void
 cogl_set_tracing_enabled_on_thread (void       *data,
-                                    const char *group,
-                                    const char *filename)
+                                    const char *group)
 {
   fprintf (stderr, "Tracing not enabled");
 }

cogl/cogl/cogl-trace.h

@@ -29,6 +29,7 @@
 #define COGL_TRACE_H
 
 #include <glib.h>
+#include <gio/gio.h>
 #include <stdint.h>
 #include <errno.h>
 
@@ -53,18 +54,26 @@ CoglTraceContext *cogl_trace_context;
 COGL_EXPORT
 GMutex cogl_trace_mutex;
 
-COGL_EXPORT void
-cogl_set_tracing_enabled_on_thread_with_fd (GMainContext *main_context,
-                                            const char   *group,
-                                            int           fd);
+COGL_EXPORT
+gboolean cogl_start_tracing_with_path (const char  *filename,
+                                       GError     **error);
 
-COGL_EXPORT void
-cogl_set_tracing_enabled_on_thread (GMainContext *main_context,
-                                    const char   *group,
-                                    const char   *filename);
+COGL_EXPORT
+gboolean cogl_start_tracing_with_fd (int      fd,
+                                     GError **error);
 
-COGL_EXPORT void
-cogl_set_tracing_disabled_on_thread (GMainContext *main_context);
+COGL_EXPORT
+void cogl_stop_tracing (void);
+
+COGL_EXPORT
+gboolean cogl_is_tracing (void);
+
+COGL_EXPORT
+void cogl_set_tracing_enabled_on_thread (GMainContext *main_context,
+                                         const char   *group);
+
+COGL_EXPORT
+void cogl_set_tracing_disabled_on_thread (GMainContext *main_context);
 
 static inline void
 cogl_trace_begin (CoglTraceHead *head,
@@ -140,16 +149,25 @@ cogl_is_tracing_enabled (void)
 #define COGL_TRACE_ANCHOR(Name) (void) 0
 #define COGL_TRACE_BEGIN_ANCHORED(Name, name) (void) 0
 
-COGL_EXPORT void
-cogl_set_tracing_enabled_on_thread_with_fd (void       *data,
-                                            const char *group,
-                                            int         fd);
-COGL_EXPORT void
-cogl_set_tracing_enabled_on_thread (void       *data,
-                                    const char *group,
-                                    const char *filename);
-COGL_EXPORT void
-cogl_set_tracing_disabled_on_thread (void *data);
+COGL_EXPORT
+gboolean cogl_start_tracing_with_path (const char  *filename,
+                                       GError     **error);
+
+COGL_EXPORT
+gboolean cogl_start_tracing_with_fd (int      fd,
+                                     GError **error);
+
+COGL_EXPORT
+void cogl_stop_tracing (void);
+
+COGL_EXPORT
+gboolean cogl_is_tracing (void);
+
+COGL_EXPORT
+void cogl_set_tracing_enabled_on_thread (void       *data,
+                                         const char *group)
+COGL_EXPORT
+void cogl_set_tracing_disabled_on_thread (void *data);
 
 #endif /* COGL_HAS_TRACING */
 

src/core/meta-profiler.c

@@ -87,6 +87,7 @@ handle_start (MetaDBusSysprof3Profiler *dbus_profiler,
 {
   MetaProfiler *profiler = META_PROFILER (dbus_profiler);
   GMainContext *main_context = g_main_context_default ();
+  g_autoptr (GError) error = NULL;
   const char *group_name;
   int position;
   int fd = -1;
@@ -111,16 +112,30 @@ handle_start (MetaDBusSysprof3Profiler *dbus_profiler,
 
   if (fd != -1)
     {
-      cogl_set_tracing_enabled_on_thread_with_fd (main_context,
-                                                  group_name,
-                                                  fd);
+      if (!cogl_start_tracing_with_fd (fd, &error))
+        {
+          g_dbus_method_invocation_return_error (invocation,
+                                                 G_DBUS_ERROR,
+                                                 G_DBUS_ERROR_FAILED,
+                                                 "Failed to start: %s",
+                                                 error->message);
+          return G_DBUS_METHOD_INVOCATION_HANDLED;
+        }
     }
   else
     {
-      cogl_set_tracing_enabled_on_thread (main_context,
-                                          group_name,
-                                          "mutter-profile.syscap");
+      if (!cogl_start_tracing_with_path ("mutter-profile.syscap", &error))
+        {
+          g_dbus_method_invocation_return_error (invocation,
+                                                 G_DBUS_ERROR,
+                                                 G_DBUS_ERROR_FAILED,
+                                                 "Failed to start: %s",
+                                                 error->message);
+          return G_DBUS_METHOD_INVOCATION_HANDLED;
         }
+    }
+
+  cogl_set_tracing_enabled_on_thread (main_context, group_name);
 
   g_mutex_lock (&profiler->mutex);
   for (l = profiler->threads; l; l = l->next)
@@ -131,18 +146,8 @@ handle_start (MetaDBusSysprof3Profiler *dbus_profiler,
       thread_group_name = g_strdup_printf ("%s (%s)",
                                            group_name,
                                            thread_info->name);
-      if (fd != -1)
-        {
-          cogl_set_tracing_enabled_on_thread_with_fd (thread_info->main_context,
-                                                      thread_group_name,
-                                                      fd);
-        }
-      else
-        {
       cogl_set_tracing_enabled_on_thread (thread_info->main_context,
-                                              thread_group_name,
-                                              NULL);
-        }
+                                          thread_group_name);
     }
   g_mutex_unlock (&profiler->mutex);
 
@@ -245,6 +250,9 @@ meta_profiler_finalize (GObject *object)
 {
   MetaProfiler *self = (MetaProfiler *)object;
 
+  if (self->persistent)
+    cogl_stop_tracing ();
+
   g_cancellable_cancel (self->cancellable);
 
   g_clear_object (&self->cancellable);
@@ -285,14 +293,22 @@ meta_profiler_new (const char *trace_file)
     {
       GMainContext *main_context = g_main_context_default ();
       const char *group_name;
+      g_autoptr (GError) error = NULL;
 
       /* Translators: this string will appear in Sysprof */
       group_name = _("Compositor");
 
-      cogl_set_tracing_enabled_on_thread (main_context,
-                                          group_name,
-                                          trace_file);
+      if (!cogl_start_tracing_with_path (trace_file, &error))
+        {
+          g_warning ("Failed to start persistent profiling: %s",
+                     error->message);
+        }
+      else
+        {
+          cogl_set_tracing_enabled_on_thread (main_context, group_name);
           profiler->persistent = TRUE;
+          profiler->running = TRUE;
+        }
     }
 
   return profiler;