bug#44674: 28.0.50; Adding current-cpu-time for performance tests

bug#44674: 28.0.50; Adding current-cpu-time for performance tests

[Stefan Monnier]

Package: Emacs
Version: 28.0.50


I tried to write a test for the performance problem seen in bug#41029,
but found it very difficult to make it work half-reliably because we
only have access to wall-clock time from Elisp.

So I suggest we add a new primitive `current-cpu-time` with which those
tests seem to be at least somewhat doable.

See my current patch below which includes a test for that
performance bug.  It clearly requires adding w32 support (or
fetching more clock functionality from gnulib) but I don't know how to
do that.


        Stefan


diff --git a/etc/NEWS b/etc/NEWS
index f21c4cb02c..703fbf5243 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1733,6 +1733,11 @@ ledit.el, lmenu.el, lucid.el and old-whitespace.el.
 \f
 * Lisp Changes in Emacs 28.1
 
+---
+** New function 'current-cpu-time'.
+It gives access to the CPU time used by the Emacs process, for
+example for benchmarking purposes.
+
 +++
 ** 'define-globalized-minor-mode' now takes a ':predicate' parameter.
 This can be used to control which major modes the minor mode should be
diff --git a/src/timefns.c b/src/timefns.c
index 4a28f707a3..b569c6d075 100644
--- a/src/timefns.c
+++ b/src/timefns.c
@@ -1793,6 +1793,18 @@ DEFUN ("current-time", Fcurrent_time, Scurrent_time, 0, 0, 0,
   return make_lisp_time (current_timespec ());
 }
 
+#ifdef CLOCKS_PER_SEC
+DEFUN ("current-cpu-time", Fcurrent_cpu_time, Scurrent_cpu_time, 0, 0, 0,
+       doc: /* Return the current CPU time along with its resolution.
+The return value is a pair (CPU-TICKS . TICKS-PER-SEC).
+The CPU-TICKS counter can wrap around, so values cannot be meaningfully
+compared if too much time has passed between them.  */)
+  (void)
+{
+  return Fcons (make_int (clock ()), make_int (CLOCKS_PER_SEC));
+}
+#endif
+
 DEFUN ("current-time-string", Fcurrent_time_string, Scurrent_time_string,
        0, 2, 0,
        doc: /* Return the current local time, as a human-readable string.
@@ -2032,6 +2044,9 @@ syms_of_timefns (void)
   DEFSYM (Qencode_time, "encode-time");
 
   defsubr (&Scurrent_time);
+#ifdef CLOCKS_PER_SEC
+  defsubr (&Scurrent_cpu_time);
+#endif
   defsubr (&Stime_convert);
   defsubr (&Stime_add);
   defsubr (&Stime_subtract);
diff --git a/test/src/data-tests.el b/test/src/data-tests.el
index ed09203907..fc57271252 100644
--- a/test/src/data-tests.el
+++ b/test/src/data-tests.el
@@ -381,6 +381,36 @@ binding-test-set-constant-nil
   "Test setting a keyword to itself"
   (with-no-warnings (should (setq :keyword :keyword))))
 
+(ert-deftest data-tests--set-default-per-buffer ()
+  :expected-result t ;; Not fixed yet!
+  ;; FIXME: Performance tests are inherently unreliable.
+  ;; Using wall-clock time makes it even worse, so don't bother unless
+  ;; we have the primitive to measure cpu-time.
+  (skip-unless (fboundp 'current-cpu-time))
+  ;; Test performance of set-default on DEFVAR_PER_BUFFER variables.
+  ;; More specifically, test the problem seen in bug#41029 where setting
+  ;; the default value of a variable takes time proportional to the
+  ;; number of buffers.
+  (let* ((fun #'error)
+         (test (lambda ()
+                 (with-temp-buffer
+                   (let ((st (car (current-cpu-time))))
+                     (dotimes (_ 1000)
+                       (let ((case-fold-search 'data-test))
+                         ;; Use an indirection through a mutable var
+                         ;; to try and make sure the byte-compiler
+                         ;; doesn't optimize away the let bindings.
+                         (funcall fun)))
+                     (- (car (current-cpu-time)) st)))))
+         (_ (setq fun #'ignore))
+         (time1 (funcall test))
+         (bufs (mapcar (lambda (_) (generate-new-buffer " data-test"))
+                       (make-list 1000 nil)))
+         (time2 (funcall test)))
+    (mapc #'kill-buffer bufs)
+    ;; Don't divide one time by the other since they may be 0.
+    (should (< time2 (* time1 5)))))
+
 ;; More tests to write -
 ;; kill-local-variable
 ;; defconst; can modify

bug#44674: 28.0.50; Adding current-cpu-time for performance tests

[Philipp Stephani]

Am Mo., 16. Nov. 2020 um 02:08 Uhr schrieb Stefan Monnier
<monnier@iro.umontreal.ca>:

> +#ifdef CLOCKS_PER_SEC
> +DEFUN ("current-cpu-time", Fcurrent_cpu_time, Scurrent_cpu_time, 0, 0, 0,
> +       doc: /* Return the current CPU time along with its resolution.
> +The return value is a pair (CPU-TICKS . TICKS-PER-SEC).
> +The CPU-TICKS counter can wrap around, so values cannot be meaningfully
> +compared if too much time has passed between them.  */)
> +  (void)
> +{
> +  return Fcons (make_int (clock ()), make_int (CLOCKS_PER_SEC));
> +}
> +#endif

