[sysprof/wip/chergert/mmap-writer] incremental work on mmap writers
- From: Christian Hergert <chergert src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [sysprof/wip/chergert/mmap-writer] incremental work on mmap writers
- Date: Tue, 11 Feb 2020 15:39:24 +0000 (UTC)
commit cce4f8ed51bff8ed9aaef41ed052ea42641a539b
Author: Christian Hergert <chergert redhat com>
Date: Tue Feb 11 07:39:07 2020 -0800
incremental work on mmap writers
src/libsysprof-capture/mmap-writer.c | 144 +++++++++-
src/libsysprof-capture/mmap-writer.h | 32 ++-
src/libsysprof-capture/sysprof-capture-reader.c | 51 ++--
src/libsysprof-capture/sysprof-capture-reader.h | 3 +-
src/libsysprof-capture/sysprof-capture-writer.c | 366 +++++-------------------
src/libsysprof-capture/sysprof-capture-writer.h | 4 -
src/tests/test-capture.c | 15 +-
7 files changed, 269 insertions(+), 346 deletions(-)
---
diff --git a/src/libsysprof-capture/mmap-writer.c b/src/libsysprof-capture/mmap-writer.c
index 21a413d..66a14d0 100644
--- a/src/libsysprof-capture/mmap-writer.c
+++ b/src/libsysprof-capture/mmap-writer.c
@@ -68,6 +68,8 @@
#include "mmap-writer.h"
+#include "sysprof-capture-util-private.h"
+
struct _MmapWriter
{
/* The file-descriptor for our underlying file that is mapped
@@ -85,6 +87,12 @@ struct _MmapWriter
*/
void *page_map;
+ /* Because special information is often stored at the head of a
+ * file, we map the head page specially so that it is always available
+ * for quick access.
+ */
+ void *head_page;
+
/* The first page that is mapped (the byte offset is calculated
* from (page*page_size).
*/
@@ -97,6 +105,78 @@ struct _MmapWriter
goffset wr_offset;
};
+static void
+mmap_writer_unmap_head (MmapWriter *self)
+{
+ g_assert (self != NULL);
+ g_assert (self->n_pages > 0);
+ g_assert (self->page_size > 0);
+
+ if (self->head_page != NULL)
+ {
+ msync (self->head_page, self->page_size, MS_SYNC);
+ madvise (self->head_page, self->page_size * self->n_pages, MADV_DONTNEED);
+ munmap (self->head_page, self->page_size);
+ self->head_page = NULL;
+ }
+}
+
+static gboolean
+mmap_writer_map_head (MmapWriter *self)
+{
+ void *map;
+
+ g_assert (self != NULL);
+ g_assert (self->head_page == NULL);
+ g_assert (self->n_pages > 0);
+ g_assert (self->page_size > 0);
+ g_assert (self->fd > -1);
+
+ if (posix_fallocate (self->fd, 0, self->page_size) < 0)
+ return FALSE;
+
+ map = mmap (NULL, self->page_size, PROT_WRITE | PROT_READ, MAP_SHARED, self->fd, 0);
+ if (map == MAP_FAILED)
+ return FALSE;
+
+ madvise (map, self->page_size * self->n_pages, MADV_WILLNEED);
+
+ self->head_page = map;
+
+ return TRUE;
+}
+
+static void
+mmap_writer_flush_page_map (MmapWriter *self)
+{
+ g_assert (self != NULL);
+
+ g_print ("Flushing %p %ld pages\n", self->page_map, self->n_pages);
+
+ if (self->page_map != NULL)
+ {
+ gsize wr_offset = self->wr_offset;
+ gsize n_pages = 0;
+
+ g_print ("Wr offset: %ld\n", self->wr_offset);
+
+ while (wr_offset > self->page_size)
+ {
+ wr_offset -= self->page_size;
+ n_pages++;
+ }
+
+ if (wr_offset > 0)
+ n_pages++;
+
+ g_print ("%ld pages\n", n_pages);
+
+ msync (self->page_map, n_pages * self->page_size, MS_SYNC);
+
+ g_print ("done\n");
+ }
+}
+
static void
mmap_writer_unmap (MmapWriter *self)
{
@@ -106,6 +186,8 @@ mmap_writer_unmap (MmapWriter *self)
if (self->page_map != NULL)
{
+ g_print ("Unmapping %p\n", self->page_map);
+ mmap_writer_flush_page_map (self);
madvise (self->page_map, self->page_size * self->n_pages, MADV_DONTNEED);
munmap (self->page_map, self->page_size * self->n_pages);
self->page_map = NULL;
@@ -123,20 +205,28 @@ mmap_writer_map (MmapWriter *self)
g_assert (self->page_size > 0);
g_assert (self->fd > -1);
+ if (posix_fallocate (self->fd,
+ self->page * self->page_size,
+ self->n_pages * self->page_size) < 0)
+ return FALSE;
+
map = mmap (NULL,
self->page_size * self->n_pages,
PROT_WRITE | PROT_READ,
MAP_SHARED,
self->fd,
self->page_size * self->page);
-
if (map == MAP_FAILED)
return FALSE;
- madvise (map, self->page_size * self->n_pages, MADV_SEQUENTIAL);
+ madvise (map,
+ self->page_size * self->n_pages,
+ (MADV_NOHUGEPAGE | MADV_SEQUENTIAL));
self->page_map = map;
+ g_print ("Map now at %p\n", map);
+
return TRUE;
}
@@ -146,6 +236,9 @@ normalize_buffer_size (MmapWriter *self,
{
g_assert (self != NULL);
g_assert (self->page_map == NULL);
+ g_assert (self->page_size > 0);
+
+ g_print ("BUFFER SIZE: %ld\n", buffer_size);
if (buffer_size == 0)
{
@@ -163,6 +256,10 @@ normalize_buffer_size (MmapWriter *self,
if (buffer_size > 0)
self->n_pages++;
+
+ g_print ("N_PAGE: %ld\n", self->n_pages);
+
+ g_assert (self->n_pages > 0);
}
MmapWriter *
@@ -176,15 +273,15 @@ mmap_writer_new_for_fd (gint fd,
ret = g_atomic_rc_box_new0 (MmapWriter);
ret->fd = fd;
- ret->page_size = sysconf (_SC_PAGESIZE);
+ ret->page_size = _sysprof_getpagesize ();
ret->page_map = NULL;
ret->page = 0;
ret->n_pages = 16;
- ret->wr_offset = 0;
+ ret->wr_offset = sizeof (SysprofCaptureFileHeader);
normalize_buffer_size (ret, buffer_size);
- if (!mmap_writer_map (ret))
+ if (!mmap_writer_map (ret) || !mmap_writer_map_head (ret))
{
close (ret->fd);
g_atomic_rc_box_release (ret);
@@ -214,8 +311,9 @@ mmap_writer_close (MmapWriter *self)
g_assert (self != NULL);
mmap_writer_unmap (self);
+ mmap_writer_unmap_head (self);
- if (self->fd >= 0)
+ if (self->fd > -1)
{
close (self->fd);
self->fd = -1;
@@ -225,9 +323,6 @@ mmap_writer_close (MmapWriter *self)
void
mmap_writer_destroy (MmapWriter *self)
{
- if (self == NULL)
- return;
-
g_atomic_rc_box_release_full (self, (GDestroyNotify)mmap_writer_close);
}
@@ -280,8 +375,8 @@ mmap_writer_advance (MmapWriter *self,
}
/* Determine how many pages we need loaded */
- req_pages = (length / self->page_size);
- if ((req_pages & 0xFFF) != 0)
+ req_pages = (self->wr_offset + length) / self->page_size;
+ if (((self->wr_offset + length) % self->page_size) > 0)
req_pages++;
/* We might need to increase the buffer size */
@@ -328,5 +423,30 @@ mmap_writer_flush (MmapWriter *self)
{
g_assert (self != NULL);
- fdatasync (self->fd);
+ if (self->head_page != NULL)
+ msync (self->head_page, self->page_size, MS_SYNC);
+
+ mmap_writer_flush_page_map (self);
+}
+
+SysprofCaptureFileHeader *
+mmap_writer_get_file_header (MmapWriter *self)
+{
+ g_assert (self != NULL);
+
+ return (SysprofCaptureFileHeader *)self->head_page;
+}
+
+gsize
+mmap_writer_get_buffer_size (MmapWriter *self)
+{
+ g_assert (self != NULL);
+
+ return self->n_pages * self->page_size;
+}
+
+goffset
+mmap_writer_tell (MmapWriter *self)
+{
+ return self->page * self->page_size + self->wr_offset;
}
diff --git a/src/libsysprof-capture/mmap-writer.h b/src/libsysprof-capture/mmap-writer.h
index c54ea45..de84d99 100644
--- a/src/libsysprof-capture/mmap-writer.h
+++ b/src/libsysprof-capture/mmap-writer.h
@@ -56,31 +56,37 @@
#pragma once
-#include <glib.h>
+#include "sysprof-capture-types.h"
G_BEGIN_DECLS
typedef struct _MmapWriter MmapWriter;
G_GNUC_INTERNAL
-MmapWriter *mmap_writer_new (const gchar *path,
- gsize buffer_size);
+MmapWriter *mmap_writer_new (const gchar *path,
+ gsize buffer_size);
G_GNUC_INTERNAL
-MmapWriter *mmap_writer_new_for_fd (gint fd,
- gsize buffer_size);
+MmapWriter *mmap_writer_new_for_fd (gint fd,
+ gsize buffer_size);
G_GNUC_INTERNAL
-void mmap_writer_close (MmapWriter *self);
+void mmap_writer_close (MmapWriter *self);
G_GNUC_INTERNAL
-void mmap_writer_flush (MmapWriter *self);
+void mmap_writer_flush (MmapWriter *self);
G_GNUC_INTERNAL
-void mmap_writer_destroy (MmapWriter *self);
+void mmap_writer_destroy (MmapWriter *self);
G_GNUC_INTERNAL
-gint mmap_writer_get_fd (MmapWriter *self);
+gint mmap_writer_get_fd (MmapWriter *self);
G_GNUC_INTERNAL
-gpointer mmap_writer_advance (MmapWriter *self,
- goffset length);
+gpointer mmap_writer_advance (MmapWriter *self,
+ goffset length);
G_GNUC_INTERNAL
-gpointer mmap_writer_rewind (MmapWriter *self,
- goffset length);
+gpointer mmap_writer_rewind (MmapWriter *self,
+ goffset length);
+G_GNUC_INTERNAL
+SysprofCaptureFileHeader *mmap_writer_get_file_header (MmapWriter *self);
+G_GNUC_INTERNAL
+gsize mmap_writer_get_buffer_size (MmapWriter *self);
+G_GNUC_INTERNAL
+goffset mmap_writer_tell (MmapWriter *self);
G_END_DECLS
diff --git a/src/libsysprof-capture/sysprof-capture-reader.c b/src/libsysprof-capture/sysprof-capture-reader.c
index 4774e3f..bd6823d 100644
--- a/src/libsysprof-capture/sysprof-capture-reader.c
+++ b/src/libsysprof-capture/sysprof-capture-reader.c
@@ -334,7 +334,20 @@ sysprof_capture_reader_ensure_space_for (SysprofCaptureReader *self,
{
g_assert (self != NULL);
g_assert (self->pos <= self->len);
- g_assert (len > 0);
+ g_assert (len >= sizeof (SysprofCaptureFrame));
+
+ /* If we have a writer that is loading data using mmap(), then there is a
+ * chance that we could hit zero section up to the end of the file. We can
+ * retry the buffered read though in that case so that we allow reading
+ * a file (without mmap) at the same time as writing with mmap. This is
+ * not really a common case except for in unit-testing.
+ */
+ if ((self->len - self->pos) >= sizeof (guint16) &&
+ *(guint16 *)(gpointer)&self->buf[self->pos] == 0)
+ {
+ self->fd_off -= (self->len - self->pos);
+ self->len = self->pos;
+ }
if ((self->len - self->pos) < len)
{
@@ -364,7 +377,17 @@ sysprof_capture_reader_ensure_space_for (SysprofCaptureReader *self,
}
}
- return (self->len - self->pos) >= len;
+ if ((self->len - self->pos) >= len)
+ {
+ /* Make sure we got valid frame data back from the
+ * FD or else we might be in the zero-fill section
+ * up to the end of the file.
+ */
+ if (*(guint16 *)(gpointer)&self->buf[self->pos] >= len)
+ return TRUE;
+ }
+
+ return FALSE;
}
gboolean
@@ -419,6 +442,10 @@ sysprof_capture_reader_peek_frame (SysprofCaptureReader *self,
sysprof_capture_reader_bswap_frame (self, frame);
+ /* In case the capture did not update the end_time during normal usage,
+ * we can update our cached known end_time based on the greatest frame
+ * we come across.
+ */
if (frame->time > self->end_time)
self->end_time = frame->time;
@@ -741,6 +768,7 @@ sysprof_capture_reader_read_jitmap (SysprofCaptureReader *self)
return NULL;
jitmap = (SysprofCaptureJitmap *)(gpointer)&self->buf[self->pos];
+ g_assert (jitmap->frame.len > 0);
ret = g_hash_table_new_full (NULL, NULL, NULL, g_free);
@@ -961,26 +989,9 @@ sysprof_capture_reader_splice (SysprofCaptureReader *self,
GError **error)
{
g_assert (self != NULL);
- g_assert (self->fd != -1);
g_assert (dest != NULL);
- /* Flush before writing anything to ensure consistency */
- if (!sysprof_capture_writer_flush (dest))
- {
- g_set_error (error,
- G_FILE_ERROR,
- g_file_error_from_errno (errno),
- "%s", g_strerror (errno));
- return FALSE;
- }
-
- /*
- * We don't need to track position because writer will
- * track the current position to avoid reseting it.
- */
-
- /* Perform the splice */
- return _sysprof_capture_writer_splice_from_fd (dest, self->fd, error);
+ return sysprof_capture_writer_cat (dest, self, error);
}
/**
diff --git a/src/libsysprof-capture/sysprof-capture-reader.h b/src/libsysprof-capture/sysprof-capture-reader.h
index 3c58524..f8cf5e9 100644
--- a/src/libsysprof-capture/sysprof-capture-reader.h
+++ b/src/libsysprof-capture/sysprof-capture-reader.h
@@ -126,7 +126,8 @@ gboolean sysprof_capture_reader_reset (
SYSPROF_AVAILABLE_IN_ALL
gboolean sysprof_capture_reader_splice (SysprofCaptureReader
*self,
SysprofCaptureWriter
*dest,
- GError
**error);
+ GError
**error)
+ G_GNUC_DEPRECATED_FOR (sysprof_capture_writer_cat);
SYSPROF_AVAILABLE_IN_ALL
gboolean sysprof_capture_reader_save_as (SysprofCaptureReader
*self,
const gchar
*filename,
diff --git a/src/libsysprof-capture/sysprof-capture-writer.c b/src/libsysprof-capture/sysprof-capture-writer.c
index 5e206af..7e08124 100644
--- a/src/libsysprof-capture/sysprof-capture-writer.c
+++ b/src/libsysprof-capture/sysprof-capture-writer.c
@@ -71,6 +71,8 @@
#include <sys/types.h>
#include <unistd.h>
+#include "mmap-writer.h"
+
#include "sysprof-capture-reader.h"
#include "sysprof-capture-util-private.h"
#include "sysprof-capture-writer.h"
@@ -91,48 +93,34 @@ typedef struct
struct _SysprofCaptureWriter
{
- /*
- * 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.
+ /* Our hashtable for JIT deduplication.
*
- * This is paired with a closed hash table for deduplication.
- */
- gchar addr_buf[4096*4];
-
- /* Our hashtable for deduplication. */
- SysprofCaptureJitmapBucket 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
+ * Our address sequence counter is 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
+ *
+ * The addr_hash_size is the number of items in @addr_hash. This is
+ * an optimization so that we avoid calculating the number of strings
* when flushing out the jitmap.
*/
+ guint8 addr_buf[3584];
+ SysprofCaptureJitmapBucket addr_hash[512];
+ gsize addr_buf_pos;
+ gsize addr_seq;
guint addr_hash_size;
- /* Capture file handle */
- int fd;
-
- /* Our write buffer for fd */
- guint8 *buf;
- gsize pos;
- gsize len;
+ /* The MmapWriter abstracts writing to a memory mapped file which is
+ * persisted to underlying storage. The goal here is to ensure that as
+ * we write data, it can be retrieved in case our program crashes. This
+ * is particularly useful when the external Sysprof profiler stops the
+ * profiler but the inferior process does not exit. The profiler can
+ * read up to the known data and then skip any unfinished data when
+ * muxing the streams.
+ *
+ * To make updating the SysprofCaptureFileHeader faster the MmapWriter
+ * keeps the head page around and available.
+ */
+ MmapWriter *writer;
/* GSource for periodic flush */
GSource *periodic_flush;
@@ -152,8 +140,6 @@ sysprof_capture_writer_frame_init (SysprofCaptureFrame *frame_,
gint64 time_,
SysprofCaptureFrameType type)
{
- g_assert (frame_ != NULL);
-
frame_->len = len;
frame_->cpu = cpu;
frame_->pid = pid;
@@ -169,74 +155,30 @@ sysprof_capture_writer_finalize (SysprofCaptureWriter *self)
if (self != NULL)
{
g_clear_pointer (&self->periodic_flush, g_source_destroy);
-
sysprof_capture_writer_flush (self);
-
- if (self->fd != -1)
- {
- close (self->fd);
- self->fd = -1;
- }
-
- g_free (self->buf);
- g_free (self);
+ g_clear_pointer (&self->writer, mmap_writer_destroy);
}
}
SysprofCaptureWriter *
sysprof_capture_writer_ref (SysprofCaptureWriter *self)
{
- g_assert (self != NULL);
- g_assert (self->ref_count > 0);
-
- g_atomic_int_inc (&self->ref_count);
-
- return self;
+ return g_atomic_rc_box_acquire (self);
}
void
sysprof_capture_writer_unref (SysprofCaptureWriter *self)
{
- g_assert (self != NULL);
- g_assert (self->ref_count > 0);
-
- if (g_atomic_int_dec_and_test (&self->ref_count))
- sysprof_capture_writer_finalize (self);
+ g_atomic_rc_box_release_full (self, (GDestroyNotify)sysprof_capture_writer_finalize);
}
static gboolean
sysprof_capture_writer_flush_data (SysprofCaptureWriter *self)
{
- const guint8 *buf;
- gssize written;
- gsize to_write;
-
g_assert (self != NULL);
- g_assert (self->pos <= self->len);
- g_assert ((self->pos % SYSPROF_CAPTURE_ALIGN) == 0);
-
- if (self->pos == 0)
- return TRUE;
- buf = self->buf;
- to_write = self->pos;
-
- while (to_write > 0)
- {
- written = _sysprof_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;
+ if (self->writer != NULL)
+ mmap_writer_flush (self->writer);
return TRUE;
}
@@ -247,52 +189,23 @@ sysprof_capture_writer_realign (gsize *pos)
*pos = (*pos + SYSPROF_CAPTURE_ALIGN - 1) & ~(SYSPROF_CAPTURE_ALIGN - 1);
}
-static inline gboolean
-sysprof_capture_writer_ensure_space_for (SysprofCaptureWriter *self,
- gsize len)
-{
- /* Check for max frame size */
- if (len > G_MAXUSHORT)
- return FALSE;
-
- if ((self->len - self->pos) < len)
- {
- if (!sysprof_capture_writer_flush_data (self))
- return FALSE;
- }
-
- return TRUE;
-}
-
static inline gpointer
sysprof_capture_writer_allocate (SysprofCaptureWriter *self,
gsize *len)
{
- gpointer p;
-
g_assert (self != NULL);
g_assert (len != NULL);
- g_assert ((self->pos % SYSPROF_CAPTURE_ALIGN) == 0);
+ g_assert (self->writer != NULL);
sysprof_capture_writer_realign (len);
- if (!sysprof_capture_writer_ensure_space_for (self, *len))
- return NULL;
-
- p = (gpointer)&self->buf[self->pos];
-
- self->pos += *len;
-
- g_assert ((self->pos % SYSPROF_CAPTURE_ALIGN) == 0);
-
- return p;
+ return mmap_writer_advance (self->writer, *len);
}
static gboolean
sysprof_capture_writer_flush_jitmap (SysprofCaptureWriter *self)
{
- SysprofCaptureJitmap jitmap;
- gssize r;
+ SysprofCaptureJitmap *jitmap;
gsize len;
g_assert (self != NULL);
@@ -302,24 +215,20 @@ sysprof_capture_writer_flush_jitmap (SysprofCaptureWriter *self)
g_assert (self->addr_buf_pos > 0);
- len = sizeof jitmap + self->addr_buf_pos;
-
+ len = sizeof *jitmap + self->addr_buf_pos;
sysprof_capture_writer_realign (&len);
- sysprof_capture_writer_frame_init (&jitmap.frame,
+ jitmap = mmap_writer_advance (self->writer, len);
+ sysprof_capture_writer_frame_init (&jitmap->frame,
len,
-1,
_sysprof_getpid (),
SYSPROF_CAPTURE_CURRENT_TIME,
SYSPROF_CAPTURE_FRAME_JITMAP);
- jitmap.n_jitmaps = self->addr_hash_size;
+ jitmap->n_jitmaps = self->addr_hash_size;
- if (sizeof jitmap != _sysprof_write (self->fd, &jitmap, sizeof jitmap))
- return FALSE;
-
- r = _sysprof_write (self->fd, self->addr_buf, len - sizeof jitmap);
- if (r < 0 || (gsize)r != (len - sizeof jitmap))
- return FALSE;
+ /* Copy the jitmap buffer into the frame data */
+ memcpy (jitmap->data, self->addr_buf, self->addr_buf_pos);
self->addr_buf_pos = 0;
self->addr_hash_size = 0;
@@ -387,7 +296,6 @@ sysprof_capture_writer_insert_jitmap (SysprofCaptureWriter *self,
g_assert (self != NULL);
g_assert (str != NULL);
- g_assert ((self->pos % SYSPROF_CAPTURE_ALIGN) == 0);
len = sizeof addr + strlen (str) + 1;
@@ -466,7 +374,6 @@ sysprof_capture_writer_new_from_fd (int fd,
g_autoptr(GDateTime) now = NULL;
SysprofCaptureWriter *self;
SysprofCaptureFileHeader *header;
- gsize header_len = sizeof(*header);
if (fd < 0)
return NULL;
@@ -480,19 +387,14 @@ sysprof_capture_writer_new_from_fd (int fd,
/* This is only useful on files, memfd, etc */
if (ftruncate (fd, 0) != 0) { /* Do Nothing */ }
- self = g_new0 (SysprofCaptureWriter, 1);
- self->ref_count = 1;
- self->fd = fd;
- self->buf = (guint8 *)g_malloc0 (buffer_size);
- self->len = buffer_size;
+ self = g_atomic_rc_box_new0 (SysprofCaptureWriter);
+ self->writer = mmap_writer_new_for_fd (fd, buffer_size);
self->next_counter_id = 1;
now = g_date_time_new_now_local ();
nowstr = g_date_time_format_iso8601 (now);
- header = sysprof_capture_writer_allocate (self, &header_len);
-
- if (header == NULL)
+ if (!(header = mmap_writer_get_file_header (self->writer)))
{
sysprof_capture_writer_finalize (self);
return NULL;
@@ -511,18 +413,7 @@ sysprof_capture_writer_new_from_fd (int fd,
header->end_time = 0;
memset (header->suffix, 0, sizeof header->suffix);
- if (!sysprof_capture_writer_flush_data (self))
- {
- sysprof_capture_writer_finalize (self);
- return NULL;
- }
-
- g_assert (self->pos == 0);
- g_assert (self->len > 0);
- g_assert (self->len % _sysprof_getpagesize() == 0);
- g_assert (self->buf != NULL);
- g_assert (self->addr_hash_size == 0);
- g_assert (self->fd != -1);
+ mmap_writer_flush (self->writer);
return self;
}
@@ -860,21 +751,12 @@ sysprof_capture_writer_add_timestamp (SysprofCaptureWriter *self,
static gboolean
sysprof_capture_writer_flush_end_time (SysprofCaptureWriter *self)
{
- gint64 end_time = SYSPROF_CAPTURE_CURRENT_TIME;
- ssize_t ret;
+ SysprofCaptureFileHeader *header;
g_assert (self != NULL);
- /* This field is opportunistic, so a failure is okay. */
-
-again:
- ret = _sysprof_pwrite (self->fd,
- &end_time,
- sizeof (end_time),
- G_STRUCT_OFFSET (SysprofCaptureFileHeader, end_time));
-
- if (ret < 0 && errno == EAGAIN)
- goto again;
+ header = mmap_writer_get_file_header (self->writer);
+ header->end_time = SYSPROF_CAPTURE_CURRENT_TIME;
return TRUE;
}
@@ -910,10 +792,11 @@ sysprof_capture_writer_save_as (SysprofCaptureWriter *self,
gsize to_write;
off_t in_off;
off_t pos;
+ int writer_fd;
int fd = -1;
g_assert (self != NULL);
- g_assert (self->fd != -1);
+ g_assert (self->writer != NULL);
g_assert (filename != NULL);
if (-1 == (fd = open (filename, O_CREAT | O_RDWR, 0640)))
@@ -922,8 +805,8 @@ sysprof_capture_writer_save_as (SysprofCaptureWriter *self,
if (!sysprof_capture_writer_flush (self))
goto handle_errno;
- if (-1 == (pos = lseek (self->fd, 0L, SEEK_CUR)))
- goto handle_errno;
+ writer_fd = mmap_writer_get_fd (self->writer);
+ pos = mmap_writer_tell (self->writer);
to_write = pos;
in_off = 0;
@@ -932,7 +815,7 @@ sysprof_capture_writer_save_as (SysprofCaptureWriter *self,
{
gssize written;
- written = _sysprof_sendfile (fd, self->fd, &in_off, pos);
+ written = _sysprof_sendfile (fd, writer_fd, &in_off, to_write);
if (written < 0)
goto handle_errno;
@@ -964,77 +847,6 @@ handle_errno:
return FALSE;
}
-/**
- * _sysprof_capture_writer_splice_from_fd:
- * @self: An #SysprofCaptureWriter
- * @fd: the fd to read from.
- * @error: A location for a #GError, or %NULL.
- *
- * This is internal API for SysprofCaptureWriter and SysprofCaptureReader to
- * communicate when splicing a reader into a writer.
- *
- * This should not be used outside of #SysprofCaptureReader or
- * #SysprofCaptureWriter.
- *
- * This will not advance the position of @fd.
- *
- * Returns: %TRUE if successful; otherwise %FALSE and @error is set.
- */
-gboolean
-_sysprof_capture_writer_splice_from_fd (SysprofCaptureWriter *self,
- int fd,
- GError **error)
-{
- struct stat stbuf;
- off_t in_off;
- gsize to_write;
-
- g_assert (self != NULL);
- g_assert (self->fd != -1);
-
- if (-1 == fstat (fd, &stbuf))
- goto handle_errno;
-
- if (stbuf.st_size < 256)
- {
- g_set_error (error,
- G_FILE_ERROR,
- G_FILE_ERROR_INVAL,
- "Cannot splice, possibly corrupt file.");
- return FALSE;
- }
-
- in_off = 256;
- to_write = stbuf.st_size - in_off;
-
- while (to_write > 0)
- {
- gssize written;
-
- written = _sysprof_sendfile (self->fd, fd, &in_off, to_write);
-
- if (written < 0)
- goto handle_errno;
-
- if (written == 0 && errno != EAGAIN)
- goto handle_errno;
-
- g_assert (written <= (gssize)to_write);
-
- to_write -= written;
- }
-
- return TRUE;
-
-handle_errno:
- g_set_error (error,
- G_FILE_ERROR,
- g_file_error_from_errno (errno),
- "%s", g_strerror (errno));
-
- return FALSE;
-}
-
/**
* sysprof_capture_writer_splice:
* @self: An #SysprofCaptureWriter
@@ -1053,41 +865,20 @@ sysprof_capture_writer_splice (SysprofCaptureWriter *self,
SysprofCaptureWriter *dest,
GError **error)
{
+ SysprofCaptureReader *reader;
gboolean ret;
- off_t pos;
g_assert (self != NULL);
- g_assert (self->fd != -1);
g_assert (dest != NULL);
- g_assert (dest->fd != -1);
-
- /* Flush before writing anything to ensure consistency */
- if (!sysprof_capture_writer_flush (self) || !sysprof_capture_writer_flush (dest))
- goto handle_errno;
- /* Track our current position so we can reset */
- if ((off_t)-1 == (pos = lseek (self->fd, 0L, SEEK_CUR)))
- goto handle_errno;
+ if (!(reader = sysprof_capture_writer_create_reader (self, error)))
+ return FALSE;
- /* Perform the splice */
- ret = _sysprof_capture_writer_splice_from_fd (dest, self->fd, error);
+ ret = sysprof_capture_writer_cat (dest, reader, error);
- /* Now reset or file-descriptor position (it should be the same */
- if (pos != lseek (self->fd, pos, SEEK_SET))
- {
- ret = FALSE;
- goto handle_errno;
- }
+ sysprof_capture_reader_unref (reader);
return ret;
-
-handle_errno:
- g_set_error (error,
- G_FILE_ERROR,
- g_file_error_from_errno (errno),
- "%s", g_strerror (errno));
-
- return FALSE;
}
/**
@@ -1109,25 +900,18 @@ sysprof_capture_writer_create_reader (SysprofCaptureWriter *self,
GError **error)
{
SysprofCaptureReader *ret;
+ int fd;
int copy;
g_return_val_if_fail (self != NULL, NULL);
- g_return_val_if_fail (self->fd != -1, NULL);
- if (!sysprof_capture_writer_flush (self))
- {
- g_set_error (error,
- G_FILE_ERROR,
- g_file_error_from_errno (errno),
- "%s", g_strerror (errno));
- return NULL;
- }
+ fd = mmap_writer_get_fd (self->writer);
/*
* We don't care about the write position, since the reader
* uses positioned reads.
*/
- if (-1 == (copy = dup (self->fd)))
+ if (-1 == (copy = dup (fd)))
return NULL;
if ((ret = sysprof_capture_reader_new_from_fd (copy, error)))
@@ -1304,27 +1088,15 @@ _sysprof_capture_writer_set_time_range (SysprofCaptureWriter *self,
gint64 start_time,
gint64 end_time)
{
- ssize_t ret;
+ SysprofCaptureFileHeader *header;
g_assert (self != NULL);
-do_start:
- ret = _sysprof_pwrite (self->fd,
- &start_time,
- sizeof (start_time),
- G_STRUCT_OFFSET (SysprofCaptureFileHeader, time));
-
- if (ret < 0 && errno == EAGAIN)
- goto do_start;
-
-do_end:
- ret = _sysprof_pwrite (self->fd,
- &end_time,
- sizeof (end_time),
- G_STRUCT_OFFSET (SysprofCaptureFileHeader, end_time));
-
- if (ret < 0 && errno == EAGAIN)
- goto do_end;
+ if ((header = mmap_writer_get_file_header (self->writer)))
+ {
+ header->time = start_time;
+ header->end_time = end_time;
+ }
return TRUE;
}
@@ -1356,7 +1128,7 @@ sysprof_capture_writer_get_buffer_size (SysprofCaptureWriter *self)
{
g_return_val_if_fail (self != NULL, 0);
- return self->len;
+ return mmap_writer_get_buffer_size (self->writer);
}
gboolean
@@ -1557,8 +1329,14 @@ sysprof_capture_writer_add_allocation (SysprofCaptureWriter *self,
{
gsize diff = (sizeof (SysprofCaptureAddress) * (MAX_UNWIND_DEPTH - ev->n_addrs));
- ev->frame.len -= diff;
- self->pos -= diff;
+ /* We can simply rewind the length to what we used here because
+ * SYSPROF_CAPTURE_ALIGN matches sizeof(SysprofCaptureAddress).
+ */
+ if (diff > 0)
+ {
+ ev->frame.len -= diff;
+ mmap_writer_rewind (self->writer, diff);
+ }
}
self->stat.frame_count[SYSPROF_CAPTURE_FRAME_ALLOCATION]++;
diff --git a/src/libsysprof-capture/sysprof-capture-writer.h b/src/libsysprof-capture/sysprof-capture-writer.h
index 657e726..4eeb18b 100644
--- a/src/libsysprof-capture/sysprof-capture-writer.h
+++ b/src/libsysprof-capture/sysprof-capture-writer.h
@@ -237,10 +237,6 @@ gboolean sysprof_capture_writer_cat (Sy
SysprofCaptureReader
*reader,
GError
**error);
G_GNUC_INTERNAL
-gboolean _sysprof_capture_writer_splice_from_fd (SysprofCaptureWriter
*self,
- int
fd,
- GError
**error) G_GNUC_INTERNAL;
-G_GNUC_INTERNAL
gboolean _sysprof_capture_writer_set_time_range (SysprofCaptureWriter
*self,
gint64
start_time,
gint64
end_time) G_GNUC_INTERNAL;
diff --git a/src/tests/test-capture.c b/src/tests/test-capture.c
index 64d6f2c..fe9c6f8 100644
--- a/src/tests/test-capture.c
+++ b/src/tests/test-capture.c
@@ -239,7 +239,7 @@ test_reader_basic (void)
const SysprofCaptureCounterDefine *def;
def = sysprof_capture_reader_read_counter_define (reader);
- g_assert (def != NULL);
+ g_assert_nonnull (def);
g_assert_cmpint (def->n_counters, ==, 10);
for (i = 0; i < def->n_counters; i++)
@@ -421,7 +421,16 @@ test_reader_basic (void)
g_assert_cmpint (buf1len, >, 0);
g_assert_cmpint (buf2len, >, 0);
- g_assert_cmpint (buf1len, ==, buf2len);
+ /* Make sure the sizes match or else there is only zero padding
+ * at the end of our capture file that is truncated from the
+ * backup file.
+ */
+ if (buf1len != buf2len)
+ {
+ for (gsize j = buf1len; j < buf2len; j++)
+ g_assert_cmpint (buf2[j], ==, 0);
+ }
+
g_assert_true (0 == memcmp (buf1, buf2, buf1len));
}
@@ -544,7 +553,9 @@ test_reader_splice (void)
r = sysprof_capture_reader_peek_type (reader, &type);
g_assert_cmpint (r, ==, FALSE);
+ G_GNUC_BEGIN_IGNORE_DEPRECATIONS
r = sysprof_capture_reader_splice (reader, writer2, &error);
+ G_GNUC_END_IGNORE_DEPRECATIONS
g_assert_no_error (error);
g_assert_cmpint (r, ==, TRUE);
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
