From 6a45f020f7e5f8d6a442028869a1e51a689c26fc Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 2 Jul 2020 12:43:15 +0100 Subject: [PATCH] libsysprof-capture: Add SysprofCaptureJitmapIter to replace GHashTable Change `sysprof_capture_reader_read_jitmap()` to return a `const SysprofCaptureJitmap *` (like the other `read` functions), and add a new `SysprofCaptureJitmapIter` type to allow easy iteration over the jitmap. This allows a use of `GHashTable` to be removed from the API. It breaks the libsysprof-capture API and ABI. All the callers iterate over the jitmap rather than looking up elements by key. If that functionality is needed in future, additional API can be added to allow it on `SysprofCaptureJitmap`. Signed-off-by: Philip Withnall Helps: #40 --- .../sysprof-capture-reader.c | 65 +++++++++++++++---- .../sysprof-capture-reader.h | 18 ++++- .../sysprof-capture-writer-cat.c | 10 +-- .../sysprof-jitmap-symbol-resolver.c | 8 +-- src/tests/test-capture.c | 27 ++++---- src/tools/sysprof-dump.c | 10 +-- 6 files changed, 99 insertions(+), 39 deletions(-) diff --git a/src/libsysprof-capture/sysprof-capture-reader.c b/src/libsysprof-capture/sysprof-capture-reader.c index 1166ae58..cfa42a6c 100644 --- a/src/libsysprof-capture/sysprof-capture-reader.c +++ b/src/libsysprof-capture/sysprof-capture-reader.c @@ -758,10 +758,9 @@ sysprof_capture_reader_read_process (SysprofCaptureReader *self) return process; } -GHashTable * +const SysprofCaptureJitmap * sysprof_capture_reader_read_jitmap (SysprofCaptureReader *self) { - g_autoptr(GHashTable) ret = NULL; SysprofCaptureJitmap *jitmap; uint8_t *buf; uint8_t *endptr; @@ -787,17 +786,15 @@ sysprof_capture_reader_read_jitmap (SysprofCaptureReader *self) if (!sysprof_capture_reader_ensure_space_for (self, jitmap->frame.len)) return NULL; - jitmap = (SysprofCaptureJitmap *)(gpointer)&self->buf[self->pos]; - - ret = g_hash_table_new_full (NULL, NULL, NULL, g_free); + jitmap = (SysprofCaptureJitmap *)(void *)&self->buf[self->pos]; buf = jitmap->data; endptr = &self->buf[self->pos + jitmap->frame.len]; + /* Check the strings are all nul-terminated. */ for (i = 0; i < jitmap->n_jitmaps; i++) { SysprofCaptureAddress addr; - const char *str; if (buf + sizeof addr >= endptr) return NULL; @@ -805,23 +802,19 @@ sysprof_capture_reader_read_jitmap (SysprofCaptureReader *self) memcpy (&addr, buf, sizeof addr); buf += sizeof addr; - str = (char *)buf; - buf = memchr (buf, '\0', (endptr - buf)); if (buf == NULL) return NULL; buf++; - - g_hash_table_insert (ret, GSIZE_TO_POINTER (addr), g_strdup (str)); } sysprof_capture_reader_bswap_jitmap (self, jitmap); self->pos += jitmap->frame.len; - return sysprof_steal_pointer (&ret); + return jitmap; } const SysprofCaptureSample * @@ -1512,3 +1505,53 @@ sysprof_capture_reader_read_allocation (SysprofCaptureReader *self) return ma; } + +typedef struct { + const SysprofCaptureJitmap *jitmap; + const uint8_t *buf; + unsigned int i; + + void *padding1; + void *padding2; +} RealSysprofCaptureJitmapIter; + +void +sysprof_capture_jitmap_iter_init (SysprofCaptureJitmapIter *iter, + const SysprofCaptureJitmap *jitmap) +{ + RealSysprofCaptureJitmapIter *real_iter = (RealSysprofCaptureJitmapIter *) iter; + + assert (iter != NULL); + assert (jitmap != NULL); + + real_iter->jitmap = jitmap; + real_iter->buf = jitmap->data; + real_iter->i = 0; +} + +bool +sysprof_capture_jitmap_iter_next (SysprofCaptureJitmapIter *iter, + SysprofCaptureAddress *addr, + const char **name) +{ + RealSysprofCaptureJitmapIter *real_iter = (RealSysprofCaptureJitmapIter *) iter; + const char *_name; + + assert (iter != NULL); + + if (real_iter->i >= real_iter->jitmap->n_jitmaps) + return false; + + if (addr != NULL) + memcpy (addr, real_iter->buf, sizeof (*addr)); + real_iter->buf += sizeof (*addr); + + _name = (const char *) real_iter->buf; + if (name != NULL) + *name = _name; + real_iter->buf += strlen (_name) + 1; + + real_iter->i++; + + return true; +} diff --git a/src/libsysprof-capture/sysprof-capture-reader.h b/src/libsysprof-capture/sysprof-capture-reader.h index 0fffa6a4..18561ada 100644 --- a/src/libsysprof-capture/sysprof-capture-reader.h +++ b/src/libsysprof-capture/sysprof-capture-reader.h @@ -115,7 +115,7 @@ const SysprofCaptureProcess *sysprof_capture_reader_read_process ( SYSPROF_AVAILABLE_IN_ALL const SysprofCaptureSample *sysprof_capture_reader_read_sample (SysprofCaptureReader *self); SYSPROF_AVAILABLE_IN_ALL -GHashTable *sysprof_capture_reader_read_jitmap (SysprofCaptureReader *self); +const SysprofCaptureJitmap *sysprof_capture_reader_read_jitmap (SysprofCaptureReader *self); SYSPROF_AVAILABLE_IN_ALL const SysprofCaptureCounterDefine *sysprof_capture_reader_read_counter_define (SysprofCaptureReader *self); SYSPROF_AVAILABLE_IN_ALL @@ -150,5 +150,21 @@ bool sysprof_capture_reader_read_file_fd ( const char *path, int fd); +typedef struct { + /*< private >*/ + void *p1; + void *p2; + unsigned int u1; + void *p3; + void *p4; +} SysprofCaptureJitmapIter; + +SYSPROF_AVAILABLE_IN_3_38 +void sysprof_capture_jitmap_iter_init (SysprofCaptureJitmapIter *iter, + const SysprofCaptureJitmap *jitmap); +SYSPROF_AVAILABLE_IN_3_38 +bool sysprof_capture_jitmap_iter_next (SysprofCaptureJitmapIter *iter, + SysprofCaptureAddress *addr, + const char **path); SYSPROF_END_DECLS diff --git a/src/libsysprof-capture/sysprof-capture-writer-cat.c b/src/libsysprof-capture/sysprof-capture-writer-cat.c index 8ee52c7e..d33b4bb8 100644 --- a/src/libsysprof-capture/sysprof-capture-writer-cat.c +++ b/src/libsysprof-capture/sysprof-capture-writer-cat.c @@ -197,10 +197,10 @@ sysprof_capture_writer_cat (SysprofCaptureWriter *self, */ while (sysprof_capture_reader_peek_type (reader, &type)) { - g_autoptr(GHashTable) jitmap = NULL; - GHashTableIter iter; + const SysprofCaptureJitmap *jitmap; + SysprofCaptureJitmapIter iter; + SysprofCaptureAddress addr; const char *name; - uint64_t addr; if (type != SYSPROF_CAPTURE_FRAME_JITMAP) { @@ -212,8 +212,8 @@ sysprof_capture_writer_cat (SysprofCaptureWriter *self, if (!(jitmap = sysprof_capture_reader_read_jitmap (reader))) goto panic; - g_hash_table_iter_init (&iter, jitmap); - while (g_hash_table_iter_next (&iter, (gpointer *)&addr, (gpointer *)&name)) + sysprof_capture_jitmap_iter_init (&iter, jitmap); + while (sysprof_capture_jitmap_iter_next (&iter, &addr, &name)) { uint64_t replace = sysprof_capture_writer_add_jitmap (self, name); /* We need to keep a table of replacement addresses so that diff --git a/src/libsysprof/sysprof-jitmap-symbol-resolver.c b/src/libsysprof/sysprof-jitmap-symbol-resolver.c index ef63e8fe..2015eb50 100644 --- a/src/libsysprof/sysprof-jitmap-symbol-resolver.c +++ b/src/libsysprof/sysprof-jitmap-symbol-resolver.c @@ -74,8 +74,8 @@ sysprof_jitmap_symbol_resolver_load (SysprofSymbolResolver *resolver, while (sysprof_capture_reader_peek_type (reader, &type)) { - g_autoptr(GHashTable) jitmap = NULL; - GHashTableIter iter; + const SysprofCaptureJitmap *jitmap; + SysprofCaptureJitmapIter iter; SysprofCaptureAddress addr; const gchar *str; @@ -89,8 +89,8 @@ sysprof_jitmap_symbol_resolver_load (SysprofSymbolResolver *resolver, if (!(jitmap = sysprof_capture_reader_read_jitmap (reader))) return; - g_hash_table_iter_init (&iter, jitmap); - while (g_hash_table_iter_next (&iter, (gpointer *)&addr, (gpointer *)&str)) + sysprof_capture_jitmap_iter_init (&iter, jitmap); + while (sysprof_capture_jitmap_iter_next (&iter, &addr, &str)) g_hash_table_insert (self->jitmap, GSIZE_TO_POINTER (addr), g_strdup (str)); } } diff --git a/src/tests/test-capture.c b/src/tests/test-capture.c index dda2ab19..cb88f018 100644 --- a/src/tests/test-capture.c +++ b/src/tests/test-capture.c @@ -32,15 +32,16 @@ #include "sysprof-capture-util-private.h" static void -copy_into (GHashTable *src, - GHashTable *dst) +copy_into (const SysprofCaptureJitmap *src, + GHashTable *dst) { - GHashTableIter iter; - gpointer k, v; + SysprofCaptureJitmapIter iter; + SysprofCaptureAddress addr; + const char *name; - g_hash_table_iter_init (&iter, src); - while (g_hash_table_iter_next (&iter, &k, &v)) - g_hash_table_insert (dst, k, g_strdup ((gchar *)v)); + sysprof_capture_jitmap_iter_init (&iter, src); + while (sysprof_capture_jitmap_iter_next (&iter, &addr, &name)) + g_hash_table_insert (dst, GSIZE_TO_POINTER (addr), g_strdup (name)); } static void @@ -318,19 +319,19 @@ test_reader_basic (void) for (;;) { SysprofCaptureFrameType type = -1; - g_autoptr(GHashTable) ret = NULL; + const SysprofCaptureJitmap *jitmap; if (sysprof_capture_reader_peek_type (reader, &type)) g_assert_cmpint (type, ==, SYSPROF_CAPTURE_FRAME_JITMAP); else break; - ret = sysprof_capture_reader_read_jitmap (reader); - g_assert (ret != NULL); + jitmap = sysprof_capture_reader_read_jitmap (reader); + g_assert_nonnull (jitmap); - i += g_hash_table_size (ret); + i += jitmap->n_jitmaps; - copy_into (ret, jmap); + copy_into (jitmap, jmap); } g_assert_cmpint (1000, ==, i); @@ -887,7 +888,7 @@ test_reader_writer_cat_jitmap (void) reader = sysprof_capture_writer_create_reader (res, &error); g_assert_no_error (error); g_assert_nonnull (reader); - g_hash_table_unref (sysprof_capture_reader_read_jitmap (reader)); + sysprof_capture_reader_read_jitmap (reader); sample = sysprof_capture_reader_read_sample (reader); g_assert_cmpint (sample->frame.pid, ==, getpid ()); g_assert_cmpint (sample->n_addrs, ==, G_N_ELEMENTS (addrs)); diff --git a/src/tools/sysprof-dump.c b/src/tools/sysprof-dump.c index 17bd3a9f..d90963dc 100644 --- a/src/tools/sysprof-dump.c +++ b/src/tools/sysprof-dump.c @@ -115,18 +115,18 @@ main (gint argc, case SYSPROF_CAPTURE_FRAME_JITMAP: { - g_autoptr(GHashTable) ret = sysprof_capture_reader_read_jitmap (reader); - GHashTableIter iter; + const SysprofCaptureJitmap *jitmap = sysprof_capture_reader_read_jitmap (reader); + SysprofCaptureJitmapIter iter; SysprofCaptureAddress addr; const gchar *str; - if (ret == NULL) + if (jitmap == NULL) return EXIT_FAILURE; g_print ("JITMAP:\n"); - g_hash_table_iter_init (&iter, ret); - while (g_hash_table_iter_next (&iter, (gpointer *)&addr, (gpointer *)&str)) + sysprof_capture_jitmap_iter_init (&iter, jitmap); + while (sysprof_capture_jitmap_iter_next (&iter, &addr, &str)) g_print (" " SYSPROF_CAPTURE_ADDRESS_FORMAT " : %s\n", addr, str); break;