Feature request: REPORTMEMORY

Feature request: REPORTMEMORY

[Shaun Jackman]

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

The REPORTTIME feature of zsh is super useful. May I make a feature request for REPORTMEMORY? Reporting the peak RSS/VSIZE would be terribly useful, similar to GNU time.

Thanks!
Shaun

-- 
http://sjackman.ca

Re: Feature request: REPORTMEMORY

[Peter Stephenson]

On Wed, 08 Jun 2016 12:44:36 -0700
Shaun Jackman <sjackman@gmail.com> wrote:
> The REPORTTIME feature of zsh is super useful. May I make a feature
> request for REPORTMEMORY? Reporting the peak RSS/VSIZE would be
> terribly useful, similar to GNU time.

This is fairly straightforward as long as we restrict ourselves to the
maximum RSS, which is available from getrusage() --- other values are
harder to get at and aren't supported on Linux even though they appear
in the structure so are hard to test for.  Presumably this is good
enough.

The value is in megabytes because that's the same as what the TIMEFMT %M
escape prints, though in principle the OS reports down to kilobytes
(what its actual granularity is is another matter).  Note you need to
add output of %M to TIMEFMT by hand --- I've documented this.

pws

diff --git a/Doc/Zsh/params.yo b/Doc/Zsh/params.yo
index d23c459..b02d24e 100644
--- a/Doc/Zsh/params.yo
+++ b/Doc/Zsh/params.yo
@@ -1390,6 +1390,20 @@ item(tt(READNULLCMD) <S>)(
 The command name to assume if a single input redirection
 is specified with no command.  Defaults to tt(more).
 )
+vindex(REPORTMEMORY)
+item(tt(REPORTMEMORY))(
+If nonnegative, commands whose maximum resident set size (roughly
+speaking, main memory usage) in megabytes is greater than this
+value have timing statistics reported.  The format used to output
+statistics is the value of the tt(TIMEFMT) parameter, which is the same
+as for the tt(REPORTTIME) variable and the tt(time) builtin; note that
+by default this does not output memory usage.  Appending
+tt(" max RSS %M") to the value of tt(TIMEFMT) causes it to output the
+value that triggered the report.  If tt(REPORTTIME) is also in use,
+at most a single report is printed for both triggers.  This feature
+requires the tt(getrusage()) system call, commonly supported by modern
+Unix-like systems.
+)
 vindex(REPORTTIME)
 item(tt(REPORTTIME))(
 If nonnegative, commands whose combined user and system execution times
diff --git a/Src/jobs.c b/Src/jobs.c
index 04cb6b3..3db2ed0 100644
--- a/Src/jobs.c
+++ b/Src/jobs.c
@@ -884,37 +884,62 @@ should_report_time(Job j)
     struct value vbuf;
     Value v;
     char *s = "REPORTTIME";
-    zlong reporttime;
+    zlong reporttime = -1;
+#ifdef HAVE_GETRUSAGE
+    char *sm = "REPORTMEMORY";
+    zlong reportmemory = -1;
+#endif
 
     /* if the time keyword was used */
     if (j->stat & STAT_TIMED)
 	return 1;
 
     queue_signals();
-    if (!(v = getvalue(&vbuf, &s, 0)) ||
-	(reporttime = getintvalue(v)) < 0) {
-	unqueue_signals();
-	return 0;
-    }
+    if ((v = getvalue(&vbuf, &s, 0)))
+	reporttime = getintvalue(v);
+#ifdef HAVE_GETRUSAGE
+    if ((v = getvalue(&vbuf, &sm, 0)))
+	reportmemory = getintvalue(v);
+#endif
     unqueue_signals();
+    if (reporttime < 0
+#ifdef HAVE_GETRUSAGE
+	&& reportmemory < 0
+#endif
+	)
+	return 0;
     /* can this ever happen? */
     if (!j->procs)
 	return 0;
     if (zleactive)
 	return 0;
 
+    if (reporttime >= 0)
+    {
 #ifdef HAVE_GETRUSAGE
-    reporttime -= j->procs->ti.ru_utime.tv_sec + j->procs->ti.ru_stime.tv_sec;
-    if (j->procs->ti.ru_utime.tv_usec +
-	j->procs->ti.ru_stime.tv_usec >= 1000000)
-	reporttime--;
-    return reporttime <= 0;
+	reporttime -= j->procs->ti.ru_utime.tv_sec +
+	    j->procs->ti.ru_stime.tv_sec;
+	if (j->procs->ti.ru_utime.tv_usec +
+	    j->procs->ti.ru_stime.tv_usec >= 1000000)
+	    reporttime--;
+	if (reporttime <= 0)
+	    return 1;
 #else
-    {
-	clktck = get_clktck();
-	return ((j->procs->ti.ut + j->procs->ti.st) / clktck >= reporttime);
+	{
+	    clktck = get_clktck();
+	    if ((j->procs->ti.ut + j->procs->ti.st) / clktck >= reporttime)
+		return 1;
+	}
+#endif
     }
+
+#ifdef HAVE_GETRUSAGE
+    if (reportmemory >= 0 &&
+	j->procs->ti.ru_maxrss / 1024 > reportmemory)
+	return 1;
 #endif
+
+    return 0;
 }
 
 /* !(lng & 3) means jobs    *

Re: Feature request: REPORTMEMORY

[Peter Stephenson]

On Thu, 9 Jun 2016 11:15:32 +0100
Peter Stephenson <p.stephenson@samsung.com> wrote:
> Note you need to
> add output of %M to TIMEFMT by hand --- I've documented this.

I'm wondering about the patch below.

The trouble is there are two completely different sorts of "concerned"
user.

"It is OUTRAGEOUS that zsh DOES NOT SUPPORT ADVANCED FEATURES BY
DEFAULT.  Any alternative opinion is INCONCEIVABLE.   Froth, gibber."

"It is OUTRAGEOUS that zsh keeps being modified to support NEWER
FEATURES when ANY RIGHT THINKING PERSON can see it does far too much
already.  Froth, gibber."

(If you see what I mean.)

Strictly speaking I think we need wait3 as well as getrusage for this to
work seamlessly --- but the changes I've made are at least compatible
with how printtime() is written.

pws

diff --git a/Src/zsh_system.h b/Src/zsh_system.h
index 17c4c64..888035f 100644
--- a/Src/zsh_system.h
+++ b/Src/zsh_system.h
@@ -438,7 +438,12 @@ struct timezone {
 #endif
 
 #define DEFAULT_WORDCHARS "*?_-.[]~=/&;!#$%^(){}<>"
+#ifdef HAVE_GETRUSAGE
+#define DEFAULT_TIMEFMT						\
+    "%J  %U user %S system %P cpu %*E total, max RSS %M MB"
+#else
 #define DEFAULT_TIMEFMT   "%J  %U user %S system %P cpu %*E total"
+#endif
 
 /* Posix getpgrp takes no argument, while the BSD version *
  * takes the process ID as an argument                    */

Re: Feature request: REPORTMEMORY

[Bart Schaefer]

On Jun 9, 11:39am, Peter Stephenson wrote:
} Subject: Re: Feature request: REPORTMEMORY
}
} On Thu, 9 Jun 2016 11:15:32 +0100
} Peter Stephenson <p.stephenson@samsung.com> wrote:
} > Note you need to
} > add output of %M to TIMEFMT by hand --- I've documented this.
} 
} I'm wondering about the patch below.