Looks useful!

It might be beneficial to use the higher-resolution clock_gettime with
CLOCK_PROCESS_CPUTIME_ID if available and only fall back to clock()
otherwise.

While there, consider also implementing a function to read the
CLOCK_MONOTONIC clock – that should be preferred over the realtime
clock for stopwatch-style measurements.

bug#44674: 28.0.50; Adding current-cpu-time for performance tests

[Eli Zaretskii]

On November 16, 2020 3:07:35 AM GMT+02:00, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
> Package: Emacs
> Version: 28.0.50
> 
> 
> I tried to write a test for the performance problem seen in bug#41029,
> but found it very difficult to make it work half-reliably because we
> only have access to wall-clock time from Elisp.
> 
> So I suggest we add a new primitive `current-cpu-time` with which
> those
> tests seem to be at least somewhat doable.
> 
> See my current patch below which includes a test for that
> performance bug.  It clearly requires adding w32 support (or
> fetching more clock functionality from gnulib) but I don't know how to
> do that.

AFAIU, using 'clock' here is not the best idea, as there are caveats wrt to calling 'system', and the origin of the returned value is not well defined to be portable.

I suggest to use 'times' instead.  For w32, we could easily implement it, as we already have that functionality for 'getloadavg'.

bug#44674: 28.0.50; Adding current-cpu-time for performance tests

[Eli Zaretskii]

On November 16, 2020 9:58:40 AM GMT+02:00, Eli Zaretskii <eliz@gnu.org> wrote:
> On November 16, 2020 3:07:35 AM GMT+02:00, Stefan Monnier
> <monnier@iro.umontreal.ca> wrote:
> > Package: Emacs
> > Version: 28.0.50
> > 
> > 
> > I tried to write a test for the performance problem seen in
> bug#41029,
> > but found it very difficult to make it work half-reliably because we
> > only have access to wall-clock time from Elisp.
> > 
> > So I suggest we add a new primitive `current-cpu-time` with which
> > those
> > tests seem to be at least somewhat doable.
> > 
> > See my current patch below which includes a test for that
> > performance bug.  It clearly requires adding w32 support (or
> > fetching more clock functionality from gnulib) but I don't know how
> to
> > do that.
> 
> AFAIU, using 'clock' here is not the best idea, as there are caveats
> wrt to calling 'system', and the origin of the returned value is not
> well defined to be portable.
> 
> I suggest to use 'times' instead.  For w32, we could easily implement
> it, as we already have that functionality for 'getloadavg'.

Btw, once this goes in, how about making benchmark-run use it?

bug#44674: 28.0.50; Adding current-cpu-time for performance tests

[Mattias Engdegård]

> +The return value is a pair (CPU-TICKS . TICKS-PER-SEC).

Perhaps not ideal to cons in a timing primitive where low overhead is called for.
What about just returning an integer and have a different way to get at TICKS-PER-SEC?
After all, the timer frequency is not specific to each measurement.

Ideally the returned value should be a fixnum to minimise overhead, but it may restrict the range on 32-bit platforms.

I also agree with Phillipp's comment about clock_gettime.

bug#44674: 28.0.50; Adding current-cpu-time for performance tests

[Eli Zaretskii]

