bug#38492: 27.0.50; Warn pdumper users when pure space has been overflowed

bug#38492: 27.0.50; Warn pdumper users when pure space has been overflowed

[Kévin Le Gouguec]

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

Hello,

For some reason, one of my setups[1] experiences a pure storage overflow
with the current master[2].  Searching previous bug reports for the
warning[3] led me to understand that increasing BASE_PURESIZE in
src/puresize.h would solve the issue; since I didn't have the patience
to recompile possibly multiple times, I tried to find a way to know
exactly how much I should increase this value.

I noticed that Fdump_emacs uses check_pure_size from alloc.c to print a
warning before dumping when this happens.  The following patch exposes
check_pure_size when we HAVE_PDUMPER, so that Fdump_emacs_portable can
use it too.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Warn-pdumper-users-when-pure-space-has-been-overflow.patch --]
[-- Type: text/x-diff, Size: 1180 bytes --]

From 02a48f2388932ae78460a76f74e27202212994d4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?K=C3=A9vin=20Le=20Gouguec?= <kevin.legouguec@gmail.com>
Date: Wed, 4 Dec 2019 18:22:29 +0100
Subject: [PATCH] Warn pdumper users when pure space has been overflowed

* src/alloc.c (check_pure_size): Expose when using the portable
dumper.
* src/pdumper.c (Fdump_emacs_portable): Use it.
---
 src/alloc.c   | 2 +-
 src/pdumper.c | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/alloc.c b/src/alloc.c
index 9fbd0d0573..52ea87c7b7 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -5125,7 +5125,7 @@ pure_alloc (size_t size, int type)
 }
 
 
-#ifdef HAVE_UNEXEC
+#if defined HAVE_UNEXEC || defined HAVE_PDUMPER
 
 /* Print a warning if PURESIZE is too small.  */
 