Anyone parsing the output of "time" might gibber frothily, I guess,
but using "time" that way seems unlikely.  On the other hand, the
circumstances in which I care about the max resident set size of a
process that has already exited, are vanishingly rare.

Re: Feature request: REPORTMEMORY

[Peter Stephenson]

On Thu, 09 Jun 2016 23:20:35 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> Anyone parsing the output of "time" might gibber frothily, I guess,
> but using "time" that way seems unlikely.  On the other hand, the
> circumstances in which I care about the max resident set size of a
> process that has already exited, are vanishingly rare.

In my case I don't tend to bother reporting the time it took because I
can estimate that from the time it took...

I've committed it without this; the manual entry reports what you need
to do if you want to see the memory.  Psychologically this is probably a
plus as it will give the user an even bigger feeling of achievement
when it works.  (Also, can you spot three lollipops hidden in the
manual?)

I'm sure I used to take this more seriously, but I might just be
misremembering.

pws

Re: Feature request: REPORTMEMORY

[Bart Schaefer]

On Jun 13,  4:31pm, Peter Stephenson wrote:
} Subject: Re: Feature request: REPORTMEMORY
}
} [...] the manual entry reports what you need
} to do if you want to see the memory.  Psychologically this is probably a
} plus as it will give the user an even bigger feeling of achievement

Years late to be asking this, but is there some easy way to tell yodl
that a series of words is not allowed to wrap?

     as for the REPORTTIME variable and the time builtin; note that by
     default this does not output memory usage.  Appending " max RSS
     %M" to the value of TIMEFMT causes it to output the value that

It'd be nice if there were not a line break before %M there.  A few other
in-line examples elsewhere could also benefit.

} (Also, can you spot three lollipops hidden in the manual?)

(I've only found two ...)

Re: Feature request: REPORTMEMORY

[Peter Stephenson]

On Tue, 14 Jun 2016 09:25:03 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> Years late to be asking this, but is there some easy way to tell yodl
> that a series of words is not allowed to wrap?
> 
>      as for the REPORTTIME variable and the time builtin; note that by
>      default this does not output memory usage.  Appending " max RSS
>      %M" to the value of TIMEFMT causes it to output the value that

It depends on the back end.  I think it's straightforward for HTML, not
sure about info, and I couldn't see any obvious way for nroff -man with
brief research (but that may simply be because the people who know about
that are too old to use the interweb thing; there's a .ms document
somewhere on an FTP server about it).

The best bet might be to use an example() paragraph so it's
guaranteed to start at the left.

pws