On November 16, 2020 12:11:34 PM GMT+02:00, "Mattias Engdegård" <mattiase@acm.org> wrote:
> > +The return value is a pair (CPU-TICKS . TICKS-PER-SEC).
> 
> Perhaps not ideal to cons in a timing primitive where low overhead is
> called for.
> What about just returning an integer and have a different way to get
> at TICKS-PER-SEC?
> After all, the timer frequency is not specific to each measurement.
> 
> Ideally the returned value should be a fixnum to minimise overhead,
> but it may restrict the range on 32-bit platforms.
> 
> I also agree with Phillipp's comment about clock_gettime.

AFAUI, CLOCK_PROCESS_CPUTIME_ID accounts for all the threads, so might not be what we want even if it is supported, since other threads might be involved which we aren't interested in (e.g., GTK threads).

bug#44674: 28.0.50; Adding current-cpu-time for performance tests

[Philipp Stephani]

Am Mo., 16. Nov. 2020 um 11:40 Uhr schrieb Eli Zaretskii <eliz@gnu.org>:
>
> On November 16, 2020 12:11:34 PM GMT+02:00, "Mattias Engdegård" <mattiase@acm.org> wrote:
> > > +The return value is a pair (CPU-TICKS . TICKS-PER-SEC).
> >
> > Perhaps not ideal to cons in a timing primitive where low overhead is
> > called for.
> > What about just returning an integer and have a different way to get
> > at TICKS-PER-SEC?
> > After all, the timer frequency is not specific to each measurement.
> >
> > Ideally the returned value should be a fixnum to minimise overhead,
> > but it may restrict the range on 32-bit platforms.
> >
> > I also agree with Phillipp's comment about clock_gettime.
>
> AFAUI, CLOCK_PROCESS_CPUTIME_ID accounts for all the threads, so might not be what we want even if it is supported, since other threads might be involved which we aren't interested in (e.g., GTK threads).

clock (on GNU/Linux) is a thin wrapper around
CLOCK_PROCESS_CPUTIME_ID, see
https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/clock.c;hb=aac0f62c47beee5b546bacc330acc2dd21cda0dc.

bug#44674: 28.0.50; Adding current-cpu-time for performance tests

[Mattias Engdegård]

16 nov. 2020 kl. 11.40 skrev Eli Zaretskii <eliz@gnu.org>:

> AFAUI, CLOCK_PROCESS_CPUTIME_ID accounts for all the threads, so might not be what we want even if it is supported, since other threads might be involved which we aren't interested in (e.g., GTK threads).

CLOCK_THREAD_CPUTIME_ID then. Actually, a case can be made for either timer.

bug#44674: 28.0.50; Adding current-cpu-time for performance tests

[Philipp Stephani]

Am Mo., 16. Nov. 2020 um 09:19 Uhr schrieb Eli Zaretskii <eliz@gnu.org>:
>
> On November 16, 2020 9:58:40 AM GMT+02:00, Eli Zaretskii <eliz@gnu.org> wrote:
> > On November 16, 2020 3:07:35 AM GMT+02:00, Stefan Monnier
> > <monnier@iro.umontreal.ca> wrote:
> > > Package: Emacs
> > > Version: 28.0.50
> > >
> > >
> > > I tried to write a test for the performance problem seen in
> > bug#41029,
> > > but found it very difficult to make it work half-reliably because we
> > > only have access to wall-clock time from Elisp.
> > >
> > > So I suggest we add a new primitive `current-cpu-time` with which
> > > those
> > > tests seem to be at least somewhat doable.
> > >
> > > See my current patch below which includes a test for that
> > > performance bug.  It clearly requires adding w32 support (or
> > > fetching more clock functionality from gnulib) but I don't know how
> > to
> > > do that.
> >
> > AFAIU, using 'clock' here is not the best idea, as there are caveats
> > wrt to calling 'system', and the origin of the returned value is not
> > well defined to be portable.
> >
> > I suggest to use 'times' instead.  For w32, we could easily implement
> > it, as we already have that functionality for 'getloadavg'.
>
> Btw, once this goes in, how about making benchmark-run use it?
>

benchmark-run measures walltime, not CPU time, it should use CLOCK_MONOTONIC.
We might consider adding another macro that returns the CPU time (or both).

bug#44674: 28.0.50; Adding current-cpu-time for performance tests

[Stefan Monnier]