diff --git a/src/pdumper.c b/src/pdumper.c
index 74f198c4ae..721d23e696 100644
--- a/src/pdumper.c
+++ b/src/pdumper.c
@@ -4004,6 +4004,8 @@ DEFUN ("dump-emacs-portable",
 {
   eassert (initialized);
 
+  check_pure_size ();
+
   if (will_dump_with_unexec_p ())
     error ("This Emacs instance was started under the assumption "
            "that it would be dumped with unexec, not the portable "
-- 
2.20.1


[-- Attachment #3: Type: text/plain, Size: 6025 bytes --]


That gets me a helpful error message at compile-time[4] (as well as an
intriguing warning[5]) that let me figure out how much I should increase
BASE_PURESIZE by, recompile Emacs and run it with no further warnings
about pure storage overflow.

Would it make sense to apply this?  (Maybe not as-is: e.g. I don't know
if the #if guard is still useful at this point; maybe the call to
check_pure_size should be moved somewhere else…)

Thank you for your time.


[1] Debian Buster on Samsung NC10 (32-bit, 2GB RAM).

[2] 8bea7e9ab4453da71d9766d582089154f31de907

[3] Warning (initialization): Building Emacs overflowed pure space.  (See the node Pure Storage in the Lisp manual for details.)

[4] emacs:0:Pure Lisp storage overflow (approx. 2005404 bytes needed)

[5] alloc.c:5136:14: warning: format ‘%d’ expects argument of type ‘int’, but argument 2 has type ‘intmax_t’ {aka ‘long long int’} [-Wformat=]
         message (("emacs:0:Pure Lisp storage overflow (approx. %"pI"d"
                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            " bytes needed)"),
            ~~~~~~~~~~~~~~~~~

    I tried to puzzle this one out by looking at the definition for pI
    in lisp.h, but I am not sure how to proceed.  The warning suggests
    that pI is defined as "" since the final format is %d, which means
    that I should look into this snippet:

    # elif INTPTR_MAX <= INT_MAX && !defined WIDE_EMACS_INT
    typedef int EMACS_INT;
    typedef unsigned int EMACS_UINT;
    enum { EMACS_INT_WIDTH = INT_WIDTH, EMACS_UINT_WIDTH = UINT_WIDTH };
    #  define EMACS_INT_MAX INT_MAX
    #  define pI ""

    … I can probably dig into this to see what's wrong on my setup[1],
    but I'll bank on someone more experienced going "Oh well duh" and
    fixing whatever needs fixing (assuming the problem isn't on my end).


In GNU Emacs 27.0.50 (build 11, i686-pc-linux-gnu, GTK+ Version 3.24.5, cairo version 1.16.0)
 of 2019-11-17 built on little-buster
Repository revision: 1c29ba034092660e73bce8c6ff459c75ff2c2d72
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12004000
System Description: Debian GNU/Linux 10 (buster)

Configured using:
 'configure --with-xwidgets --with-cairo'

Configured features:
XPM JPEG TIFF GIF PNG RSVG CAIRO SOUND GPM DBUS GSETTINGS GLIB NOTIFY
INOTIFY ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE HARFBUZZ M17N_FLT LIBOTF
ZLIB TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM MODULES THREADS XWIDGETS
LIBSYSTEMD JSON PDUMPER LCMS2 GMP

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Features:
(shadow sort mail-extr emacsbug sendmail etags fileloop generator xref
vc-mtn vc-hg mule-util warnings filecache markdown-mode rx color
noutline outline help-fns radix-tree magit-extras sh-script smie
flyspell ispell executable cus-edit wid-edit whitespace notifications
dbus xml misearch multi-isearch bug-reference cc-mode cc-fonts cc-guess
cc-menus cc-cmds cc-styles cc-align cc-engine cc-vars cc-defs thingatpt
magit-submodule magit-obsolete magit-blame magit-stash magit-reflog
magit-bisect magit-push magit-pull magit-fetch magit-clone magit-remote
magit-commit magit-sequence magit-notes magit-worktree magit-tag
magit-merge magit-branch magit-reset magit-files magit-refs magit-status
magit magit-repos magit-apply magit-wip magit-log which-func imenu
magit-diff smerge-mode magit-core magit-autorevert autorevert filenotify
magit-margin magit-transient magit-process magit-mode git-commit
magit-git magit-section magit-utils crm log-edit message rmc puny dired
dired-loaddefs rfc822 mml mml-sec epa derived epg epg-config gnus-util
rmail rmail-loaddefs text-property-search time-date mm-decode mm-bodies
mm-encode mail-parse rfc2231 rfc2047 rfc2045 mm-util ietf-drums
mail-prsvr mailabbrev mail-utils gmm-utils mailheader pcvs-util add-log
with-editor async-bytecomp async shell pcomplete server dash vc-git
vc-bzr vc-src vc-sccs vc-svn vc-cvs vc-rcs project delight
eighters-theme rg rg-info-hack advice rg-menu transient cl-extra
format-spec rg-ibuffer rg-result wgrep-rg wgrep s rg-history rg-header
ibuf-ext ibuffer ibuffer-loaddefs grep compile comint ansi-color ring
quail help-mode edmacro kmacro disp-table paren mb-depth icomplete
page-break-lines elec-pair diff-hl-flydiff diff diff-hl vc-dir ewoc vc
vc-dispatcher diff-mode easy-mmode delsel cus-start cus-load tex-site
info package easymenu browse-url url-handlers url-parse auth-source
cl-seq eieio eieio-core cl-macs eieio-loaddefs password-cache json
subr-x map url-vars seq byte-opt gv bytecomp byte-compile cconv
cl-loaddefs cl-lib tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel term/x-win x-win term/common-win x-dnd tool-bar
dnd fontset image regexp-opt fringe tabulated-list replace newcomment
text-mode elisp-mode lisp-mode prog-mode register page tab-bar menu-bar
rfn-eshadow isearch timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core term/tty-colors frame minibuffer cl-generic
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote threads dbusbind
inotify lcms2 dynamic-setting system-font-setting font-render-setting
xwidget-internal cairo move-toolbar gtk x-toolkit x multi-tty
make-network-process emacs)

Memory information:
((conses 8 330562 41453)
 (symbols 24 23104 2)
 (strings 16 73924 11567)
 (string-bytes 1 2445511)
 (vectors 8 42019)
 (vector-slots 4 959888 63404)
 (floats 8 281 269)
 (intervals 28 21802 357)
 (buffers 568 52))

bug#38492: 27.0.50; Warn pdumper users when pure space has been overflowed

[Eli Zaretskii]

> From: Kévin Le Gouguec <kevin.legouguec@gmail.com>
> Date: Wed, 04 Dec 2019 20:02:12 +0100
> 
> For some reason, one of my setups[1] experiences a pure storage overflow
> with the current master[2].  Searching previous bug reports for the
> warning[3] led me to understand that increasing BASE_PURESIZE in
> src/puresize.h would solve the issue; since I didn't have the patience
> to recompile possibly multiple times, I tried to find a way to know
> exactly how much I should increase this value.
> 
> I noticed that Fdump_emacs uses check_pure_size from alloc.c to print a
> warning before dumping when this happens.  The following patch exposes
> check_pure_size when we HAVE_PDUMPER, so that Fdump_emacs_portable can
> use it too.

I'm not sure we should care about overflowing the pure space in
pdumped Emacs.  Why does it matter?  It mattered in unexec'ed Emacs,
that's why we warned about it.  Maybe the right solution is to remove
the warning from startup.el instead.

Daniel, what are your thoughts about this?

bug#38492: 27.0.50; Warn pdumper users when pure space has been overflowed

[Kévin Le Gouguec]

Eli Zaretskii <eliz@gnu.org> writes:

> I'm not sure we should care about overflowing the pure space in
> pdumped Emacs.  Why does it matter?  It mattered in unexec'ed Emacs,
> that's why we warned about it.  Maybe the right solution is to remove
> the warning from startup.el instead.

Sorry for the false alarm then.  I took the startup warning at face
value; I didn't see anything in (elisp)Pure Storage suggesting that the
portable dumper invalidated it.

Had I tried to run garbage-collect manually, I would have noticed that
the return value was non-nil, suggesting that GC wasn't disabled…

bug#38492: 27.0.50; Warn pdumper users when pure space has been overflowed

[Daniel Colascione]

On 12/6/19 8:37 AM, Kévin Le Gouguec wrote:
> Eli Zaretskii <eliz@gnu.org> writes:
> 
>> I'm not sure we should care about overflowing the pure space in
>> pdumped Emacs.  Why does it matter?  It mattered in unexec'ed Emacs,
>> that's why we warned about it.  Maybe the right solution is to remove
>> the warning from startup.el instead.
> 
> Sorry for the false alarm then.  I took the startup warning at face
> value; I didn't see anything in (elisp)Pure Storage suggesting that the
> portable dumper invalidated it.
> 
> Had I tried to run garbage-collect manually, I would have noticed that
> the return value was non-nil, suggesting that GC wasn't disabled…

We should probably consider just removing the pure space mechanism at 
some point.

bug#38492: 27.0.50; Warn pdumper users when pure space has been overflowed

[Eli Zaretskii]

> Cc: 38492@debbugs.gnu.org
> From: Daniel Colascione <dancol@dancol.org>
> Date: Fri, 6 Dec 2019 08:49:04 -0800
> 
> We should probably consider just removing the pure space mechanism at 
> some point.

Does this mean you agree with removing the warning in startup.el
about overflowed pure space?

bug#38492: 27.0.50; Warn pdumper users when pure space has been overflowed

[dancol]

[-- Attachment #1: Type: text/html, Size: 527 bytes --]

bug#38492: 27.0.50; Warn pdumper users when pure space has been overflowed

[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. ]]]

With portable dumping. I think pure space provides no benefit.
So it would be fine to turn off use of it in that case.

But I think it would be a mistake to delete the code for pure space
until we are sure that everything other than portable dumpimng is
obsolete.

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

bug#38492: 27.0.50; Warn pdumper users when pure space has been overflowed

[Eli Zaretskii]

> From: Richard Stallman <rms@gnu.org>
> Cc: eliz@gnu.org, 38492@debbugs.gnu.org, kevin.legouguec@gmail.com
> Date: Fri, 06 Dec 2019 23:48:22 -0500
> 
> With portable dumping. I think pure space provides no benefit.
> So it would be fine to turn off use of it in that case.
> 
> But I think it would be a mistake to delete the code for pure space
> until we are sure that everything other than portable dumpimng is
> obsolete.

We are not going to delete the pure space code in Emacs 27, because we
still support unexec there.  I think I will just remove the warning in
startup.el about pure-space overflow.

bug#38492: 27.0.50; Warn pdumper users when pure space has been overflowed

[Eli Zaretskii]

> Date: Fri, 06 Dec 2019 11:02:28 -0800
> From: dancol@dancol.org
> Cc: kevin.legouguec@gmail.com, 38492@debbugs.gnu.org
> 
> > > We should probably consider just removing the pure space mechanism at 
> > > some point. 
> >
> > Does this mean you agree with removing the warning in startup.el 
> > about overflowed pure space?
> 
> Yes. 

Thanks, I removed the warning in a pdumper build, and I'm closing this
bug report.

bug#38492: 27.0.50; Warn pdumper users when pure space has been overflowed

[Kévin Le Gouguec]

Eli Zaretskii <eliz@gnu.org> writes:

> Thanks, I removed the warning in a pdumper build, and I'm closing this
> bug report.

Thanks!  I had started looking at the ifs and whens in startup.el, but
wasn't sure how to translate "have we been dumped with pdumper" into
Lisp: dumped_with_pdumper_p is not callable from Lisp, I wasn't sure
about pdumper-stats's perennity, and dump-mode is only non-nil when
Emacs is dumping itself.

I'm glad you solved this conundrum for me :)

bug#38492: 27.0.50; Warn pdumper users when pure space has been overflowed

[Stefan Monnier]

>> > We should probably consider just removing the pure space mechanism at 
>> > some point. 
>> Does this mean you agree with removing the warning in startup.el 
>> about overflowed pure space?
> Yes. 

Why?

This warning was placed because we needed to disable GC in dumps that
had overflown the purespace, so the dumped Emacs was not
fully functional.

I see that with the current pdump we don't disable GC in dumps that
overflew the purespace.  Why is that?

IIRC, the problem with running GC in an Emacs with overflown purespace
is that overflowing the purespace created pointers from the purespace to
the heap and those aren't traced by the GC so it resulted in core dumps.

I can't exactly remember the cases where this happened, but I can't see
why the pdumper would be less affected than the unexec code.

I see that the code has a slightly different explanation:

    /* Can't GC if pure storage overflowed because we can't determine
       if something is a pure object or not.  */
    garbage_collection_inhibited++;

but that comment similarly seems to apply to pdump just as much as it
applies to unexec.


        Stefan

bug#38492: 27.0.50; Warn pdumper users when pure space has been overflowed

[Daniel Colascione]

>>> > We should probably consider just removing the pure space mechanism at
>>> > some point.
>>> Does this mean you agree with removing the warning in startup.el
>>> about overflowed pure space?
>> Yes.
>
> Why?
>
> This warning was placed because we needed to disable GC in dumps that
> had overflown the purespace, so the dumped Emacs was not
> fully functional.
>
> I see that with the current pdump we don't disable GC in dumps that
> overflew the purespace.  Why is that?
>
> IIRC, the problem with running GC in an Emacs with overflown purespace
> is that overflowing the purespace created pointers from the purespace to
> the heap and those aren't traced by the GC so it resulted in core dumps.
>
> I can't exactly remember the cases where this happened, but I can't see
> why the pdumper would be less affected than the unexec code.
>
> I see that the code has a slightly different explanation:
>
>     /* Can't GC if pure storage overflowed because we can't determine
>        if something is a pure object or not.  */
>     garbage_collection_inhibited++;
>
> but that comment similarly seems to apply to pdump just as much as it
> applies to unexec.

In a pdumped Emacs, we don't have anything in purespace on pdumper load,
so we can GC normally. We do disable GC during dump operation itself if we
overflow, true, and we shouldn't do that, but this problem affects only
the build.

We should just make sure Vpurify_flag is off if we're going to make a
pdumper image.

bug#38492: 27.0.50; Warn pdumper users when pure space has been overflowed

[Stefan Monnier]

> With portable dumping.  I think pure space provides no benefit.

AFAICT, the purespace provides the same benefit with portable dumping as
it does with unexec.  The benefit is to save time during the GC by not
traversing the purespace.  It's basically an "old" generation in a kind
of very restricted form of generational GC.

Maybe purespace is too small compared to typical heap sizes to make
a significant difference, nowadays.


        Stefan

bug#38492: 27.0.50; Warn pdumper users when pure space has been overflowed

[Daniel Colascione]

>> With portable dumping.  I think pure space provides no benefit.
>
> AFAICT, the purespace provides the same benefit with portable dumping as
> it does with unexec.  The benefit is to save time during the GC by not
> traversing the purespace.  It's basically an "old" generation in a kind
> of very restricted form of generational GC.
>
> Maybe purespace is too small compared to typical heap sizes to make
> a significant difference, nowadays.

I think the ratio of dumped-heap to runtime-heap is high enough these days
that the pure optimization you're describing doesn't really matter. I
think we need a better general-purpose GC that does generational tracking
even after the dump and without purespace's restriction against
intergenerational pointers. Whether that's something we reuse or write, I
don't know yet, but I have some ideas for how we can combine these
techniques with our conservative stack scanning.

bug#38492: 27.0.50; Warn pdumper users when pure space has been overflowed

[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. ]]]

  > AFAICT, the purespace provides the same benefit with portable dumping as
  > it does with unexec.  The benefit is to save time during the GC by not
  > traversing the purespace.  It's basically an "old" generation in a kind
  > of very restricted form of generational GC.

I never thought about it that way, but I guess you are right.
(The original intended benefit was to share memory between Emacs processes.)

Perhaps for this reason we should keep pure space.

It would be interesting to measure the effect on eliminating pure space
on the speed of a GC-intensive task.

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

bug#38492: 27.0.50; Warn pdumper users when pure space has been overflowed

[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. ]]]

  > >> > We should probably consider just removing the pure space mechanism at 
  > >> > some point. 
  > >> Does this mean you agree with removing the warning in startup.el 
  > >> about overflowed pure space?
  > > Yes. 

  > Why?

  > This warning was placed because we needed to disable GC in dumps that
  > had overflown the purespace, so the dumped Emacs was not
  > fully functional.

If pure space provides some benefit with portable dumping, so we keep
it, we need something to inform developers that we need to increase
the size of pure space.  That warning is the way we do that.

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