From 4250abf81e5665c5e9f811ce3633b002c4d30fa7 Mon Sep 17 00:00:00 2001 From: Christian Hergert Date: Mon, 7 Aug 2023 12:13:28 -0700 Subject: [PATCH] libsysprof: setup perf streams in prepare This starts the perf streams from prepare instead of from record so that we can do the linux instrument work in prepare. The samples are dropped until our start-time is set. Doing it this way removes sysprof-cli and sysprofd greatly from the overhead in the callgraph which is useful so that the user gets to see what they really care about. It has the added benefit that we're less likely to see the pkla processes showing up from authorizing our D-Bus connection for creating per streams. --- src/libsysprof/sysprof-linux-instrument.c | 40 ++++------------------- src/libsysprof/sysprof-sampler.c | 31 ++++++++++-------- 2 files changed, 25 insertions(+), 46 deletions(-) diff --git a/src/libsysprof/sysprof-linux-instrument.c b/src/libsysprof/sysprof-linux-instrument.c index 046e7630..ef2f3b6a 100644 --- a/src/libsysprof/sysprof-linux-instrument.c +++ b/src/libsysprof/sysprof-linux-instrument.c @@ -234,7 +234,11 @@ static DexFuture * sysprof_linux_instrument_prepare_fiber (gpointer user_data) { SysprofRecording *recording = user_data; + g_autoptr(GDBusConnection) bus = NULL; + g_autoptr(GVariant) process_info_reply = NULL; + g_autoptr(GVariant) process_info = NULL; g_autoptr(GError) error = NULL; + gint64 at_time; g_assert (SYSPROF_IS_RECORDING (recording)); @@ -247,34 +251,6 @@ sysprof_linux_instrument_prepare_fiber (gpointer user_data) &error)) return dex_future_new_for_error (g_steal_pointer (&error)); - return dex_future_new_for_boolean (TRUE); -} - -static DexFuture * -sysprof_linux_instrument_prepare (SysprofInstrument *instrument, - SysprofRecording *recording) -{ - g_assert (SYSPROF_IS_INSTRUMENT (instrument)); - g_assert (SYSPROF_IS_RECORDING (recording)); - - return dex_scheduler_spawn (NULL, 0, - sysprof_linux_instrument_prepare_fiber, - g_object_ref (recording), - g_object_unref); -} - -static DexFuture * -sysprof_linux_instrument_record_fiber (gpointer user_data) -{ - SysprofRecording *recording = user_data; - g_autoptr(GDBusConnection) bus = NULL; - g_autoptr(GVariant) process_info_reply = NULL; - g_autoptr(GVariant) process_info = NULL; - g_autoptr(GError) error = NULL; - gint64 at_time; - - g_assert (SYSPROF_IS_RECORDING (recording)); - /* We need access to the bus to call various sysprofd API directly */ if (!(bus = dex_await_object (dex_bus_get (G_BUS_TYPE_SYSTEM), &error))) return dex_future_new_for_error (g_steal_pointer (&error)); @@ -303,15 +279,14 @@ sysprof_linux_instrument_record_fiber (gpointer user_data) } static DexFuture * -sysprof_linux_instrument_record (SysprofInstrument *instrument, - SysprofRecording *recording, - GCancellable *cancellable) +sysprof_linux_instrument_prepare (SysprofInstrument *instrument, + SysprofRecording *recording) { g_assert (SYSPROF_IS_INSTRUMENT (instrument)); g_assert (SYSPROF_IS_RECORDING (recording)); return dex_scheduler_spawn (NULL, 0, - sysprof_linux_instrument_record_fiber, + sysprof_linux_instrument_prepare_fiber, g_object_ref (recording), g_object_unref); } @@ -341,7 +316,6 @@ sysprof_linux_instrument_class_init (SysprofLinuxInstrumentClass *klass) instrument_class->list_required_policy = sysprof_linux_instrument_list_required_policy; instrument_class->prepare = sysprof_linux_instrument_prepare; - instrument_class->record = sysprof_linux_instrument_record; instrument_class->process_started = sysprof_linux_instrument_process_started; } diff --git a/src/libsysprof/sysprof-sampler.c b/src/libsysprof/sysprof-sampler.c index 4a906dbe..6948cfa6 100644 --- a/src/libsysprof/sysprof-sampler.c +++ b/src/libsysprof/sysprof-sampler.c @@ -218,7 +218,8 @@ sysprof_sampler_perf_event_stream_cb (const SysprofPerfEvent *event, break; case PERF_RECORD_SAMPLE: - sysprof_sampler_add_callback (writer, cpu, &event->callchain); + if (recording->start_time != 0 && event->callchain.time >= recording->start_time) + sysprof_sampler_add_callback (writer, cpu, &event->callchain); break; case PERF_RECORD_THROTTLE: @@ -382,6 +383,22 @@ try_again: g_ptr_array_add (prepare->sampler->perf_event_streams, g_steal_pointer (&stream)); } + /* Start all of the samplers immediately as we will drop events that + * are incoming before our start time. This allows the linux instrument + * to collect processes in prepare and not race to get new process info. + */ + for (guint i = 0; i < prepare->sampler->perf_event_streams->len; i++) + { + SysprofPerfEventStream *stream = g_ptr_array_index (prepare->sampler->perf_event_streams, i); + + if (!sysprof_perf_event_stream_enable (stream, &error)) + g_debug ("%s", error->message); + else + g_debug ("Sampler %d enabled", i); + + g_clear_error (&error); + } + return dex_future_new_for_boolean (TRUE); } @@ -432,18 +449,6 @@ sysprof_sampler_record_fiber (gpointer user_data) g_assert (SYSPROF_IS_RECORDING (record->recording)); g_assert (DEX_IS_FUTURE (record->cancellable)); - for (guint i = 0; i < record->sampler->perf_event_streams->len; i++) - { - SysprofPerfEventStream *stream = g_ptr_array_index (record->sampler->perf_event_streams, i); - - if (!sysprof_perf_event_stream_enable (stream, &error)) - g_debug ("%s", error->message); - else - g_debug ("Sampler %d enabled", i); - - g_clear_error (&error); - } - if (!dex_await (dex_ref (record->cancellable), &error)) g_debug ("Sampler shutting down for reason: %s", error->message);