Philipp wrote:
> It might be beneficial to use the higher-resolution clock_gettime with
> CLOCK_PROCESS_CPUTIME_ID if available and only fall back to clock()
> otherwise.
>
> While there, consider also implementing a function to read the
> CLOCK_MONOTONIC clock – that should be preferred over the realtime
> clock for stopwatch-style measurements.

Given that we've lived without any of it for so many years, I'm not sure
it's worth the trouble.  Also since I'm not familiar with any of those
interfaces, I'd welcome it if someone else could do that if
they're interested.

Eli said:
> AFAIU, using 'clock' here is not the best idea, as there are caveats wrt to
> calling 'system', and the origin of the returned value is not well defined
> to be portable.

What do you mean by "origin" and by "calling 'system'"?

Mattias added:
>> +The return value is a pair (CPU-TICKS . TICKS-PER-SEC).
> Perhaps not ideal to cons in a timing primitive where low overhead is
> called for.

I expect that the time it takes to perform the `funcall` to get to this
function is itself larger than the time it takes to run `Fcons`, so I'm
not too worried about this overhead.

> What about just returning an integer and have a different way to get
> at TICKS-PER-SEC?

I designed it this way to keep the change as simple as possible, but
yes, we could have two separate functions.

> Ideally the returned value should be a fixnum to minimise overhead, but it
> may restrict the range on 32-bit platforms.

`make_int` will return a fixnum when possible.  Are you suggesting we
modulo-truncate the integer so it fits i a fixnum?  I think that on
32bit platforms this will lose too much range (I guess we could divide
instead of modulo-truncate, and lose precision instead, tho).  Not sure
it's worth the trouble, tho.

In my experience measuring CPU or wall clock time for things like
benchmarks is only meaningful if it's a non-trivial amount of time (like
0.1s or more), so the added cost of Fcons or allocating a bignum
is negligible.


        Stefan

bug#44674: 28.0.50; Adding current-cpu-time for performance tests

[Mattias Engdegård]

16 nov. 2020 kl. 16.27 skrev Stefan Monnier <monnier@iro.umontreal.ca>:

> I expect that the time it takes to perform the `funcall` to get to this
> function is itself larger than the time it takes to run `Fcons`, so I'm
> not too worried about this overhead.

Perhaps, but the lower the overhead, the shorter the intervals that can effectively be measured. It would be a shame to build pointless latency into the calling interface from the get-go.

> `make_int` will return a fixnum when possible.  Are you suggesting we
> modulo-truncate the integer so it fits i a fixnum?

As you say it's not worth doing so for 32 bits, but in case a full value would require 64 bits, we should at least consider truncating it to 63.

The values returned from clock, clock_gettime etc are generally wrapping, which means that the user would need to know how to mask the difference in case the timer wraps during a measured interval. This can neatly be side-stepped by providing a function for computing the duration from two sample points, eg,

 (cpu-time-duration CPU-TIME-1 CPU-TIME-2)

which would apply the necessary modulo-reduction and return the duration in seconds as a float. That way, we save the user the trouble to do

 (/ (logand (- CPU-TIME-2 CPU-TIME-1)
            (1- (ash 1 CPU-TIMER-BITS))
    (float CPU-TIMER-FREQ))

and having to retrieve two system constants.

> In my experience measuring CPU or wall clock time for things like
> benchmarks is only meaningful if it's a non-trivial amount of time (like
> 0.1s or more), so the added cost of Fcons or allocating a bignum
> is negligible.

Being able to measure short intervals without too much noise can be quite useful. Of course, CPU time is sometimes measured with much less resolution than real time which sets an effective lower limit; 100 Hz used to be the norm but it should be better these days.

bug#44674: 28.0.50; Adding current-cpu-time for performance tests

[Eli Zaretskii]

> From: Mattias Engdegård <mattiase@acm.org>
> Date: Mon, 16 Nov 2020 11:11:34 +0100
> Cc: Eli Zaretskii <eliz@gnu.org>, 44674@debbugs.gnu.org,
>         Philipp Stephani <p.stephani2@gmail.com>
> 
> > +The return value is a pair (CPU-TICKS . TICKS-PER-SEC).
> 
> Perhaps not ideal to cons in a timing primitive where low overhead is called for.
> What about just returning an integer and have a different way to get at TICKS-PER-SEC?

Why not just return a float in seconds instead?  Do we envision any
application that could be interested in (non-portable) ticks?

