bug#31852: Make memory-limit obsolete

bug#31852: Make memory-limit obsolete

[Paul Eggert]

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

Tags: patch

The memory-limit function has an implementation that is a relic of 
long-ago days when the heap was allocated via sbrk. This assumption is 
often no longer true now that ASLR is popular, and once we get portable 
dumping work it'll be true even less often. Since memory-limit returns 
nonsense so often and since nobody cares, we can and should mark 
memory-limit as obsolete. On its way out we can move it to a Lisp 
implementation and cause it to be at least somewhat more plausible.

Proposed patches attached. The first patch moves memory-limit to Lisp, 
the second one marks it obsolete. I didn't know where to put the Lisp 
implementation so I put it into lisp/subr.el.


[-- Attachment #2: 0001-Rewrite-memory-limit-in-Lisp.patch --]
[-- Type: text/x-patch, Size: 3715 bytes --]

From 52f4585c1353c3c12079bc7bfe85ef44755134db Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Fri, 15 Jun 2018 18:05:28 -0700
Subject: [PATCH 1/2] Rewrite memory-limit in Lisp

Have it return Emacs virtual memory size, not the sbrk value
which is often useless newadays.
* doc/lispref/internals.texi (Garbage Collection):
* etc/NEWS: Document this.
* lisp/subr.el (memory-limit): New implementation in Lisp,
written in terms of process-attributes, and which returns
virtual memory size.
* src/alloc.c (Fmemory_limit): Remove C implementation.
---
 doc/lispref/internals.texi |  6 ++----
 etc/NEWS                   |  3 +++
 lisp/subr.el               |  4 ++++
 src/alloc.c                | 19 -------------------
 4 files changed, 9 insertions(+), 23 deletions(-)

diff --git a/doc/lispref/internals.texi b/doc/lispref/internals.texi
index 9cf1a4f9a3..faaf26f4f7 100644
--- a/doc/lispref/internals.texi
+++ b/doc/lispref/internals.texi
@@ -507,10 +507,8 @@ Garbage Collection
 memory Emacs is currently using.
 
 @defun memory-limit
-This function returns the address of the last byte Emacs has allocated,
-divided by 1024.  We divide the value by 1024 to make sure it fits in a
-Lisp integer.
-
+This function returns an estimate of the total amount of bytes of
+virtual memory that Emacs is currently using, divided by 1024.
 You can use this to get a general idea of how your actions affect the
 memory usage.
 @end defun
diff --git a/etc/NEWS b/etc/NEWS
index e89402db13..30409a89b5 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -656,6 +656,9 @@ socket has been passed to Emacs (Bug#24218).
 instead of just Microsoft platforms.  This fixes a 'get-free-disk-space'
 bug on OS X 10.8 and later (Bug#28639).
 
++++
+** 'memory-limit' now returns a better estimate of memory consumption.
+
 +++
 ** New macro 'combine-change-calls' arranges to call the change hooks
 ('before-change-functions' and 'after-change-functions') just once
diff --git a/lisp/subr.el b/lisp/subr.el
index 914112ccef..e4da2b2075 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -2182,6 +2182,10 @@ process-put
   (set-process-plist process
 		     (plist-put (process-plist process) propname value)))
 
+(defun memory-limit ()
+  "Return an estimate of Emacs virtual memory usage, divided by 1024."
+  (or (cdr (assq 'vsize (process-attributes (emacs-pid)))) 0))
+
 \f
 ;;;; Input and display facilities.
 
diff --git a/src/alloc.c b/src/alloc.c
index 7b2140501e..52620a54ac 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -7120,24 +7120,6 @@ or memory information can't be obtained, return nil.  */)
 
 /* Debugging aids.  */
 
-DEFUN ("memory-limit", Fmemory_limit, Smemory_limit, 0, 0, 0,
-       doc: /* Return the address of the last byte Emacs has allocated, divided by 1024.
-This may be helpful in debugging Emacs's memory usage.
-We divide the value by 1024 to make sure it fits in a Lisp integer.  */)
-  (void)
-{
-  Lisp_Object end;
-
-#if defined HAVE_NS || defined __APPLE__ || !HAVE_SBRK
-  /* Avoid warning.  sbrk has no relation to memory allocated anyway.  */
-  XSETINT (end, 0);
-#else
-  XSETINT (end, (intptr_t) (char *) sbrk (0) / 1024);
-#endif
-
-  return end;
-}
-
 DEFUN ("memory-use-counts", Fmemory_use_counts, Smemory_use_counts, 0, 0, 0,
        doc: /* Return a list of counters that measure how much consing there has been.
 Each of these counters increments for a certain kind of object.
@@ -7495,7 +7477,6 @@ The time is in seconds as a floating point value.  */);
   defsubr (&Smake_finalizer);
   defsubr (&Spurecopy);
   defsubr (&Sgarbage_collect);
-  defsubr (&Smemory_limit);
   defsubr (&Smemory_info);
   defsubr (&Smemory_use_counts);
   defsubr (&Ssuspicious_object);
-- 
2.17.1


[-- Attachment #3: 0002-Obsolete-memory-limit.patch --]
[-- Type: text/x-patch, Size: 2133 bytes --]

From 5645d547959780e21fe7c5eff12a0d0ae22702da Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Fri, 15 Jun 2018 18:21:10 -0700
Subject: [PATCH 2/2] Obsolete memory-limit

* doc/lispref/internals.texi (Garbage Collection):
Remove documentation of memory-limit.
* etc/NEWS: Say it's obsolete.
* lisp/subr.el (memory-limit): Declare that it's obsolete.
---
 doc/lispref/internals.texi | 7 -------
 etc/NEWS                   | 3 ++-
 lisp/subr.el               | 1 +
 3 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/doc/lispref/internals.texi b/doc/lispref/internals.texi
index faaf26f4f7..e8fe26856c 100644
--- a/doc/lispref/internals.texi
+++ b/doc/lispref/internals.texi
@@ -506,13 +506,6 @@ Garbage Collection
 function @code{memory-limit} provides information on the total amount of
 memory Emacs is currently using.
 
-@defun memory-limit
-This function returns an estimate of the total amount of bytes of
-virtual memory that Emacs is currently using, divided by 1024.
-You can use this to get a general idea of how your actions affect the
-memory usage.
-@end defun
-
 @defvar memory-full
 This variable is @code{t} if Emacs is nearly out of memory for Lisp
 objects, and @code{nil} otherwise.
diff --git a/etc/NEWS b/etc/NEWS
index 30409a89b5..a1f853e8cd 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -657,7 +657,8 @@ instead of just Microsoft platforms.  This fixes a 'get-free-disk-space'
 bug on OS X 10.8 and later (Bug#28639).
 
 +++
-** 'memory-limit' now returns a better estimate of memory consumption.
+** The 'memory-limit' function is now obsolete.  You can use
+the 'vsize' component of (process-attributes (emacs-pid)) instead.
 
 +++
 ** New macro 'combine-change-calls' arranges to call the change hooks
diff --git a/lisp/subr.el b/lisp/subr.el
index e4da2b2075..6af32c95bc 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -2184,6 +2184,7 @@ process-put
 
 (defun memory-limit ()
   "Return an estimate of Emacs virtual memory usage, divided by 1024."
+  (declare (obsolete process-attributes "27.1"))
   (or (cdr (assq 'vsize (process-attributes (emacs-pid)))) 0))
 
 \f
-- 
2.17.1

bug#31852: Make memory-limit obsolete

[Eli Zaretskii]

> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Fri, 15 Jun 2018 18:33:47 -0700
> 
> The memory-limit function has an implementation that is a relic of 
> long-ago days when the heap was allocated via sbrk. This assumption is 
> often no longer true now that ASLR is popular, and once we get portable 
> dumping work it'll be true even less often. Since memory-limit returns 
> nonsense so often and since nobody cares, we can and should mark 
> memory-limit as obsolete. On its way out we can move it to a Lisp 
> implementation and cause it to be at least somewhat more plausible.
> 
> Proposed patches attached. The first patch moves memory-limit to Lisp, 
> the second one marks it obsolete. I didn't know where to put the Lisp 
> implementation so I put it into lisp/subr.el.

I don't have anything against moving this to Lisp (provided that all
the platforms we care about have a non-trivial implementation of
process-attributes), but I don't understand what we gain by declaring
this tiny function obsolete, especially since the alternative proposed
in the warning is exactly what's used in the proposed Lisp
implementation.  Wouldn't it be enough just to add to the function's
documentation a note that the estimation should be expected to be
inaccurate with modern memory-management technologies?

Thanks.

bug#31852: Make memory-limit obsolete

[Paul Eggert]

Eli Zaretskii wrote:
> I don't understand what we gain by declaring
> this tiny function obsolete

Nobody uses the function (clearly, since it doesn't work) so we gain simplicity, 
which is a win.

> Wouldn't it be enough just to add to the function's
> documentation a note that the estimation should be expected to be
> inaccurate with modern memory-management technologies?

Complicating the documentation would work against the goal of simplicity. If the 
goal is something other than simplicity, it'd be helpful to know what the goal 
is before proposing a different patch 2. The idea behind patch 2 is that 
simplicity trumps compatibility here, as nobody uses the function and nobody 
will use it even if patch 1 is applied, partly because of the inaccuracies that 
you mention.

bug#31852: Make memory-limit obsolete

[Eli Zaretskii]

> Cc: 31852@debbugs.gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Sat, 16 Jun 2018 06:38:55 -0700
> 
> Eli Zaretskii wrote:
> > I don't understand what we gain by declaring
> > this tiny function obsolete
> 
> Nobody uses the function (clearly, since it doesn't work) so we gain simplicity, 
> which is a win.

If nobody uses the function, I see no gain at all, because obsoleting
a function tells those who use it not to.

> > Wouldn't it be enough just to add to the function's
> > documentation a note that the estimation should be expected to be
> > inaccurate with modern memory-management technologies?
> 
> Complicating the documentation would work against the goal of
> simplicity.

Then let's just move this to Lisp, and do nothing else.  That's even
simpler, IMO.

bug#31852: Make memory-limit obsolete

[Paul Eggert]

Eli Zaretskii wrote:

> If nobody uses the function, I see no gain at all, because obsoleting
> a function tells those who use it not to.

If Emacs has useless and confusing features that consume documentation space and 
implementation resources, that is a net harm to current and future users. 
Omitting such features is therefore a net gain to users. The benefit to users by 
omitting useless and confusing features is worth the maintenance cost to us of 
obsoleting these features.

That being said, it appears I haven't convinced you to obsolete memory-limit, so 
I installed patch 1 but not patch 2 and am closing the bug report.

bug#31852: Make memory-limit obsolete

[Daniel Colascione]

> Eli Zaretskii wrote:
>
>> If nobody uses the function, I see no gain at all, because obsoleting
>> a function tells those who use it not to.
>
> If Emacs has useless and confusing features that consume documentation
> space and
> implementation resources, that is a net harm to current and future users.
> Omitting such features is therefore a net gain to users. The benefit to
> users by
> omitting useless and confusing features is worth the maintenance cost to
> us of
> obsoleting these features.
>
> That being said, it appears I haven't convinced you to obsolete
> memory-limit, so
> I installed patch 1 but not patch 2 and am closing the bug report.

I agree with obsoleting memory-limit. I don't like the vsize patch you
installed though: what does the amount of address space reserved actually
tell anyone? There's no relationship to actual resource consumption.

IMHO, memory-limit should return a combination of the Lisp heap size with
whatever malloc says is its own heap size.

bug#31852: Make memory-limit obsolete

[Paul Eggert]

Daniel Colascione wrote:
> I agree with obsoleting memory-limit. I don't like the vsize patch you
> installed though: what does the amount of address space reserved actually
> tell anyone? There's no relationship to actual resource consumption.

It's not that good a number, true; but it's better than the number we were 
returning, which was typically nonsense.

One of the benefits of moving memory-limit to Lisp was the removal of the bogus 
call to sbrk. Emacs should never call sbrk, and once the portable dumper code is 
merged, Emacs won't ever need to call sbrk again. Yay!

All things considered, I'd rather not go down the rabbit hole of trying to 
improve memory-limit further, because memory-limit was a mistake in the first 
place and nobody uses it (and rightly so) and we should be deprecating it rather 
than trying to improve it. That being said, if you'd like to make it better 
please feel free.