From 2abc180fefbeb877829aefe9a6e38bec04cab0e0 Mon Sep 17 00:00:00 2001 From: Christian Hergert Date: Mon, 3 Jun 2019 14:08:47 -0700 Subject: [PATCH 1/4] profiler: port to sysprof-capture-3 Rather than embedding the old sysprof sources into the tree this change uses the static sysprof-capture-3 library when available. If --enable- profiler=auto, then the profiler will only be enabled if the sources are found. If the build is --enable-profiler=yes, the sysprof-capture-3 library must be available. Now that sysprof is maintaining an ABI, the function prefix was also changed to sysprof_. This patch also reflects that. Related to GNOME/Initiatives#10. --- Makefile.am | 4 +- configure.ac | 36 +- gjs-srcs.mk | 6 - gjs/profiler.cpp | 35 +- test/org.gnome.GjsDevel.json | 21 ++ util/sp-capture-types.h | 127 ------- util/sp-capture-writer.c | 628 ----------------------------------- util/sp-capture-writer.h | 81 ----- 8 files changed, 72 insertions(+), 866 deletions(-) delete mode 100644 util/sp-capture-types.h delete mode 100644 util/sp-capture-writer.c delete mode 100644 util/sp-capture-writer.h diff --git a/Makefile.am b/Makefile.am index 7882af0a8..0949f63f5 100644 --- a/Makefile.am +++ b/Makefile.am @@ -87,8 +87,8 @@ endif libgjs_la_SOURCES = $(gjs_srcs) if ENABLE_PROFILER -libgjs_la_SOURCES += $(gjs_sysprof_srcs) -libgjs_la_LIBADD += $(LIB_TIMER_TIME) +libgjs_la_CPPFLAGS += $(SYSPROF_CAPTURE_CFLAGS) +libgjs_la_LIBADD += $(LIB_TIMER_TIME) $(SYSPROF_CAPTURE_LIBS) endif # Also, these files used to be a separate library diff --git a/configure.ac b/configure.ac index 00269d745..ed4db9472 100644 --- a/configure.ac +++ b/configure.ac @@ -147,9 +147,29 @@ AS_IF([test x$have_gtk = xyes], [ ], [AS_IF([test "x$with_gtk" = "xyes"], [AC_MSG_ERROR([GTK requested but not found])])]) -# Some Linux APIs required for profiler AC_ARG_ENABLE([profiler], - [AS_HELP_STRING([--disable-profiler], [Don't build profiler])]) + [AS_HELP_STRING([--disable-profiler], [Don't build profiler])], + [], + [enable_profiler=auto]) + +# Use sysprof-capture-3 for profiler +PKG_CHECK_MODULES([SYSPROF_CAPTURE], + [sysprof-capture-3], + [have_sysprof_capture=yes], + [have_sysprof_capture=no]) +AC_SUBST([SYSPROF_CAPTURE_CFLAGS]) +AC_SUBST([SYSPROF_CAPTURE_LIBS]) + +AS_IF([test x$enable_profiler = xauto],[ + AS_IF([test $have_sysprof_capture = no],[enable_profiler=no]) +]) +AS_IF([test x$enable_profiler = xyes],[ + AS_IF([test $have_sysprof_capture = no],[ + AC_MSG_ERROR([--enable-profiler requires sysprof-capture-3.pc]) + ]) +]) + +# Some Linux APIs required for profiler AS_IF([test x$enable_profiler != xno], [ # Requires timer_settime() - only on Linux gl_TIMER_TIME @@ -163,11 +183,15 @@ AS_IF([test x$enable_profiler != xno], [ AC_MSG_RESULT([yes]) ], [AC_MSG_RESULT([no])]) AS_IF([test x$ac_cv_func_timer_settime = xno -o x$have_sigev_thread_id = xno], - [AC_MSG_ERROR([The profiler is currently only supported on Linux. + AS_IF([test x$enable_profiler = xyes], + [AC_MSG_ERROR([The profiler is currently only supported on Linux. The standard library must support timer_settime() and SIGEV_THREAD_ID. -Configure with --disable-profiler to skip it on other platforms.])]) - AC_DEFINE([ENABLE_PROFILER], [1], [Define if the profiler should be built.]) +Configure with --disable-profiler to skip it on other platforms.])], + [enable_profiler=no])]) + AS_IF([test x$enable_profiler = xauto],[enable_profiler=yes]) + AC_DEFINE([ENABLE_PROFILER], [test x$enable_profiler = xyes], [Define if the profiler should be built.]) ]) + AM_CONDITIONAL([ENABLE_PROFILER], [test x$enable_profiler != xno]) # Optional redline dep (enabled by default) @@ -388,7 +412,7 @@ AC_MSG_RESULT([ readline: ${ac_cv_header_readline_readline_h} dtrace: ${enable_dtrace:-no} systemtap: ${enable_systemtap:-no} - Profiler: ${enable_profiler:-yes} + Profiler: ${enable_profiler} Run tests under: ${TEST_MSG} Code coverage: ${enable_code_coverage} ]) diff --git a/gjs-srcs.mk b/gjs-srcs.mk index dcdc4387d..2fcdfb8fd 100644 --- a/gjs-srcs.mk +++ b/gjs-srcs.mk @@ -115,9 +115,3 @@ gjs_gtk_private_srcs = \ gjs_console_srcs = \ gjs/console.cpp \ $(NULL) - -gjs_sysprof_srcs = \ - util/sp-capture-types.h \ - util/sp-capture-writer.c \ - util/sp-capture-writer.h \ - $(NULL) diff --git a/gjs/profiler.cpp b/gjs/profiler.cpp index 6f37eaca5..b332b8598 100644 --- a/gjs/profiler.cpp +++ b/gjs/profiler.cpp @@ -47,7 +47,7 @@ # ifdef G_OS_UNIX # include # endif -# include "util/sp-capture-writer.h" +# include #endif /* @@ -95,7 +95,7 @@ struct _GjsProfiler { JSContext *cx; /* Buffers and writes our sampled stacks */ - SpCaptureWriter *capture; + SysprofCaptureWriter* capture; #endif /* ENABLE_PROFILER */ /* The filename to write to */ @@ -166,8 +166,8 @@ gjs_profiler_extract_maps(GjsProfiler *self) inode = 0; } - if (!sp_capture_writer_add_map(self->capture, now, -1, self->pid, start, - end, offset, inode, file)) + if (!sysprof_capture_writer_add_map(self->capture, now, -1, self->pid, + start, end, offset, inode, file)) return false; } @@ -247,7 +247,7 @@ _gjs_profiler_free(GjsProfiler *self) g_clear_pointer(&self->filename, g_free); #ifdef ENABLE_PROFILER - g_clear_pointer(&self->capture, sp_capture_writer_unref); + g_clear_pointer(&self->capture, sysprof_capture_writer_unref); #endif g_free(self); } @@ -304,7 +304,8 @@ gjs_profiler_sigprof(int signum, * here since we are in a signal handler. */ // cppcheck-suppress allocaCalled - SpCaptureAddress *addrs = static_cast(alloca(sizeof *addrs * depth)); + SysprofCaptureAddress* addrs = + static_cast(alloca(sizeof *addrs * depth)); for (uint32_t ix = 0; ix < depth; ix++) { js::ProfileEntry& entry = self->stack.entries[ix]; @@ -357,12 +358,14 @@ gjs_profiler_sigprof(int signum, * everything will show up as [stack] when building callgraphs. */ if (final_string[0] != '\0') - addrs[flipped] = sp_capture_writer_add_jitmap(self->capture, final_string); + addrs[flipped] = + sysprof_capture_writer_add_jitmap(self->capture, final_string); else - addrs[flipped] = SpCaptureAddress(entry.stackAddress()); + addrs[flipped] = SysprofCaptureAddress(entry.stackAddress()); } - if (!sp_capture_writer_add_sample(self->capture, now, -1, self->pid, addrs, depth)) + if (!sysprof_capture_writer_add_sample(self->capture, now, -1, self->pid, + -1, addrs, depth)) gjs_profiler_stop(self); } @@ -403,7 +406,7 @@ gjs_profiler_start(GjsProfiler *self) if (!path) path = g_strdup_printf("gjs-%jd.syscap", intmax_t(self->pid)); - self->capture = sp_capture_writer_new(path, 0); + self->capture = sysprof_capture_writer_new(path, 0); if (!self->capture) { g_warning("Failed to open profile capture"); @@ -412,7 +415,7 @@ gjs_profiler_start(GjsProfiler *self) if (!gjs_profiler_extract_maps(self)) { g_warning("Failed to extract proc maps"); - g_clear_pointer(&self->capture, sp_capture_writer_unref); + g_clear_pointer(&self->capture, sysprof_capture_writer_unref); return; } @@ -423,7 +426,7 @@ gjs_profiler_start(GjsProfiler *self) if (sigaction(SIGPROF, &sa, nullptr) == -1) { g_warning("Failed to register sigaction handler: %s", g_strerror(errno)); - g_clear_pointer(&self->capture, sp_capture_writer_unref); + g_clear_pointer(&self->capture, sysprof_capture_writer_unref); return; } @@ -443,7 +446,7 @@ gjs_profiler_start(GjsProfiler *self) if (timer_create(CLOCK_MONOTONIC, &sev, &self->timer) == -1) { g_warning("Failed to create profiler timer: %s", g_strerror(errno)); - g_clear_pointer(&self->capture, sp_capture_writer_unref); + g_clear_pointer(&self->capture, sysprof_capture_writer_unref); return; } @@ -457,7 +460,7 @@ gjs_profiler_start(GjsProfiler *self) if (timer_settime(self->timer, 0, &its, &old_its) != 0) { g_warning("Failed to enable profiler timer: %s", g_strerror(errno)); timer_delete(self->timer); - g_clear_pointer(&self->capture, sp_capture_writer_unref); + g_clear_pointer(&self->capture, sysprof_capture_writer_unref); return; } @@ -512,9 +515,9 @@ gjs_profiler_stop(GjsProfiler *self) js::EnableContextProfilingStack(self->cx, false); js::SetContextProfilingStack(self->cx, nullptr); - sp_capture_writer_flush(self->capture); + sysprof_capture_writer_flush(self->capture); - g_clear_pointer(&self->capture, sp_capture_writer_unref); + g_clear_pointer(&self->capture, sysprof_capture_writer_unref); g_message("Profiler stopped"); diff --git a/test/org.gnome.GjsDevel.json b/test/org.gnome.GjsDevel.json index 292ddfd80..659e6aee0 100644 --- a/test/org.gnome.GjsDevel.json +++ b/test/org.gnome.GjsDevel.json @@ -16,9 +16,30 @@ "--socket=pulseaudio" ], "modules": [ + { + "name": "sysprof", + "builddir": true, + "buildsystem": "meson", + "config-opts": [ + "-Denable_gtk=false", + "-Dhelp=false", + "-Dlibsysprof=false", + "-Dwith_sysprofd=none" + ], + "sources": [ + { + "type": "git", + "branch": "master", + "url": "https://gitlab.gnome.org/GNOME/sysprof.git" + } + ] + }, { "name": "gjs", "builddir": true, + "config-opts": [ + "--enable-profiler" + ], "sources": [ { "type": "git", diff --git a/util/sp-capture-types.h b/util/sp-capture-types.h deleted file mode 100644 index 7fd52c64d..000000000 --- a/util/sp-capture-types.h +++ /dev/null @@ -1,127 +0,0 @@ -/* sp-capture-types.h - * - * Copyright (C) 2016 Christian Hergert - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to - * deal in the Software without restriction, including without limitation the - * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or - * sell copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING - * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS - * IN THE SOFTWARE. - */ - -/* The original source of this file is: - * https://git.gnome.org/browse/sysprof/tree/lib/capture/sp-capture-types.c - * It has been modified to remove unneeded functionality. - */ - -#ifndef SP_CAPTURE_FORMAT_H -#define SP_CAPTURE_FORMAT_H - -#include -#include - -G_BEGIN_DECLS - -#define SP_CAPTURE_MAGIC (GUINT32_TO_LE(0xFDCA975E)) -#define SP_CAPTURE_ALIGN (sizeof(SpCaptureAddress)) - -#if __WORDSIZE == 64 -# define SP_CAPTURE_JITMAP_MARK G_GUINT64_CONSTANT(0xE000000000000000) -# define SP_CAPTURE_ADDRESS_FORMAT "0x%016lx" -#else -# define SP_CAPTURE_JITMAP_MARK G_GUINT64_CONSTANT(0xE0000000) -# define SP_CAPTURE_ADDRESS_FORMAT "0x%016llx" -#endif - -#define SP_CAPTURE_CURRENT_TIME (g_get_monotonic_time() * 1000L) - -typedef struct _SpCaptureWriter SpCaptureWriter; - -typedef guint64 SpCaptureAddress; - -typedef enum -{ - SP_CAPTURE_FRAME_TIMESTAMP = 1, - SP_CAPTURE_FRAME_SAMPLE = 2, - SP_CAPTURE_FRAME_MAP = 3, - SP_CAPTURE_FRAME_PROCESS = 4, - SP_CAPTURE_FRAME_FORK = 5, - SP_CAPTURE_FRAME_EXIT = 6, - SP_CAPTURE_FRAME_JITMAP = 7, - SP_CAPTURE_FRAME_CTRDEF = 8, - SP_CAPTURE_FRAME_CTRSET = 9, -} SpCaptureFrameType; - -#pragma pack(push, 1) - -typedef struct -{ - guint32 magic; - guint8 version; - guint32 little_endian : 1; - guint32 padding : 23; - gchar capture_time[64]; - gint64 time; - gint64 end_time; - gchar suffix[168]; -} SpCaptureFileHeader; - -typedef struct -{ - guint16 len; - gint16 cpu; - gint32 pid; - gint64 time; - guint8 type; - guint64 padding : 56; - guint8 data[0]; -} SpCaptureFrame; - -typedef struct -{ - SpCaptureFrame frame; - guint64 start; - guint64 end; - guint64 offset; - guint64 inode; - gchar filename[0]; -} SpCaptureMap; - -typedef struct -{ - SpCaptureFrame frame; - guint32 n_jitmaps; - guint8 data[0]; -} SpCaptureJitmap; - -typedef struct -{ - SpCaptureFrame frame; - guint16 n_addrs; - guint64 padding : 48; - SpCaptureAddress addrs[0]; -} SpCaptureSample; - -#pragma pack(pop) - -G_STATIC_ASSERT (sizeof (SpCaptureFileHeader) == 256); -G_STATIC_ASSERT (sizeof (SpCaptureFrame) == 24); -G_STATIC_ASSERT (sizeof (SpCaptureMap) == 56); -G_STATIC_ASSERT (sizeof (SpCaptureJitmap) == 28); -G_STATIC_ASSERT (sizeof (SpCaptureSample) == 32); - -G_END_DECLS - -#endif /* SP_CAPTURE_FORMAT_H */ diff --git a/util/sp-capture-writer.c b/util/sp-capture-writer.c deleted file mode 100644 index 5dc49972a..000000000 --- a/util/sp-capture-writer.c +++ /dev/null @@ -1,628 +0,0 @@ -/* sp-capture-writer.c - * - * Copyright (C) 2016 Christian Hergert - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to - * deal in the Software without restriction, including without limitation the - * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or - * sell copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING - * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS - * IN THE SOFTWARE. - */ - -/* The original source of this file is: - * https://git.gnome.org/browse/sysprof/tree/lib/capture/sp-capture-writer.c - * It has been modified to remove unneeded functionality. - */ - -#ifndef _GNU_SOURCE -# define _GNU_SOURCE -#endif - -#include -#include -#include -#include -#include -#include -#include -#include - -#include "sp-capture-writer.h" - -#define DEFAULT_BUFFER_SIZE (getpagesize() * 64L) -#define INVALID_ADDRESS (G_GUINT64_CONSTANT(0)) - -typedef struct -{ - /* A pinter into the string buffer */ - const gchar *str; - - /* The unique address for the string */ - guint64 addr; -} SpCaptureJitmapBucket; - -struct _SpCaptureWriter -{ - /* - * This is our buffer location for incoming strings. This is used - * similarly to GStringChunk except there is only one-page, and after - * it fills, we flush to disk. - * - * This is paired with a closed hash table for deduplication. - */ - gchar addr_buf[4096*4]; - - /* Our hashtable for deduplication. */ - SpCaptureJitmapBucket addr_hash[512]; - - /* We keep the large fields above so that our allocation will be page - * alinged for the write buffer. This improves the performance of large - * writes to the target file-descriptor. - */ - volatile gint ref_count; - - /* - * Our address sequence counter. The value that comes from - * monotonically increasing this is OR'd with JITMAP_MARK to denote - * the address name should come from the JIT map. - */ - gsize addr_seq; - - /* Our position in addr_buf. */ - gsize addr_buf_pos; - - /* - * The number of hash table items in @addr_hash. This is an - * optimization so that we can avoid calculating the number of strings - * when flushing out the jitmap. - */ - guint addr_hash_size; - - /* Capture file handle */ - int fd; - - /* Our write buffer for fd */ - guint8 *buf; - gsize pos; - gsize len; - - /* counter id sequence */ - gint next_counter_id; - - /* Statistics while recording */ - SpCaptureStat stat; -}; - -G_DEFINE_BOXED_TYPE (SpCaptureWriter, sp_capture_writer, - sp_capture_writer_ref, sp_capture_writer_unref) - -static inline void -sp_capture_writer_frame_init (SpCaptureFrame *frame_, - gint len, - gint cpu, - GPid pid, - gint64 time_, - SpCaptureFrameType type) -{ - g_assert (frame_ != NULL); - - frame_->len = len; - frame_->cpu = cpu; - frame_->pid = pid; - frame_->time = time_; - frame_->type = type; - frame_->padding = 0; -} - -static void -sp_capture_writer_finalize (SpCaptureWriter *self) -{ - if (self != NULL) - { - sp_capture_writer_flush (self); - close (self->fd); - g_free (self->buf); - g_free (self); - } -} - -SpCaptureWriter * -sp_capture_writer_ref (SpCaptureWriter *self) -{ - g_assert (self != NULL); - g_assert (self->ref_count > 0); - - g_atomic_int_inc (&self->ref_count); - - return self; -} - -void -sp_capture_writer_unref (SpCaptureWriter *self) -{ - g_assert (self != NULL); - g_assert (self->ref_count > 0); - - if (g_atomic_int_dec_and_test (&self->ref_count)) - sp_capture_writer_finalize (self); -} - -static gboolean -sp_capture_writer_flush_data (SpCaptureWriter *self) -{ - const guint8 *buf; - gssize written; - gsize to_write; - - g_assert (self != NULL); - g_assert (self->pos <= self->len); - g_assert ((self->pos % SP_CAPTURE_ALIGN) == 0); - - buf = self->buf; - to_write = self->pos; - - while (to_write > 0) - { - written = write (self->fd, buf, to_write); - if (written < 0) - return FALSE; - - if (written == 0 && errno != EAGAIN) - return FALSE; - - g_assert (written <= (gssize)to_write); - - buf += written; - to_write -= written; - } - - self->pos = 0; - - return TRUE; -} - -static inline void -sp_capture_writer_realign (gsize *pos) -{ - *pos = (*pos + SP_CAPTURE_ALIGN - 1) & ~(SP_CAPTURE_ALIGN - 1); -} - -static inline gboolean -sp_capture_writer_ensure_space_for (SpCaptureWriter *self, - gsize len) -{ - /* Check for max frame size */ - if (len > G_MAXUSHORT) - return FALSE; - - if ((self->len - self->pos) < len) - { - if (!sp_capture_writer_flush_data (self)) - return FALSE; - } - - return TRUE; -} - -static inline gpointer -sp_capture_writer_allocate (SpCaptureWriter *self, - gsize *len) -{ - gpointer p; - - g_assert (self != NULL); - g_assert (len != NULL); - g_assert ((self->pos % SP_CAPTURE_ALIGN) == 0); - - sp_capture_writer_realign (len); - - if (!sp_capture_writer_ensure_space_for (self, *len)) - return NULL; - - p = (gpointer)&self->buf[self->pos]; - - self->pos += *len; - - g_assert ((self->pos % SP_CAPTURE_ALIGN) == 0); - - return p; -} - -static gboolean -sp_capture_writer_flush_jitmap (SpCaptureWriter *self) -{ - SpCaptureJitmap jitmap; - gssize r; - gsize len; - - g_assert (self != NULL); - - if (self->addr_hash_size == 0) - return TRUE; - - g_assert (self->addr_buf_pos > 0); - - len = sizeof jitmap + self->addr_buf_pos; - - sp_capture_writer_realign (&len); - - sp_capture_writer_frame_init (&jitmap.frame, - len, - -1, - getpid (), - SP_CAPTURE_CURRENT_TIME, - SP_CAPTURE_FRAME_JITMAP); - jitmap.n_jitmaps = self->addr_hash_size; - - if (sizeof jitmap != write (self->fd, &jitmap, sizeof jitmap)) - return FALSE; - - r = write (self->fd, self->addr_buf, len - sizeof jitmap); - if (r < 0 || (gsize)r != (len - sizeof jitmap)) - return FALSE; - - self->addr_buf_pos = 0; - self->addr_hash_size = 0; - memset (self->addr_hash, 0, sizeof self->addr_hash); - - self->stat.frame_count[SP_CAPTURE_FRAME_JITMAP]++; - - return TRUE; -} - -static gboolean -sp_capture_writer_lookup_jitmap (SpCaptureWriter *self, - const gchar *name, - SpCaptureAddress *addr) -{ - guint hash; - guint i; - - g_assert (self != NULL); - g_assert (name != NULL); - g_assert (addr != NULL); - - hash = g_str_hash (name) % G_N_ELEMENTS (self->addr_hash); - - for (i = hash; i < G_N_ELEMENTS (self->addr_hash); i++) - { - SpCaptureJitmapBucket *bucket = &self->addr_hash[i]; - - if (bucket->str == NULL) - return FALSE; - - if (strcmp (bucket->str, name) == 0) - { - *addr = bucket->addr; - return TRUE; - } - } - - for (i = 0; i < hash; i++) - { - SpCaptureJitmapBucket *bucket = &self->addr_hash[i]; - - if (bucket->str == NULL) - return FALSE; - - if (strcmp (bucket->str, name) == 0) - { - *addr = bucket->addr; - return TRUE; - } - } - - return FALSE; -} - -static SpCaptureAddress -sp_capture_writer_insert_jitmap (SpCaptureWriter *self, - const gchar *str) -{ - SpCaptureAddress addr; - gchar *dst; - gsize len; - guint hash; - guint i; - - g_assert (self != NULL); - g_assert (str != NULL); - g_assert ((self->pos % SP_CAPTURE_ALIGN) == 0); - - len = sizeof addr + strlen (str) + 1; - - if ((self->addr_hash_size == G_N_ELEMENTS (self->addr_hash)) || - ((sizeof self->addr_buf - self->addr_buf_pos) < len)) - { - if (!sp_capture_writer_flush_jitmap (self)) - return INVALID_ADDRESS; - - g_assert (self->addr_hash_size == 0); - g_assert (self->addr_buf_pos == 0); - } - - g_assert (self->addr_hash_size < G_N_ELEMENTS (self->addr_hash)); - g_assert (len > sizeof addr); - - /* Allocate the next unique address */ - addr = SP_CAPTURE_JITMAP_MARK | ++self->addr_seq; - - /* Copy the address into the buffer */ - dst = (gchar *)&self->addr_buf[self->addr_buf_pos]; - memcpy (dst, &addr, sizeof addr); - - /* - * Copy the string into the buffer, keeping dst around for - * when we insert into the hashtable. - */ - dst += sizeof addr; - memcpy (dst, str, len - sizeof addr); - - /* Advance our string cache position */ - self->addr_buf_pos += len; - g_assert (self->addr_buf_pos <= sizeof self->addr_buf); - - /* Now place the address into the hashtable */ - hash = g_str_hash (str) % G_N_ELEMENTS (self->addr_hash); - - /* Start from the current hash bucket and go forward */ - for (i = hash; i < G_N_ELEMENTS (self->addr_hash); i++) - { - SpCaptureJitmapBucket *bucket = &self->addr_hash[i]; - - if (G_LIKELY (bucket->str == NULL)) - { - bucket->str = dst; - bucket->addr = addr; - self->addr_hash_size++; - return addr; - } - } - - /* Wrap around to the beginning */ - for (i = 0; i < hash; i++) - { - SpCaptureJitmapBucket *bucket = &self->addr_hash[i]; - - if (G_LIKELY (bucket->str == NULL)) - { - bucket->str = dst; - bucket->addr = addr; - self->addr_hash_size++; - return addr; - } - } - - g_assert_not_reached (); - - return INVALID_ADDRESS; -} - -SpCaptureWriter * -sp_capture_writer_new_from_fd (int fd, - gsize buffer_size) -{ - g_autofree gchar *nowstr = NULL; - SpCaptureWriter *self; - SpCaptureFileHeader *header; - GTimeVal tv; - gsize header_len = sizeof(*header); - - if (buffer_size == 0) - buffer_size = DEFAULT_BUFFER_SIZE; - - g_assert (fd != -1); - g_assert (buffer_size % getpagesize() == 0); - - if (ftruncate (fd, 0) != 0) - return NULL; - - self = g_new0 (SpCaptureWriter, 1); - self->ref_count = 1; - self->fd = fd; - self->buf = (guint8 *)g_malloc0 (buffer_size); - self->len = buffer_size; - self->next_counter_id = 1; - - g_get_current_time (&tv); - nowstr = g_time_val_to_iso8601 (&tv); - - header = sp_capture_writer_allocate (self, &header_len); - - if (header == NULL) - { - sp_capture_writer_finalize (self); - return NULL; - } - - header->magic = SP_CAPTURE_MAGIC; - header->version = 1; -#ifdef G_LITTLE_ENDIAN - header->little_endian = TRUE; -#else - header->little_endian = FALSE; -#endif - header->padding = 0; - g_strlcpy (header->capture_time, nowstr, sizeof header->capture_time); - header->time = SP_CAPTURE_CURRENT_TIME; - header->end_time = 0; - memset (header->suffix, 0, sizeof header->suffix); - - if (!sp_capture_writer_flush_data (self)) - { - sp_capture_writer_finalize (self); - return NULL; - } - - g_assert (self->pos == 0); - g_assert (self->len > 0); - g_assert (self->len % getpagesize() == 0); - g_assert (self->buf != NULL); - g_assert (self->addr_hash_size == 0); - g_assert (self->fd != -1); - - return self; -} - -SpCaptureWriter * -sp_capture_writer_new (const gchar *filename, - gsize buffer_size) -{ - SpCaptureWriter *self; - int fd; - - g_assert (filename != NULL); - g_assert (buffer_size % getpagesize() == 0); - - if ((-1 == (fd = open (filename, O_CREAT | O_RDWR, 0640))) || - (-1 == ftruncate (fd, 0L))) - return NULL; - - self = sp_capture_writer_new_from_fd (fd, buffer_size); - - if (self == NULL) - close (fd); - - return self; -} - -gboolean -sp_capture_writer_add_map (SpCaptureWriter *self, - gint64 time, - gint cpu, - GPid pid, - guint64 start, - guint64 end, - guint64 offset, - guint64 inode, - const gchar *filename) -{ - SpCaptureMap *ev; - gsize len; - - if (filename == NULL) - filename = ""; - - g_assert (self != NULL); - g_assert (filename != NULL); - - len = sizeof *ev + strlen (filename) + 1; - - ev = (SpCaptureMap *)sp_capture_writer_allocate (self, &len); - if (!ev) - return FALSE; - - sp_capture_writer_frame_init (&ev->frame, - len, - cpu, - pid, - time, - SP_CAPTURE_FRAME_MAP); - ev->start = start; - ev->end = end; - ev->offset = offset; - ev->inode = inode; - - g_strlcpy (ev->filename, filename, len - sizeof *ev); - ev->filename[len - sizeof *ev - 1] = '\0'; - - self->stat.frame_count[SP_CAPTURE_FRAME_MAP]++; - - return TRUE; -} - -SpCaptureAddress -sp_capture_writer_add_jitmap (SpCaptureWriter *self, - const gchar *name) -{ - SpCaptureAddress addr = INVALID_ADDRESS; - - if (name == NULL) - name = ""; - - g_assert (self != NULL); - g_assert (name != NULL); - - if (!sp_capture_writer_lookup_jitmap (self, name, &addr)) - addr = sp_capture_writer_insert_jitmap (self, name); - - return addr; -} - -gboolean -sp_capture_writer_add_sample (SpCaptureWriter *self, - gint64 time, - gint cpu, - GPid pid, - const SpCaptureAddress *addrs, - guint n_addrs) -{ - SpCaptureSample *ev; - gsize len; - - g_assert (self != NULL); - - len = sizeof *ev + (n_addrs * sizeof (SpCaptureAddress)); - - ev = (SpCaptureSample *)sp_capture_writer_allocate (self, &len); - if (!ev) - return FALSE; - - sp_capture_writer_frame_init (&ev->frame, - len, - cpu, - pid, - time, - SP_CAPTURE_FRAME_SAMPLE); - ev->n_addrs = n_addrs; - - memcpy (ev->addrs, addrs, (n_addrs * sizeof (SpCaptureAddress))); - - self->stat.frame_count[SP_CAPTURE_FRAME_SAMPLE]++; - - return TRUE; -} - -static gboolean -sp_capture_writer_flush_end_time (SpCaptureWriter *self) -{ - gint64 end_time = SP_CAPTURE_CURRENT_TIME; - ssize_t ret; - - g_assert (self != NULL); - - /* This field is opportunistic, so a failure is okay. */ - -again: - ret = pwrite (self->fd, - &end_time, - sizeof (end_time), - G_STRUCT_OFFSET (SpCaptureFileHeader, end_time)); - - if (ret < 0 && errno == EAGAIN) - goto again; - - return TRUE; -} - -gboolean -sp_capture_writer_flush (SpCaptureWriter *self) -{ - g_assert (self != NULL); - - return (sp_capture_writer_flush_jitmap (self) && - sp_capture_writer_flush_data (self) && - sp_capture_writer_flush_end_time (self)); -} diff --git a/util/sp-capture-writer.h b/util/sp-capture-writer.h deleted file mode 100644 index c54501320..000000000 --- a/util/sp-capture-writer.h +++ /dev/null @@ -1,81 +0,0 @@ -/* sp-capture-writer.h - * - * Copyright (C) 2016 Christian Hergert - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to - * deal in the Software without restriction, including without limitation the - * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or - * sell copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING - * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS - * IN THE SOFTWARE. - */ - -/* The original source of this file is: - * https://git.gnome.org/browse/sysprof/tree/lib/capture/sp-capture-writer.h - * It has been modified to remove unneeded functionality. - */ - -#ifndef SP_CAPTURE_WRITER_H -#define SP_CAPTURE_WRITER_H - -#include "sp-capture-types.h" - -G_BEGIN_DECLS - -typedef struct _SpCaptureWriter SpCaptureWriter; - -typedef struct -{ - /* - * The number of frames indexed by SpCaptureFrameType - */ - gsize frame_count[16]; - - /* - * Padding for future expansion. - */ - gsize padding[48]; -} SpCaptureStat; - -SpCaptureWriter *sp_capture_writer_new (const gchar *filename, - gsize buffer_size); -SpCaptureWriter *sp_capture_writer_new_from_fd (int fd, - gsize buffer_size); -SpCaptureWriter *sp_capture_writer_ref (SpCaptureWriter *self); -void sp_capture_writer_unref (SpCaptureWriter *self); -gboolean sp_capture_writer_add_map (SpCaptureWriter *self, - gint64 time, - gint cpu, - GPid pid, - guint64 start, - guint64 end, - guint64 offset, - guint64 inode, - const gchar *filename); -guint64 sp_capture_writer_add_jitmap (SpCaptureWriter *self, - const gchar *name); -gboolean sp_capture_writer_add_sample (SpCaptureWriter *self, - gint64 time, - gint cpu, - GPid pid, - const SpCaptureAddress *addrs, - guint n_addrs); -gboolean sp_capture_writer_flush (SpCaptureWriter *self); - -#define SP_TYPE_CAPTURE_WRITER (sp_capture_writer_get_type()) -GType sp_capture_writer_get_type (void); - -G_END_DECLS - -#endif /* SP_CAPTURE_WRITER_H */ -- GitLab From c5ae557f0c67cb69ad300ca74d9274e7ed924aa7 Mon Sep 17 00:00:00 2001 From: Christian Hergert Date: Mon, 3 Jun 2019 14:09:44 -0700 Subject: [PATCH 2/4] profiler: record duration of GC sweeps Now that we are using sysprof-capture-3, we can record marks from the various subsystems in GJS. This adds marks to the capture when we perform a GC sweep operation. Related to GNOME/Initiatives#10 --- gjs/context-private.h | 5 ++++- gjs/context.cpp | 20 ++++++++++++++++++++ gjs/profiler-private.h | 4 ++++ gjs/profiler.cpp | 15 +++++++++++++++ 4 files changed, 43 insertions(+), 1 deletion(-) diff --git a/gjs/context-private.h b/gjs/context-private.h index 10dcf6884..274352fa2 100644 --- a/gjs/context-private.h +++ b/gjs/context-private.h @@ -117,6 +117,8 @@ class GjsContextPrivate { bool m_should_profile : 1; bool m_should_listen_sigusr2 : 1; + int64_t m_sweep_begin_time; + void schedule_gc_internal(bool force_gc); static gboolean trigger_gc_if_needed(void* data); static gboolean drain_job_queue_idle_handler(void* data); @@ -148,7 +150,6 @@ class GjsContextPrivate { GJS_USE const GjsAtoms& atoms(void) const { return m_atoms; } GJS_USE bool destroying(void) const { return m_destroying; } GJS_USE bool sweeping(void) const { return m_in_gc_sweep; } - void set_sweeping(bool value) { m_in_gc_sweep = value; } GJS_USE const char* program_name(void) const { return m_program_name; } void set_program_name(char* value) { m_program_name = value; } void set_search_path(char** value) { m_search_path = value; } @@ -194,6 +195,8 @@ class GjsContextPrivate { void register_unhandled_promise_rejection(uint64_t id, GjsAutoChar&& stack); void unregister_unhandled_promise_rejection(uint64_t id); + void set_sweeping(bool value); + static void trace(JSTracer* trc, void* data); void free_profiler(void); diff --git a/gjs/context.cpp b/gjs/context.cpp index 0ac7fb68f..6e2d54af4 100644 --- a/gjs/context.cpp +++ b/gjs/context.cpp @@ -609,6 +609,26 @@ void GjsContextPrivate::schedule_gc_if_needed(void) { schedule_gc_internal(false); } +void GjsContextPrivate::set_sweeping(bool value) { + // If we have a profiler enabled, record the duration of GC sweep + if (this->m_profiler != nullptr) { + int64_t now = g_get_monotonic_time() * 1000L; + + if (value) { + m_sweep_begin_time = now; + } else { + if (m_sweep_begin_time != 0) { + _gjs_profiler_add_mark(this->m_profiler, m_sweep_begin_time, + now - m_sweep_begin_time, "GJS", "Sweep", + nullptr); + m_sweep_begin_time = 0; + } + } + } + + m_in_gc_sweep = value; +} + void GjsContextPrivate::exit(uint8_t exit_code) { g_assert(!m_should_exit); m_should_exit = true; diff --git a/gjs/profiler-private.h b/gjs/profiler-private.h index 20cb9f467..0697be25b 100644 --- a/gjs/profiler-private.h +++ b/gjs/profiler-private.h @@ -33,6 +33,10 @@ G_BEGIN_DECLS GjsProfiler *_gjs_profiler_new(GjsContext *context); void _gjs_profiler_free(GjsProfiler *self); +void _gjs_profiler_add_mark(GjsProfiler* self, gint64 time, gint64 duration, + const char* group, const char* name, + const char* message); + GJS_USE bool _gjs_profiler_is_running(GjsProfiler *self); diff --git a/gjs/profiler.cpp b/gjs/profiler.cpp index b332b8598..3f35aabb0 100644 --- a/gjs/profiler.cpp +++ b/gjs/profiler.cpp @@ -639,3 +639,18 @@ gjs_profiler_set_filename(GjsProfiler *self, g_free(self->filename); self->filename = g_strdup(filename); } + +void _gjs_profiler_add_mark(GjsProfiler* self, gint64 time_nsec, + gint64 duration_nsec, const char* group, + const char* name, const char* message) { + g_return_if_fail(self); + g_return_if_fail(group); + g_return_if_fail(name); + +#ifdef ENABLE_PROFILER + if (self->running && self->capture != nullptr) { + sysprof_capture_writer_add_mark(self->capture, time_nsec, -1, self->pid, + duration_nsec, group, name, message); + } +#endif +} -- GitLab From 20854ba3c2acd45c3a3083ceee21108659388b6c Mon Sep 17 00:00:00 2001 From: Christian Hergert Date: Mon, 3 Jun 2019 14:10:26 -0700 Subject: [PATCH 3/4] profiler: add support for GJS_TRACE_FD This commit adds support for a special environment variable GJS_TRACE_FD. The console will, in response to this environment variable, enable tracing of the GJS context and deliver the output to the provided file- descriptor. In the upcoming Sysprof-3 branch, this can be used to automatically sample some GJS-based applications. Applications that use libgjs from a C-based main() will need to implement the g_getenv() GJS_TRACE_FD manually. --- gjs/console.cpp | 20 ++++++++++++++++++++ gjs/profiler.cpp | 34 ++++++++++++++++++++++++++++++---- gjs/profiler.h | 2 ++ test/gjs-tests.cpp | 1 + 4 files changed, 53 insertions(+), 4 deletions(-) diff --git a/gjs/console.cpp b/gjs/console.cpp index 4f8761539..2f813c7ca 100644 --- a/gjs/console.cpp +++ b/gjs/console.cpp @@ -281,10 +281,21 @@ main(int argc, char **argv) /* This should be removed after a suitable time has passed */ check_script_args_for_stray_gjs_args(script_argc, script_argv); + /* Check for GJS_TRACE_FD for sysprof profiling */ + const char* env_tracefd = g_getenv("GJS_TRACE_FD"); + int tracefd = -1; + if (env_tracefd) { + tracefd = g_ascii_strtoll(env_tracefd, nullptr, 10); + g_setenv("GJS_TRACE_FD", "", true); + if (tracefd > 0) + enable_profiler = true; + } + if (interactive_mode && enable_profiler) { g_message("Profiler disabled in interactive mode."); enable_profiler = false; g_unsetenv("GJS_ENABLE_PROFILER"); /* ignore env var in eval() */ + g_unsetenv("GJS_TRACE_FD"); /* ignore env var in eval() */ } js_context = (GjsContext*) g_object_new(GJS_TYPE_CONTEXT, @@ -318,6 +329,15 @@ main(int argc, char **argv) if (enable_profiler && profile_output_path) { GjsProfiler *profiler = gjs_context_get_profiler(js_context); gjs_profiler_set_filename(profiler, profile_output_path); + } else if (enable_profiler && tracefd > -1) { + GjsProfiler* profiler = gjs_context_get_profiler(js_context); + gjs_profiler_set_fd(profiler, tracefd); + tracefd = -1; + } + + if (tracefd != -1) { + close(tracefd); + tracefd = -1; } /* prepare command line arguments */ diff --git a/gjs/profiler.cpp b/gjs/profiler.cpp index 3f35aabb0..a85223174 100644 --- a/gjs/profiler.cpp +++ b/gjs/profiler.cpp @@ -101,6 +101,9 @@ struct _GjsProfiler { /* The filename to write to */ char *filename; + /* An FD to capture to */ + int fd; + #ifdef ENABLE_PROFILER /* Our POSIX timer to wakeup SIGPROF */ timer_t timer; @@ -219,6 +222,7 @@ _gjs_profiler_new(GjsContext *context) self->cx = static_cast(gjs_context_get_native_context(context)); self->pid = getpid(); #endif + self->fd = -1; profiling_context = context; @@ -248,6 +252,9 @@ _gjs_profiler_free(GjsProfiler *self) g_clear_pointer(&self->filename, g_free); #ifdef ENABLE_PROFILER g_clear_pointer(&self->capture, sysprof_capture_writer_unref); + + if (self->fd != -1) + close(self->fd); #endif g_free(self); } @@ -402,11 +409,16 @@ gjs_profiler_start(GjsProfiler *self) struct itimerspec its = { 0 }; struct itimerspec old_its; - GjsAutoChar path = g_strdup(self->filename); - if (!path) - path = g_strdup_printf("gjs-%jd.syscap", intmax_t(self->pid)); + if (self->fd != -1) { + self->capture = sysprof_capture_writer_new_from_fd(self->fd, 0); + self->fd = -1; + } else { + GjsAutoChar path = g_strdup(self->filename); + if (!path) + path = g_strdup_printf("gjs-%jd.syscap", intmax_t(self->pid)); - self->capture = sysprof_capture_writer_new(path, 0); + self->capture = sysprof_capture_writer_new(path, 0); + } if (!self->capture) { g_warning("Failed to open profile capture"); @@ -654,3 +666,17 @@ void _gjs_profiler_add_mark(GjsProfiler* self, gint64 time_nsec, } #endif } + +void gjs_profiler_set_fd(GjsProfiler* self, int fd) { + g_return_if_fail(self); + g_return_if_fail(!self->filename); + g_return_if_fail(!self->running); + +#ifdef ENABLE_PROFILER + if (self->fd != fd) { + if (self->fd != -1) + close(self->fd); + self->fd = fd; + } +#endif +} diff --git a/gjs/profiler.h b/gjs/profiler.h index 8bebed253..95615bd4b 100644 --- a/gjs/profiler.h +++ b/gjs/profiler.h @@ -40,6 +40,8 @@ GType gjs_profiler_get_type(void); GJS_EXPORT void gjs_profiler_set_filename(GjsProfiler *self, const char *filename); +GJS_EXPORT +void gjs_profiler_set_fd(GjsProfiler* self, int fd); GJS_EXPORT void gjs_profiler_start(GjsProfiler *self); diff --git a/test/gjs-tests.cpp b/test/gjs-tests.cpp index d438b6150..4ab3f8d69 100644 --- a/test/gjs-tests.cpp +++ b/test/gjs-tests.cpp @@ -391,6 +391,7 @@ main(int argc, { /* Avoid interference in the tests from stray environment variable */ g_unsetenv("GJS_ENABLE_PROFILER"); + g_unsetenv("GJS_TRACE_FD"); g_test_init(&argc, &argv, NULL); -- GitLab From 0218b1232da179adf230b5bd09dcfdc2baf7f1d5 Mon Sep 17 00:00:00 2001 From: Christian Hergert Date: Wed, 5 Jun 2019 16:22:02 -0700 Subject: [PATCH 4/4] profiler: auto-flush capture writer every 3 seconds To reduce the amount of data that could be lost in the buffer during a terminating signal scenario, GJS can use the auto-flush feature of SysprofCaptureWriter. Related to GNOME/Initiatives#10 --- gjs/profiler.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/gjs/profiler.cpp b/gjs/profiler.cpp index a85223174..c28080ea8 100644 --- a/gjs/profiler.cpp +++ b/gjs/profiler.cpp @@ -50,6 +50,8 @@ # include #endif +#define FLUSH_DELAY_SECONDS 3 + /* * This is mostly non-exciting code wrapping the builtin Profiler in * mozjs. In particular, the profiler consumer is required to "bring your @@ -425,6 +427,11 @@ gjs_profiler_start(GjsProfiler *self) return; } + /* Automatically flush to be resilient against SIGINT, etc */ + sysprof_capture_writer_set_flush_delay(self->capture, + g_main_context_get_thread_default(), + FLUSH_DELAY_SECONDS); + if (!gjs_profiler_extract_maps(self)) { g_warning("Failed to extract proc maps"); g_clear_pointer(&self->capture, sysprof_capture_writer_unref); -- GitLab