> Ideally the returned value should be a fixnum to minimise overhead, but it may restrict the range on 32-bit platforms.

If it must be a fixnum, then it will need to be in nanoseconds, which
would limit its applicability for measuring long intervals.

bug#44674: 28.0.50; Adding current-cpu-time for performance tests

[Eli Zaretskii]

> From: Mattias Engdegård <mattiase@acm.org>
> Date: Mon, 16 Nov 2020 11:53:37 +0100
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, 44674@debbugs.gnu.org,
>         Philipp Stephani <p.stephani2@gmail.com>
> 
> 16 nov. 2020 kl. 11.40 skrev Eli Zaretskii <eliz@gnu.org>:
> 
> > AFAUI, CLOCK_PROCESS_CPUTIME_ID accounts for all the threads, so might not be what we want even if it is supported, since other threads might be involved which we aren't interested in (e.g., GTK threads).
> 
> CLOCK_THREAD_CPUTIME_ID then. Actually, a case can be made for either timer.

Can you make a case for process-global timing, given that we don't
control all of the threads, and that some of them are utterly
unrelated to timing Lisp programs?

bug#44674: 28.0.50; Adding current-cpu-time for performance tests

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Mon, 16 Nov 2020 12:46:07 +0100
> Cc: 44674@debbugs.gnu.org, Stefan Monnier <monnier@iro.umontreal.ca>
> 
> > Btw, once this goes in, how about making benchmark-run use it?
> 
> benchmark-run measures walltime, not CPU time

It does now, but we may want to change that.  Does it really make
sense to "benchmark" something using elapsed time?

bug#44674: 28.0.50; Adding current-cpu-time for performance tests

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Eli Zaretskii <eliz@gnu.org>,  44674@debbugs.gnu.org,  Philipp Stephani
>  <p.stephani2@gmail.com>
> Date: Mon, 16 Nov 2020 10:27:29 -0500
> 
> > AFAIU, using 'clock' here is not the best idea, as there are caveats wrt to
> > calling 'system', and the origin of the returned value is not well defined
> > to be portable.
> 
> What do you mean by "origin" and by "calling 'system'"?

Quoting https://pubs.opengroup.org/onlinepubs/9699919799/:

          The clock() function shall return the implementation's best
          approximation to the processor time used by the process
          since the beginning of an implementation-defined era
	  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
	  related only to the process invocation.

How do you write portable Lisp code that returns consistent results
based on such shaky foundations?

bug#44674: 28.0.50; Adding current-cpu-time for performance tests

[Eli Zaretskii]

> Date: Mon, 16 Nov 2020 19:28:24 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: mattiase@acm.org, p.stephani2@gmail.com, 44674@debbugs.gnu.org
> 
> > What do you mean by "origin" and by "calling 'system'"?
> 
> Quoting https://pubs.opengroup.org/onlinepubs/9699919799/:
> 
>           The clock() function shall return the implementation's best
>           approximation to the processor time used by the process
>           since the beginning of an implementation-defined era
> 	  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 	  related only to the process invocation.
> 
> How do you write portable Lisp code that returns consistent results
> based on such shaky foundations?

Sorry, forgot to answer the 'system' part.  The issue there is whether
calling 'system' (and in general waiting for sub-processes to exit) is
counted against the process's use of CPU or not.  AFAIK, this is not
well defined, either.

bug#44674: 28.0.50; Adding current-cpu-time for performance tests

[Andreas Schwab]

On Nov 16 2020, Eli Zaretskii wrote:

> Sorry, forgot to answer the 'system' part.  The issue there is whether
> calling 'system' (and in general waiting for sub-processes to exit) is
> counted against the process's use of CPU or not.  AFAIK, this is not
> well defined, either.

There's times and getrusage.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

bug#44674: 28.0.50; Adding current-cpu-time for performance tests

[Eli Zaretskii]

> From: Andreas Schwab <schwab@linux-m68k.org>
> Cc: monnier@iro.umontreal.ca,  mattiase@acm.org,  p.stephani2@gmail.com,
>   44674@debbugs.gnu.org
> Date: Mon, 16 Nov 2020 18:59:28 +0100
> 
> On Nov 16 2020, Eli Zaretskii wrote:
> 
> > Sorry, forgot to answer the 'system' part.  The issue there is whether
> > calling 'system' (and in general waiting for sub-processes to exit) is
> > counted against the process's use of CPU or not.  AFAIK, this is not
> > well defined, either.
> 
> There's times and getrusage.

