Re: Changes in GC and in pure space

Re: Changes in GC and in pure space

[Eli Zaretskii]

> Cc: emacs-devel@gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Mon, 22 Jul 2019 10:47:27 -0700
> 
> > After all, these two issues are not
> > terribly urgent to fix (unlike, say, the unexec thingy).
> 
> As it happens, the issue with hash tables is bound up with the unexec thingy, in 
> that portable dumping currently can screw up hash tables with user-defined 
> tests.

I wasn't talking about the hash tables; that bug needs to be fixed,
indeed.  I was talking about making more thorough changes in GC (and
elsewhere) than are strictly needed for fixing the hash-table bug.
(AFAIR, the fix for the hash tables didn't involve GC, last time I
looked.)

Re: Changes in GC and in pure space

[Stefan Monnier]

>> As it happens, the issue with hash tables is bound up with the unexec thingy, in 
>> that portable dumping currently can screw up hash tables with user-defined 
>> tests.

Note that AFAIK we currently don't dump any such hash-tables, so if
there's a bug there it only affects the case of the user *re*dumping
with a special config, which AFAIK we do not intend to officially
support yet for 27.1.

> I wasn't talking about the hash tables; that bug needs to be fixed,
> indeed.

Not sure which bug you're referring to, because the one discussed
recently (linked to the pdumper's need to rehash tables) should be fixed
already (Pip Cet provided both that simple fix I installed as well as
a subsequent better but more intrusive fix).


        Stefan

Re: Changes in GC and in pure space

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 1022 bytes --]

Stefan Monnier wrote:
>>> As it happens, the issue with hash tables is bound up with the unexec thingy, in
>>> that portable dumping currently can screw up hash tables with user-defined
>>> tests.
> 
> Note that AFAIK we currently don't dump any such hash-tables, so if
> there's a bug there it only affects the case of the user *re*dumping
> with a special config, which AFAIK we do not intend to officially
> support yet for 27.1.

Although I vaguely recall an email conversation to that effect, I don't see it 
in the Emacs documentation. And since unexec is now disabled by default, if 
people want to use any dumping method at all then by default they'll be 
redumping in the way that you suggest. If we really intend to not support that, 
we should be clearer about it in NEWS and documentation.

At any rate the simplest workaround is to avoid pdumping the problematic hash 
tables, and I installed the attached patch to do that. We can add support for 
pdumping those hash tables later, if anybody cares about it.

