Subject: bug#46881: 28.0.50; pdumper dumping causes way too many syscalls

From: Pip Cet <[email-masked]>
Date: Fri, 5 Mar 2021 15:12:31 +0000

On Fri, Mar 5, 2021 at 2:56 PM Eli Zaretskii <[email-masked]> wrote:
> > From: Pip Cet <[email-masked]>
> > Date: Fri, 5 Mar 2021 14:02:33 +0000
> > Cc: Daniel Colascione <[email-masked]>, [email-masked], [email-masked]
> >
> > However, there's open_memstream
>
> That's glibc-only, AFAIK.  Not portable enough for us.

POSIX.1-2008. Not portable enough to require, certainly, but portable
enough to use?

Pip
From b600c444e046d3888f56839dbe70d6d209a6ba29 Mon Sep 17 00:00:00 2001
From: Pip Cet <[email-masked]>
Date: Tue, 2 Mar 2021 20:38:23 +0000
Subject: [PATCH] Use stdio, memstreams if available for pdumper (Bug#46881)

* configure.ac: Check for open_memstream.
* src/pdumper.c (dump_unwind_cleanup): Handle memstreams.
(dump_seek): Record maximum offset for memstreams. Use stdio.
(struct dump_context): Add buffer information for memstreams.
(Fdump_emacs_portable): Use memstreams if available.
(dump_write): Use stdio.
(Fdump_emacs_portable): Use memstreams if available.
---
 configure.ac  |  1 +
 src/pdumper.c | 64 ++++++++++++++++++++++++++++++++++++++++-----------
 2 files changed, 51 insertions(+), 14 deletions(-)

diff --git a/configure.ac b/configure.ac
index 11a06a39bee3f..4225aea9635ce 100644
--- a/configure.ac
+++ b/configure.ac
@@ -4554,6 +4554,7 @@ AC_DEFUN
 dnl AC_CHECK_FUNCS_ONCE wouldn’t be right for snprintf, which needs
 dnl the current CFLAGS etc.
 AC_CHECK_FUNCS(snprintf)
+AC_CHECK_FUNCS(open_memstream)
 
 dnl Check for glib.  This differs from other library checks in that
 dnl Emacs need not link to glib unless some other library is already
diff --git a/src/pdumper.c b/src/pdumper.c
index 337742fda4ade..a524f87300b51 100644
--- a/src/pdumper.c
+++ b/src/pdumper.c
@@ -482,8 +482,14 @@ dump_fingerprint (char const *label,
   bool blocked_ralloc;
 #endif
 
-  /* File descriptor for dumpfile; < 0 if closed.  */
-  int fd;
+  /* File handle for dumpfile; NULL if closed.  */
+  FILE *file;
+#ifdef HAVE_OPEN_MEMSTREAM
+  /* buf is non-NULL if file referred to a memstream and has been closed. */
+  char *buf;
+  size_t buf_size;
+  size_t max_offset;
+#endif
   /* Name of dump file --- used for error reporting.  */
   Lisp_Object dump_filename;
   /* Current offset in dump file.  */
@@ -747,9 +753,16 @@ dump_write (struct dump_context *ctx, const void *buf, dump_off nbyte)
   eassert (nbyte == 0 || buf != NULL);
   eassert (ctx->obj_offset == 0);
   eassert (ctx->flags.dump_object_contents);
-  if (emacs_write (ctx->fd, buf, nbyte) < nbyte)
-    report_file_error ("Could not write to dump file", ctx->dump_filename);
-  ctx->offset += nbyte;
+  while (nbyte)
+    {
+      size_t res = fwrite (buf, 1, nbyte, ctx->file);
+      if (res == 0)
+	report_file_error ("Could not write to dump file",
+			   ctx->dump_filename);
+      buf = (char *)buf + res;
+      ctx->offset += res;
+      nbyte -= res;
+    }
 }
 
 static Lisp_Object
@@ -828,10 +841,13 @@ dump_tailq_pop (struct dump_tailq *tailq)
 static void
 dump_seek (struct dump_context *ctx, dump_off offset)
 {
+#ifdef HAVE_OPEN_MEMSTREAM
+  if (ctx->offset > ctx->max_offset)
+    ctx->max_offset = ctx->offset;
+#endif
   eassert (ctx->obj_offset == 0);
-  if (lseek (ctx->fd, offset, SEEK_SET) < 0)
-    report_file_error ("Setting file position",
-                       ctx->dump_filename);
+  if (fseek (ctx->file, offset, SEEK_SET) != 0)
+    report_file_error ("Setting file position", ctx->dump_filename);
   ctx->offset = offset;
 }
 
@@ -3513,8 +3529,25 @@ dump_drain_user_remembered_data_cold (struct dump_context *ctx)
 dump_unwind_cleanup (void *data)
 {
   struct dump_context *ctx = data;
-  if (ctx->fd >= 0)
-    emacs_close (ctx->fd);
+#ifdef HAVE_OPEN_MEMSTREAM
+  if (ctx->file)
+    {
+      fseek (ctx->file, ctx->max_offset, SEEK_SET);
+      fclose (ctx->file);
+    }
+  if (ctx->buf)
+    {
+      eassert (ctx->max_offset <= ctx->buf_size);
+      ctx->file = emacs_fopen (SSDATA (ctx->dump_filename), "w");
+      if (ctx->file == NULL)
+	report_file_error ("Could not open dump file",
+			   ctx->dump_filename);
+      ctx->offset = 0;
+      dump_write (ctx, ctx->buf, ctx->max_offset);
+    }
+#endif
+  if (ctx->file)
+    fclose (ctx->file);
 #ifdef REL_ALLOC
   if (ctx->blocked_ralloc)
     r_alloc_inhibit_buffer_relocation (0);
@@ -3952,7 +3985,7 @@ DEFUN ("dump-emacs-portable",
 
   struct dump_context ctx_buf = {0};
   struct dump_context *ctx = &ctx_buf;
-  ctx->fd = -1;
+  ctx->file = NULL;
 
   ctx->objects_dumped = make_eq_hash_table ();
   dump_queue_init (&ctx->dump_queue);
@@ -4012,9 +4045,12 @@ DEFUN ("dump-emacs-portable",
   ctx->old_process_environment = Vprocess_environment;
   Vprocess_environment = Qnil;
 
-  ctx->fd = emacs_open (SSDATA (filename),
-                        O_RDWR | O_TRUNC | O_CREAT, 0666);
-  if (ctx->fd < 0)
+#ifdef HAVE_OPEN_MEMSTREAM
+  ctx->file = open_memstream (&ctx->buf, &ctx->buf_size);
+#endif
+  if (ctx->file == NULL)
+    ctx->file = emacs_fopen (SSDATA (filename), "w+");
+  if (ctx->file == NULL)
     report_file_error ("Opening dump output", filename);
   verify (sizeof (ctx->header.magic) == sizeof (dump_magic));
   memcpy (&ctx->header.magic, dump_magic, sizeof (dump_magic));
-- 
2.30.1