Right.  I was talking about 'clock', and I proposed to use 'times',
instead for that very reason.  It could also be a more useful feature
to provide user and system time separately, btw.

bug#44674: 28.0.50; Adding current-cpu-time for performance tests

[Philipp Stephani]

Am Mo., 16. Nov. 2020 um 18:17 Uhr schrieb Eli Zaretskii <eliz@gnu.org>:
>
> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Date: Mon, 16 Nov 2020 12:46:07 +0100
> > Cc: 44674@debbugs.gnu.org, Stefan Monnier <monnier@iro.umontreal.ca>
> >
> > > Btw, once this goes in, how about making benchmark-run use it?
> >
> > benchmark-run measures walltime, not CPU time
>
> It does now, but we may want to change that.

That would be a breaking change.

> Does it really make
> sense to "benchmark" something using elapsed time?

Yes, the benchmarks I know all measure wall time. After all, that's
what the user cares about.

bug#44674: 28.0.50; Adding current-cpu-time for performance tests

[Philipp Stephani]

Am Mo., 16. Nov. 2020 um 18:28 Uhr schrieb Eli Zaretskii <eliz@gnu.org>:
>
> > From: Stefan Monnier <monnier@iro.umontreal.ca>
> > Cc: Eli Zaretskii <eliz@gnu.org>,  44674@debbugs.gnu.org,  Philipp Stephani
> >  <p.stephani2@gmail.com>
> > Date: Mon, 16 Nov 2020 10:27:29 -0500
> >
> > > AFAIU, using 'clock' here is not the best idea, as there are caveats wrt to
> > > calling 'system', and the origin of the returned value is not well defined
> > > to be portable.
> >
> > What do you mean by "origin" and by "calling 'system'"?
>
> Quoting https://pubs.opengroup.org/onlinepubs/9699919799/:
>
>           The clock() function shall return the implementation's best
>           approximation to the processor time used by the process
>           since the beginning of an implementation-defined era
>           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>           related only to the process invocation.
>
> How do you write portable Lisp code that returns consistent results
> based on such shaky foundations?

For non-walltime clocks, only the difference between two measurements
is meaningful.

bug#44674: 28.0.50; Adding current-cpu-time for performance tests

[Philipp Stephani]

Am Mo., 16. Nov. 2020 um 16:27 Uhr schrieb Stefan Monnier
<monnier@iro.umontreal.ca>:
>
> Philipp wrote:
> > It might be beneficial to use the higher-resolution clock_gettime with
> > CLOCK_PROCESS_CPUTIME_ID if available and only fall back to clock()
> > otherwise.
> >
> > While there, consider also implementing a function to read the
> > CLOCK_MONOTONIC clock – that should be preferred over the realtime
> > clock for stopwatch-style measurements.
>
> Given that we've lived without any of it for so many years, I'm not sure
> it's worth the trouble.

It's definitely not a pressing concern, but the current approach does
give wrong results if a leap second happens or on other occasions. See
e.g. https://go.googlesource.com/proposal/+/master/design/12914-monotonic.md
for why Go introduced monotonic clock measurements. Other languages
had the distinction between wall clocks and monotonic clocks for a
long time as well (e.g. Java System.nanoTime).

>  Also since I'm not familiar with any of those
> interfaces, I'd welcome it if someone else could do that if
> they're interested.

I wouldn't mind doing it, if there's general agreement that it's worth having.
But it would really just be:
1. clock_gettime(CLOCK_MONOTONIC_ID)
2. check for errors
3. make_lisp_time

bug#44674: 28.0.50; Adding current-cpu-time for performance tests

[Stefan Monnier]

>> What do you mean by "origin" and by "calling 'system'"?
> Quoting https://pubs.opengroup.org/onlinepubs/9699919799/:
>
>           The clock() function shall return the implementation's best
>           approximation to the processor time used by the process
>           since the beginning of an implementation-defined era
> 	  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 	  related only to the process invocation.
>
> How do you write portable Lisp code that returns consistent results
> based on such shaky foundations?

You only consider differences between measurements.


        Stefan

bug#44674: 28.0.50; Adding current-cpu-time for performance tests

[Stefan Monnier]