[-- Attachment #2: 0001-Do-not-pdump-user-defined-hashtabs.patch --]
[-- Type: text/x-patch, Size: 2119 bytes --]

From 3f4a9a5a3b267fbc13a8bebc4295bbfadd6ff03e Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Mon, 22 Jul 2019 18:33:39 -0700
Subject: [PATCH] Do not pdump user-defined hashtabs

* src/pdumper.c (dump_hash_table_stable_p):
Signal an error if a hash table has user-defined tests (Bug#36769).
* src/fns.c (hashfn_user_defined): Now extern.
---
 src/fns.c     | 2 +-
 src/lisp.h    | 1 +
 src/pdumper.c | 2 ++
 3 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/fns.c b/src/fns.c
index 8eefa7c72b..734a2e253c 100644
--- a/src/fns.c
+++ b/src/fns.c
@@ -4017,7 +4017,7 @@ hashfn_eql (Lisp_Object key, struct Lisp_Hash_Table *h)
 /* Given HT, return a hash code for KEY which uses a user-defined
    function to compare keys.  */
 
-static Lisp_Object
+Lisp_Object
 hashfn_user_defined (Lisp_Object key, struct Lisp_Hash_Table *h)
 {
   Lisp_Object args[] = { h->test.user_hash_function, key };
diff --git a/src/lisp.h b/src/lisp.h
index 9d37629bc4..e96fcfe94d 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -3606,6 +3606,7 @@ EMACS_UINT hash_string (char const *, ptrdiff_t);
 EMACS_UINT sxhash (Lisp_Object, int);
 Lisp_Object hashfn_eql (Lisp_Object, struct Lisp_Hash_Table *);
 Lisp_Object hashfn_equal (Lisp_Object, struct Lisp_Hash_Table *);
+Lisp_Object hashfn_user_defined (Lisp_Object, struct Lisp_Hash_Table *);
 Lisp_Object make_hash_table (struct hash_table_test, EMACS_INT, float, float,
                              Lisp_Object, bool);
 ptrdiff_t hash_lookup (struct Lisp_Hash_Table *, Lisp_Object, Lisp_Object *);
diff --git a/src/pdumper.c b/src/pdumper.c
index 2abac80a37..cda8b40be4 100644
--- a/src/pdumper.c
+++ b/src/pdumper.c
@@ -2628,6 +2628,8 @@ dump_vectorlike_generic (struct dump_context *ctx,
 static bool
 dump_hash_table_stable_p (const struct Lisp_Hash_Table *hash)
 {
+  if (hash->test.hashfn == hashfn_user_defined)
+    error ("cannot dump hash tables with user-defined tests");  /* Bug#36769 */
   bool is_eql = hash->test.hashfn == hashfn_eql;
   bool is_equal = hash->test.hashfn == hashfn_equal;
   ptrdiff_t size = HASH_TABLE_SIZE (hash);
-- 
2.17.1

Re: Changes in GC and in pure space

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Paul Eggert <eggert@cs.ucla.edu>,  pipcet@gmail.com,  emacs-devel@gnu.org
> Date: Mon, 22 Jul 2019 15:58:27 -0400
> 
> > I wasn't talking about the hash tables; that bug needs to be fixed,
> > indeed.
> 
> Not sure which bug you're referring to, because the one discussed
> recently (linked to the pdumper's need to rehash tables) should be fixed
> already (Pip Cet provided both that simple fix I installed as well as
> a subsequent better but more intrusive fix).

That one.

Re: Changes in GC and in pure space

[Eli Zaretskii]

> Cc: pipcet@gmail.com, emacs-devel@gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Mon, 22 Jul 2019 18:43:45 -0700
> 
> > Note that AFAIK we currently don't dump any such hash-tables, so if
> > there's a bug there it only affects the case of the user *re*dumping
> > with a special config, which AFAIK we do not intend to officially
> > support yet for 27.1.
> 
> Although I vaguely recall an email conversation to that effect, I don't see it 
> in the Emacs documentation. And since unexec is now disabled by default, if 
> people want to use any dumping method at all then by default they'll be 
> redumping in the way that you suggest. If we really intend to not support that, 
> we should be clearer about it in NEWS and documentation.

We do want to support re-dumping eventually, but we are clearly not
there yet.  If you tried that, you probably saw a few quirks very
quickly.  It will take time to rectify all of them, and I agree we
shouldn't aim for that milestone in Emacs 27.1.

Re: Changes in GC and in pure space

[Stefan Monnier]

> Four tag bits for "annotated" (e.g., immutable) objects: very far from
> ready, and problematic on 32-bit machines (perhaps this is no longer a
> concern for Emacs 28...)

I'm still using 32bit Debian on pretty much all my machines (except for
one).  I could change to 64bit OSes on most of them, but not on the
Thinkpad X60 I use for my in-class presentations nor on the two BananaPis
I use as servers.

So, AFAIC, 32bit is not something we can drop yet:
It's OK to support it less-well, but it's not OK not to support it.

> Turning pseudovectors into their own tag type, as miscellaneous
> objects: not convinced it's worth the change, yet.

I'm not sure what this is referring to.

But in this area, I'd welcome a change which makes symbols and strings
(and intervals) use the same representation as pseudovectors (just like
buffer and misc has been merged many years ago).

They could still use their own "enum Lisp_Type" tag (some benchmarking
might be needed to decide if it's worthwhile), but the idea would
be to drop their *_blocks from alloc.c.


        Stefan

Re: Changes in GC and in pure space

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 451 bytes --]

>> we should be clearer about it in NEWS and documentation.
> 
> We do want to support re-dumping eventually, but we are clearly not
> there yet.

OK, I added something to that effect to the NEWS file and to internals.texi. 
While in the neighborhood, I also put in a statement that unexec is deprecated 
and rewrote instances of "portable dump file" as the dump files themselves are 
no more portable than an Emacs executable is. See attached patch.

[-- Attachment #2: 0001-Improve-pdumper-doc-say-unexec-is-deprecated.patch --]
[-- Type: text/x-patch, Size: 15372 bytes --]

From 8dd5b6ea56c38669bc98104ee2d6b31496624d28 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Tue, 23 Jul 2019 09:19:09 -0700
Subject: [PATCH] Improve pdumper doc; say unexec is deprecated

Say that pdumping cannot redump unless -batch is used.  Say that
the traditional unexec dumping method is by default not available,
and is deprecated.  Don't call dump files "portable", as dump files
are not any more portable than the Emacs executables themselves.
Just call them "dump files".  Similar, prefer "portable dumper"
(since the dumper code is portable) to "portable dumping" (since
the dump file is not).  Be more systematic about calling them
"dump files" instead of "dumped images" or whatnot.
---
 doc/lispref/internals.texi | 34 ++++++++++++++++++++--------------
 etc/NEWS                   | 14 +++++++++-----
 etc/TODO                   |  6 +-----
 lib/fingerprint.h          |  3 +--
 lisp/startup.el            |  4 ++--
 src/alloc.c                |  4 ++--
 src/emacs.c                |  6 +++---
 src/minibuf.c              |  2 +-
 src/pdumper.c              | 10 +++++-----
 src/pdumper.h              |  4 ++--
 src/unexaix.c              |  2 +-
 11 files changed, 47 insertions(+), 42 deletions(-)

diff --git a/doc/lispref/internals.texi b/doc/lispref/internals.texi
index 72066d34f4..f85c266ede 100644
--- a/doc/lispref/internals.texi
+++ b/doc/lispref/internals.texi
@@ -61,10 +61,10 @@ Building Emacs
 
 @table @samp
 @item pdump
-@cindex portable dump file
-Record the preloaded Lisp data in a @dfn{portable dump} file.  This
+@cindex dump file
+Record the preloaded Lisp data in a @dfn{dump file}.  This
 method produces an additional data file which Emacs will load at
-startup.  The portable dump file is usually called @file{emacs.pdmp},
+startup.  The produced dump file is usually called @file{emacs.pdmp},
 and is installed in the Emacs @code{exec-directory} (@pxref{Help
 Functions}).  This method is the most preferred one, as it does not
 require Emacs to employ any special techniques of memory allocation,
@@ -75,7 +75,7 @@ Building Emacs
 @cindex bootstrapping Emacs
 Like @samp{pdump}, but used while @dfn{bootstrapping} Emacs, when no
 previous Emacs binary and no @file{*.elc} byte-compiled Lisp files are
-available.  The produced portable dump file is usually named
+available.  The produced dump file is usually named
 @file{bootstrap-emacs.pdmp} in this case.
 
 @item dump
@@ -88,6 +88,8 @@ Building Emacs
 dumped Emacs.)  This method is also known as @dfn{unexec}, because it
 produces a program file from a running process, and thus is in some
 sense the opposite of executing a program to start a process.
+Although this method was the way that Emacs traditionally saved its
+state, it is now deprecated.
 
 @item bootstrap
 Like @samp{dump}, but used when bootstrapping Emacs with the
@@ -97,11 +99,11 @@ Building Emacs
 @cindex preloaded Lisp files
 @vindex preloaded-file-list
   The dumped @file{emacs} executable (also called a @dfn{pure} Emacs)
-is the one which is installed.  If the portable dumping was used to
+is the one which is installed.  If the portable dumper was used to
 build Emacs, the @file{emacs} executable is actually an exact copy of
 @file{temacs}, and the corresponding @file{emacs.pdmp} file is
 installed as well.  The variable @code{preloaded-file-list} stores a
-list of the preloaded Lisp files recorded in the portable dump file or
+list of the preloaded Lisp files recorded in the dump file or
 in the dumped Emacs executable.  If you port Emacs to a new operating
 system, and are not able to implement dumping of any kind, then Emacs
 must load @file{loadup.el} each time it starts.
@@ -201,15 +203,19 @@ Building Emacs
 @code{before-init-hook} (@pxref{Startup Summary}).
 
 @defun dump-emacs-portable to-file &optional track-referrers
-This function dumps the current state of Emacs into a portable dump
+This function dumps the current state of Emacs into a dump
 file @var{to-file}, using the @code{pdump} method.  Normally, the
-portable dump file is called @file{@var{emacs-name}.dmp}, where
+dump file is called @file{@var{emacs-name}.dmp}, where
 @var{emacs-name} is the name of the Emacs executable file.  The
 optional argument @var{track-referrers}, if non-@code{nil}, causes the
-portable dumping process keep additional information to help track
+portable dumper to keep additional information to help track
 down the provenance of object types that are not yet supported by the
 @code{pdump} method.
 
+Although the portable dumper code can run on many platforms, the dump
+files that it produces are not portable---they can be loaded only by
+the Emacs executable that dumped them.
+
 If you want to use this function in an Emacs that was already dumped,
 you must run Emacs with the @samp{-batch} option.
 @end defun
@@ -220,20 +226,20 @@ Building Emacs
 @var{to-file}, using the @code{unexec} method.  It takes symbols from
 @var{from-file} (this is normally the executable file @file{temacs}).
 
-This function cannot be used in an Emacs that was already dumped.  If
-Emacs was built without @code{unexec} support, this function will not
-be available.
+This function cannot be used in an Emacs that was already dumped.
+This function is deprecated, and by default Emacs is built without
+@code{unexec} support so this function is not available.
 @end defun
 
 @defun pdumper-stats
-If the current Emacs session restored its state from a portable dump
+If the current Emacs session restored its state from a dump
 file, this function returns information about the dump file and the
 time it took to restore the Emacs state.  The value is an alist
 @w{@code{((dumped-with-pdumper . t) (load-time . @var{time})
 (dump-file-name . @var{file}))}},
 where @var{file} is the name of the dump file, and @var{time} is the
 time in seconds it took to restore the state from the dump file.
-If the current session was not restored from a portable dump file, the
+If the current session was not restored from a dump file, the
 value is nil.
 @end defun
 
diff --git a/etc/NEWS b/etc/NEWS
index 5378e56bca..7fd2214582 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -92,11 +92,6 @@ and in particular better supports the Address Space Layout
 Randomization (ASLR) feature, a security technique used by most modern
 operating systems.
 
-Portable dumping can be disabled at configure time via the configure
-option '--with-dumping=unexec' (but we don't recommend that, unless
-the portable dumping doesn't work on your system for some
-reason---please report such systems to the Emacs developers as bugs).
-
 When built with the portable dumping support (which is the default),
 Emacs looks for the 'emacs.pdmp' file, generated during the build, in
 its data directory at startup, and loads the dumped state from there.
@@ -104,6 +99,15 @@ The new command-line argument '--dump-file=FILE' allows to specify a
 non-default '.pdmp' file to load the state from; see the node "Initial
 Options" in the Emacs manual for more information.
 
+An Emacs started via a dump file can create a new dump file only if it
+was invoked with the -batch option.
+
+Although the portable dumper has been tested, it may have a bug on
+unusual platforms.  If you require traditional unexec dumping you can
+use the configure-time option '--with-dumping=unexec'; however, please
+file a bug report describing the situation, as unexec dumping is
+deprecated.
+
 +++
 ** The new configure option '--enable-checking=structs' attempts to
 check that the portable dumper code has been updated to match the last
diff --git a/etc/TODO b/etc/TODO
index b2446b0d91..a065763933 100644
--- a/etc/TODO
+++ b/etc/TODO
@@ -297,11 +297,7 @@ One way of doing this is to start with fx's dynamic loading, and use it
 to implement things like auto-loaded buffer parsers and database
 access in cases which need more than Lisp.
 
-** Replace unexec with a more portable form of dumping
-See eg https://lists.gnu.org/r/emacs-devel/2014-01/msg01034.html
-       https://lists.gnu.org/r/emacs-devel/2014-06/msg00452.html
-
-One way is to provide portable undumping using mmap (per gerd design).
+** Fix portable dumping so that you can redump without using -batch.
 
 ** Imenu could be extended into a file-structure browsing mechanism
 using code like that of customize-groups.
diff --git a/lib/fingerprint.h b/lib/fingerprint.h
index ba2e740cd9..7d2160c988 100644
--- a/lib/fingerprint.h
+++ b/lib/fingerprint.h
@@ -22,8 +22,7 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 
 /* We generate fingerprint.c and fingerprint.o from all the sources in
    Emacs.  This way, we have a unique value that we can use to pair
-   data files (like a portable dump image) with a specific build of
-   Emacs.  */
+   data files (like a dump file) with a specific build of Emacs.  */
 extern volatile unsigned char fingerprint[32];
 
 #endif
diff --git a/lisp/startup.el b/lisp/startup.el
index 7759ed5aed..564428580b 100644
--- a/lisp/startup.el
+++ b/lisp/startup.el
@@ -354,8 +354,8 @@ site-run-file
   "File containing site-wide run-time initializations.
 This file is loaded at run-time before `~/.emacs'.  It contains inits
 that need to be in place for the entire site, but which, due to their
-higher incidence of change, don't make sense to load into Emacs's
-dumped image.  Thus, the run-time load order is: 1. file described in
+higher incidence of change, don't make sense to put into Emacs's
+dump file.  Thus, the run-time load order is: 1. file described in
 this variable, if non-nil; 2. `~/.emacs'; 3. `default.el'.
 
 Don't use the `site-start.el' file for things some users may not like.
diff --git a/src/alloc.c b/src/alloc.c
index 5d8003ffb5..f256ff71b0 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -4531,9 +4531,9 @@ mark_maybe_object (Lisp_Object obj)
 
   void *po = XPNTR (obj);
 
-  /* If the pointer is in the dumped image and the dump has a record
+  /* If the pointer is in the dump image and the dump has a record
      of the object starting at the place where the pointer points, we
-     definitely have an object.  If the pointer is in the dumped image
+     definitely have an object.  If the pointer is in the dump image
      and the dump has no idea what the pointer is pointing at, we
      definitely _don't_ have an object.  */
   if (pdumper_object_p (po))
diff --git a/src/emacs.c b/src/emacs.c
index ad661a081b..cc5818393a 100644
--- a/src/emacs.c
+++ b/src/emacs.c
@@ -686,7 +686,7 @@ dump_error_to_string (enum pdumper_load_result result)
 }
 
 /* Find a path (absolute or relative) to the Emacs executable.
-   Called early in initialization by portable dump loading code, so we
+   Called early in initialization by portable dumper loading code, so we
    can't use lisp and associated machinery.  On success, *EXENAME is
    set to a heap-allocated string giving a path to the Emacs
    executable or to NULL if we can't determine the path immediately.
@@ -801,12 +801,12 @@ load_pdump (int argc, char **argv)
     ;
 
   /* TODO: maybe more thoroughly scrub process environment in order to
-     make this use case (loading a pdumper image in an unexeced emacs)
+     make this use case (loading a dump file in an unexeced emacs)
      possible?  Right now, we assume that things we don't touch are
      zero-initialized, and in an unexeced Emacs, this assumption
      doesn't hold.  */
   if (initialized)
-    fatal ("cannot load pdumper image in unexeced Emacs");
+    fatal ("cannot load dump file in unexeced Emacs");
 
   /* Look for an explicitly-specified dump file.  */
   const char *path_exec = PATH_EXEC;
diff --git a/src/minibuf.c b/src/minibuf.c
index d9a6e15b05..8920f37827 100644
--- a/src/minibuf.c
+++ b/src/minibuf.c
@@ -1885,7 +1885,7 @@ init_minibuf_once_for_pdumper (void)
   PDUMPER_IGNORE (minibuf_prompt_width);
 
   /* We run this function on first initialization and whenever we
-     restore from a pdumper image.  pdumper doesn't try to preserve
+     restore from a dump file.  pdumper doesn't try to preserve
      frames, windows, and so on, so reset everything related here.  */
   Vminibuffer_list = Qnil;
   minibuf_level = 0;
diff --git a/src/pdumper.c b/src/pdumper.c
index cda8b40be4..84147353e8 100644
--- a/src/pdumper.c
+++ b/src/pdumper.c
@@ -333,8 +333,8 @@ dump_fingerprint (char const *label,
   fprintf (stderr, "%s: %.*s\n", label, hexbuf_size, hexbuf);
 }
 
-/* Format of an Emacs portable dump file.  All offsets are relative to
-   the beginning of the file.  An Emacs portable dump file is coupled
+/* Format of an Emacs dump file.  All offsets are relative to
+   the beginning of the file.  An Emacs dump file is coupled
    to exactly the Emacs binary that produced it, so details of
    alignment and endianness are unimportant.
 
@@ -3990,7 +3990,7 @@ dump_drain_deferred_symbols (struct dump_context *ctx)
 DEFUN ("dump-emacs-portable",
        Fdump_emacs_portable, Sdump_emacs_portable,
        1, 2, 0,
-       doc: /* Dump current state of Emacs into portable dump file FILENAME.
+       doc: /* Dump current state of Emacs into dump file FILENAME.
 If TRACK-REFERRERS is non-nil, keep additional debugging information
 that can help track down the provenance of unsupported object
 types.  */)
@@ -5470,14 +5470,14 @@ pdumper_record_wd (const char *wd)
 
 DEFUN ("pdumper-stats", Fpdumper_stats, Spdumper_stats, 0, 0, 0,
        doc: /* Return statistics about portable dumping used by this session.
-If this Emacs sesion was started from a portable dump file,
+If this Emacs session was started from a dump file,
 the return value is an alist of the form:
 
   ((dumped-with-pdumper . t) (load-time . TIME) (dump-file-name . FILE))
 
 where TIME is the time in seconds it took to restore Emacs state
 from the dump file, and FILE is the name of the dump file.
-Value is nil if this session was not started using a portable dump file.*/)
+Value is nil if this session was not started using a dump file.*/)
      (void)
 {
   if (!dumped_with_pdumper_p ())
diff --git a/src/pdumper.h b/src/pdumper.h
index ab2f426c1e..5d1e9c3aea 100644
--- a/src/pdumper.h
+++ b/src/pdumper.h
@@ -35,7 +35,7 @@ INLINE_HEADER_BEGIN
    variables to which the Lisp heap points.  It doesn't know anything
    about other C variables.  The functions below allow code from other
    parts of Emacs to tell the portable dumper about other bits of
-   information to preserve in dumped images.
+   information to preserve in dump files.
 
    These memory-records are themselves preserved in the dump, so call
    the functions below only on the !initialized init path, just
@@ -44,7 +44,7 @@ INLINE_HEADER_BEGIN
    There are no special functions to preserve a global Lisp_Object.
    You should just staticpro these.  */
 
-/* Remember the value of THING in dumped images.  THING must not
+/* Remember the value of THING in dump files.  THING must not
    contain any pointers or Lisp_Object variables: these values are not
    valid across dump and load.  */
 #define PDUMPER_REMEMBER_SCALAR(thing)                  \
diff --git a/src/unexaix.c b/src/unexaix.c
index 349d365383..c95486cf72 100644
--- a/src/unexaix.c
+++ b/src/unexaix.c
@@ -1,4 +1,4 @@
-/* Dump an executable image.
+/* Dump an executable file.
    Copyright (C) 1985-1988, 1999, 2001-2019 Free Software Foundation,
    Inc.
 
-- 
2.17.1

Re: Changes in GC and in pure space

[Eli Zaretskii]

> Cc: monnier@iro.umontreal.ca, pipcet@gmail.com, emacs-devel@gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Tue, 23 Jul 2019 09:27:32 -0700
> 
> While in the neighborhood, I also put in a statement that unexec is deprecated 
> and rewrote instances of "portable dump file" as the dump files themselves are 
> no more portable than an Emacs executable is. See attached patch.

Thanks.

Not to start another bikeshedding thread, but just a thought: I wonder
whether using "dump file" for the emacs.pdmp file is the best idea.
Recall that building with unexec says "Dumping under the name...", so
"dumping" is not necessarily alien to unexec.  If "portable dump file"
is deemed too confusing, then perhaps we should use "pdump file"?

Re: Changes in GC and in pure space

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

Some of the machines that we use Libreboot on are 32-bit machines.
We need to keep supporting 32-bit mode for years
unless we get some unforeseen good news.

-- 
Dr Richard Stallman
President, Free Software Foundation (https://gnu.org, https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

Re: Changes in GC and in pure space

[Stefan Monnier]

> How portable is "INLINE" (and if it's portable enough, why do we use a
> macro for it)?  If some platforms don't support it, and these macros
> become non-inline functions, those platforms will be punished by this
> kind of changes.

I'm not sure if replacing INLINE with nothing at all would lead to much
worse code.  Obviously, in some cases it will, but the optimizer should
generally still inline them anyway.

> I FWIW, personally find the issue of confusion about macro argument
> evaluation to be a very weak one as justification to get rid of macros
> that needed approximately zero maintenance for many years.

I don't find it terribly compelling either, although I have been bitten
a few times.  OTOH as functions, they tend to behave better in GDB.
So overall, I think it's good to use functions rather than macros where
both are applicable (just like in Elisp).


        Stefan

Re: Changes in GC and in pure space

[Óscar Fuentes]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> How portable is "INLINE" (and if it's portable enough, why do we use a
>> macro for it)?  If some platforms don't support it, and these macros
>> become non-inline functions, those platforms will be punished by this
>> kind of changes.
>
> I'm not sure if replacing INLINE with nothing at all would lead to much
> worse code.  Obviously, in some cases it will, but the optimizer should
> generally still inline them anyway.

At least in the C++ world, `inline' is used for putting function
definitions on headers, not for inlining.

There is a consensus from many years ago that the compiler is expected
to do the right thing wrt inlining irrespectively of the presence or
ausence of the keyword, the same way `register` is no longer used for
overriding the compiler's register allocation system.

Re: Changes in GC and in pure space

[Paul Eggert]

Óscar Fuentes wrote:
> At least in the C++ world, `inline' is used for putting function
> definitions on headers, not for inlining.
> 
> There is a consensus from many years ago that the compiler is expected
> to do the right thing wrt inlining irrespectively of the presence or
> ausence of the keyword

Things are a bit different in C. In C, the consensus you mention holds for 
'static inline' (where the 'inline' is typically just a noise word nowadays), 
but it does not hold for the plain 'inline' functions that we're talking about 
because C requires that if one compilation unit defines a plain 'inline' 
function, then exactly one other compilation unit must define that function as 
'extern inline'.

Emacs rarely uses 'static inline', due to the consensus that you mention. In the 
few exceptions where Emacs does use 'static inline', I vaguely recall there 
being performance advantages when compiled by GCC in typical platforms, as GCC 
does not ignore the 'inline' in these exceptional situations.

Re: Changes in GC and in pure space

[Angelo Graziosi]

Eli Zaretskii wrote
>
> If some platforms don't support it, and these macros
> become non-inline functions, those platforms will be punished by this
> kind of changes
>
> [...] But that's me.
>

Me too! 

When I was a kid I was taught that macros are preferable to functions if these are short, don't use loop etc., and this to save on the cost of the function call.. also old FORTRAN has 'statement functions'. If I remember correctly, even if language as C++ has 'inline' statement, this does not mean that the function will be really 'inlined', is the compiler that decide about the fate of functions declared 'inline'..

Maybe all I learned from Borland manuals was wrong... and all programs I wrote up to 2004 have to be rewritten..... Gulp!

Re: Changes in GC and in pure space

[Paul Eggert]

Angelo Graziosi wrote:
> Maybe all I learned from Borland manuals was wrong.

It is indeed wrong, at least for the buffer.h macros that I recently turned into 
INLINE functions, as that sped up my usual nontrivial benchmark ('cd lisp; make 
compile-always') by 5.7% on my platform.

Stefan made a suggestion in this thread that I think can get us another ~5% 
speedup on the same benchmark. I'm looking into that. It also involves using 
functions not macros.

Re: Changes in GC and in pure space

[Stefan Monnier]

> When I was a kid I was taught that macros are preferable to functions if
> these are short, don't use loop etc., and this to save on the cost of the
> function call.. also old FORTRAN has 'statement functions'.
[...]
> Maybe all I learned from Borland manuals was wrong...

I don't think we can expect Emacs will be compiled by a compiler
comparable to Borland's (these were focused on generating code as fast
as possible).

Nowadays compilers work hard to encourage you to write readable code and
to focus your optimization efforts on algorithmic changes rather than
low-level micro-optimization (since these may just as well end up being
pessimizations unless you really know what's going on).

This said, I'd expect that using macros to perform "manual inlining" can
still be beneficial for the case where you compile with -O0 (which is
why I asked Paul to still use such macros in a few specific places where
it seemed worthwhile: I use -O0 most of the time).


        Stefan

Changes in GC and in pure space

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 753 bytes --]

Stefan Monnier wrote:
> I'm not sure if replacing INLINE with nothing at all would lead to much
> worse code.

I assume you meant "'static'" instead of "nothing at all" - we couldn't replace 
INLINE with nothing at all, as that would cause duplicate function definitions.

I did try 'static' (actually, 'static ATTRIBUTE_UNUSED' to pacify gcc 
-Wunused-function) and found that it improved performance of 'make 
compile-always' by 8% on my old work desktop and by 6% on a newer machine, so I 
installed the attached patch. Using 'static' also shrank the text size of the 
executable by 4.5%, for what that's worth. 'static' also has the virtue of being 
simpler. At some point we can remove the EMACS_EXTERN_INLINE code as I expect it 
won't be needed.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Use-plain-static-for-Emacs-C-inline-functions.patch --]
[-- Type: text/x-patch; name="0001-Use-plain-static-for-Emacs-C-inline-functions.patch", Size: 3549 bytes --]

From 365dad197bac5deec9244fd9c189d23c46c99b31 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Wed, 4 Sep 2019 23:13:54 -0700
Subject: [PATCH] =?UTF-8?q?Use=20plain=20=E2=80=98static=E2=80=99=20for=20?=
 =?UTF-8?q?Emacs=20C=20inline=20functions?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This improved performance of ‘make compile-always’ by 8.2%
on my platform (AMD Phenom II X4 910e, Fedora 30 x86-64).
* src/conf_post.h (INLINE, EXTERN_INLINE, INLINE_HEADER_BEGIN)
(INLINE_HEADER_END) [!EMACS_EXTERN_INLINE]: Use plain ‘static’.
---
 src/conf_post.h | 43 ++++++++++++++++++++++++++++++++++---------
 1 file changed, 34 insertions(+), 9 deletions(-)

diff --git a/src/conf_post.h b/src/conf_post.h
index 4af1ba9331..43f98620a4 100644
--- a/src/conf_post.h
+++ b/src/conf_post.h
@@ -373,8 +373,13 @@ #define ATTRIBUTE_MALLOC_SIZE(args) ATTRIBUTE_MALLOC ATTRIBUTE_ALLOC_SIZE (args)
 #undef noinline
 #endif
 
-/* Use Gnulib's extern-inline module for extern inline functions.
-   An include file foo.h should prepend FOO_INLINE to function
+/* INLINE marks functions defined in Emacs-internal C headers.
+   INLINE is implemented via C99-style 'extern inline' if Emacs is built
+   with -DEMACS_EXTERN_INLINE; otherwise it is implemented via 'static'.
+   EMACS_EXTERN_INLINE is no longer the default, as 'static' seems to
+   have better performance with GCC.
+
+   An include file foo.h should prepend INLINE to function
    definitions, with the following overall pattern:
 
       [#include any other .h files first.]
@@ -399,20 +404,40 @@ #define ATTRIBUTE_MALLOC_SIZE(args) ATTRIBUTE_MALLOC ATTRIBUTE_ALLOC_SIZE (args)
    For Emacs, this is done by having emacs.c first '#define INLINE
    EXTERN_INLINE' and then include every .h file that uses INLINE.
 
-   The INLINE_HEADER_BEGIN and INLINE_HEADER_END suppress bogus
-   warnings in some GCC versions; see ../m4/extern-inline.m4.
+   The INLINE_HEADER_BEGIN and INLINE_HEADER_END macros suppress bogus
+   warnings in some GCC versions; see ../m4/extern-inline.m4.  */
+
+#ifdef EMACS_EXTERN_INLINE
+
+/* Use Gnulib's extern-inline module for extern inline functions.
 
    C99 compilers compile functions like 'incr' as C99-style extern
    inline functions.  Buggy GCC implementations do something similar with
    GNU-specific keywords.  Buggy non-GCC compilers use static
    functions, which bloats the code but is good enough.  */
 
-#ifndef INLINE
-# define INLINE _GL_INLINE
+# ifndef INLINE
+#  define INLINE _GL_INLINE
+# endif
+# define EXTERN_INLINE _GL_EXTERN_INLINE
+# define INLINE_HEADER_BEGIN _GL_INLINE_HEADER_BEGIN
+# define INLINE_HEADER_END _GL_INLINE_HEADER_END
+
+#else
+
+/* Use 'static' instead of 'extern inline' because 'static' typically
+   has better performance for Emacs.  Do not use the 'inline' keyword,
+   as modern compilers inline automatically.  ATTRIBUTE_UNUSED
+   pacifies gcc -Wunused-function.  */
+
+# ifndef INLINE
+#  define INLINE EXTERN_INLINE
+# endif
+# define EXTERN_INLINE static ATTRIBUTE_UNUSED
+# define INLINE_HEADER_BEGIN
+# define INLINE_HEADER_END
+
 #endif
-#define EXTERN_INLINE _GL_EXTERN_INLINE
-#define INLINE_HEADER_BEGIN _GL_INLINE_HEADER_BEGIN
-#define INLINE_HEADER_END _GL_INLINE_HEADER_END
 
 /* 'int x UNINIT;' is equivalent to 'int x;', except it cajoles GCC
    into not warning incorrectly about use of an uninitialized variable.  */
-- 
2.17.1