> Sorry, forgot to answer the 'system' part.  The issue there is whether
> calling 'system' (and in general waiting for sub-processes to exit) is
> counted against the process's use of CPU or not.  AFAIK, this is not
> well defined, either.

Ah, I must admit that for the cases that interest me the distinction is
of no consequence.  I don't even know which behavior I'd prefer.


        Stefan

bug#44674: 28.0.50; Adding current-cpu-time for performance tests

[Stefan Monnier]

>> Also since I'm not familiar with any of those interfaces, I'd welcome
>> it if someone else could do that if they're interested.
> I wouldn't mind doing it, if there's general agreement that it's worth having.
> But it would really just be:
> 1. clock_gettime(CLOCK_MONOTONIC_ID)
> 2. check for errors
> 3. make_lisp_time

CLOCK_MONOTONIC wouldn't satisfy my needs, because it makes the duration
of execution of a piece of code too depend on OS's scheduling choices.\
In my use case I need to run two chunks of code and make sure their
run-time is not "wildly" different, but with wall-clock time (and
CLOCK_MONOTONIC is close enough to wallclock to suffer from the same
problem) any context switch in the middle of one of the two runs can
cause such "wildly" different results, including make the slow code
appear to be much faster than the fast one.


        Stefan

bug#44674: 28.0.50; Adding current-cpu-time for performance tests

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Mon, 16 Nov 2020 19:31:47 +0100
> Cc: 44674@debbugs.gnu.org, Stefan Monnier <monnier@iro.umontreal.ca>
> 
> Am Mo., 16. Nov. 2020 um 18:17 Uhr schrieb Eli Zaretskii <eliz@gnu.org>:
> >
> > > From: Philipp Stephani <p.stephani2@gmail.com>
> > > Date: Mon, 16 Nov 2020 12:46:07 +0100
> > > Cc: 44674@debbugs.gnu.org, Stefan Monnier <monnier@iro.umontreal.ca>
> > >
> > > > Btw, once this goes in, how about making benchmark-run use it?
> > >
> > > benchmark-run measures walltime, not CPU time
> >
> > It does now, but we may want to change that.
> 
> That would be a breaking change.

I could argue that it's a bugfix.

> > Does it really make
> > sense to "benchmark" something using elapsed time?
> 
> Yes, the benchmarks I know all measure wall time. After all, that's
> what the user cares about.

??? On a system that is heavily loaded, and Emacs doesn't get an
execution unit close to 100% of the time, the wallclock time can be so
skewed as to defeat any meaningful measurements.  E.g., imagine that
you are running a benchmark while (another version of) Emacs is being
built with "make -jN", with N large enough to make all the cores busy.

bug#44674: 28.0.50; Adding current-cpu-time for performance tests

[Mattias Engdegård]

16 nov. 2020 kl. 18.13 skrev Eli Zaretskii <eliz@gnu.org>:

>> Perhaps not ideal to cons in a timing primitive where low overhead is called for.
>> What about just returning an integer and have a different way to get at TICKS-PER-SEC?
> 
> Why not just return a float in seconds instead?

Precision would vary depending on the magnitude of the time stamp (which should be irrelevant since only differences are used). Floats are also always boxed, for that matter.

> Can you make a case for process-global timing, given that we don't
> control all of the threads, and that some of them are utterly
> unrelated to timing Lisp programs?

If Lisp actions trigger work done in other threads the completion of which has to be awaited then those threads are effectively part of the same computation. (Admitted, not a very strong case.)

bug#44674: 28.0.50; Adding current-cpu-time for performance tests

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Mon, 16 Nov 2020 19:32:35 +0100
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, Mattias Engdegård <mattiase@acm.org>, 
> 	44674@debbugs.gnu.org
> 
> >           The clock() function shall return the implementation's best
> >           approximation to the processor time used by the process
> >           since the beginning of an implementation-defined era
> >           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >           related only to the process invocation.
> >
> > How do you write portable Lisp code that returns consistent results
> > based on such shaky foundations?
> 
> For non-walltime clocks, only the difference between two measurements
> is meaningful.

Which is a limitation, when compared to APIs that always measure since
the program/thread start.  For example, with 'clock' you cannot
measure how long the startup takes.

bug#44674: 28.0.50; Adding current-cpu-time for performance tests

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: mattiase@acm.org,  p.stephani2@gmail.com,  44674@debbugs.gnu.org
> Date: Mon, 16 Nov 2020 13:41:59 -0500
> 
> > Sorry, forgot to answer the 'system' part.  The issue there is whether
> > calling 'system' (and in general waiting for sub-processes to exit) is
> > counted against the process's use of CPU or not.  AFAIK, this is not
> > well defined, either.
> 
> Ah, I must admit that for the cases that interest me the distinction is
> of no consequence.  I don't even know which behavior I'd prefer.

Well, we are talking about a general-purpose API, so the semantics
should be well defined.  And for 'clock', it isn't.

bug#44674: 28.0.50; Adding current-cpu-time for performance tests

[Stefan Monnier]

>> > > AFAIU, using 'clock' here is not the best idea, as there are caveats wrt to
>> > > calling 'system', and the origin of the returned value is not well defined
>> > > to be portable.
>> >
>> > What do you mean by "origin" and by "calling 'system'"?
>>
>> Quoting https://pubs.opengroup.org/onlinepubs/9699919799/:
>>
>>           The clock() function shall return the implementation's best
>>           approximation to the processor time used by the process
>>           since the beginning of an implementation-defined era
>>           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>           related only to the process invocation.
>>
>> How do you write portable Lisp code that returns consistent results
>> based on such shaky foundations?
>
> For non-walltime clocks, only the difference between two measurements
> is meaningful.

Mabe the API we should expose should give access to the actual time but
only to time-differences, as in:

    (funcall-with-cpu-time FUNCTION)

    Call FUNCTION and return the CPU time used.
    The return value is of the form (TIME . VALUE) where TIME is the CPU
    time used during execution of FUNCTION, measured in seconds, and
    VALUE is the return value of FUNCTION.


-- Stefan

bug#44674: 28.0.50; Adding current-cpu-time for performance tests

[Lars Ingebrigtsen]

Philipp Stephani <p.stephani2@gmail.com> writes:

>> It does now, but we may want to change that.
>
> That would be a breaking change.

Yeah, we can't do that.  But a new benchmark-run command would be fine.

>> Does it really make
>> sense to "benchmark" something using elapsed time?
>
> Yes, the benchmarks I know all measure wall time. After all, that's
> what the user cares about.

The most used benchmark every (I'm guessing), which is the "time" shell
command, reports both.  Most people only care about the "real" (i.e.,
wall time), though, as you point out.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#44674: 28.0.50; Adding current-cpu-time for performance tests

[Lars Ingebrigtsen]

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

> So I suggest we add a new primitive `current-cpu-time` with which those
> tests seem to be at least somewhat doable.

This was over a year ago -- there was some discussion about tweaking
certain bits of this, but in general everybody seemed to be in favour.
Was there any specific reason this wasn't pushed, Stefan?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#44674: 28.0.50; Adding current-cpu-time for performance tests

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Lars Ingebrigtsen [2022-04-26 15:55:45] wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> So I suggest we add a new primitive `current-cpu-time` with which those
>> tests seem to be at least somewhat doable.
> This was over a year ago -- there was some discussion about tweaking
> certain bits of this, but in general everybody seemed to be in favour.
> Was there any specific reason this wasn't pushed, Stefan?

The "tweaking certain bits of this" never happened.


        Stefan

bug#44674: 28.0.50; Adding current-cpu-time for performance tests

[Lars Ingebrigtsen]

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

>> This was over a year ago -- there was some discussion about tweaking
>> certain bits of this, but in general everybody seemed to be in favour.
>> Was there any specific reason this wasn't pushed, Stefan?
>
> The "tweaking certain bits of this" never happened.

I think adding it to Emacs as is makes sense (because it's useful in the
current form, I think?), and then it can be tweaked later.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#44674: 28.0.50; Adding current-cpu-time for performance tests

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Lars Ingebrigtsen [2022-04-27 15:09:09] wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>>> This was over a year ago -- there was some discussion about tweaking
>>> certain bits of this, but in general everybody seemed to be in favour.
>>> Was there any specific reason this wasn't pushed, Stefan?
>> The "tweaking certain bits of this" never happened.
> I think adding it to Emacs as is makes sense (because it's useful in the
> current form, I think?), and then it can be tweaked later.

You have my blessing,


        Stefan

bug#44674: 28.0.50; Adding current-cpu-time for performance tests

[Lars Ingebrigtsen]

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

> You have my blessing,

Now pushed.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no