Re: How to canonicalize a time object

[Paul Eggert <eggert@cs.ucla.edu>]

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

On 8/14/22 10:15, Stefan Monnier wrote:
> E.g. if we do (time-convert FLOAT t)
> the returned value will be of the form (TICKS . 2^51) and for
> (time-convert (HI LOW US) t) it'll be (TICKS . 10^6), but
> `lisp_to_timespec` doesn't seem to have a fast path for 2^51 and I'm not
> sure it has one for 10^6 (nor 10^12 used for (HI LOW US PS)).

I hadn't written fast paths for those, no.

The idea is that (time-convert X t) should be best if you want full 
precision. If you want only one-second precision, (time-convert X 
'integer) should yield timestamps that are a bit faster, because there's 
no need for consing.

I just now looked at the code and found a couple of places where integer 
timestamps were significantly slower, and wrote and installed the 
attached performance tweaks to fix that.


> - There doesn't seem to be an easy way to get `timespec_hz` from ELisp.

Why would code need timespec_hz, if (time-convert X t) suffices for 
conversion to full precision?

Code that needs timespec_hz can get it with (cdr (time-convert nil t)). 
If that's too cumbersome we could add a builtin integer for it, though 
there's a caveat here: some timestamps can use a different hz value than 
others, so the builtin integer would be only for the timestamps 
retrieved from (time-convert ... t) and not necessarily for other (TICKS 
. HZ) timestamps retrieved from the system via file-attributes etc. To 
avoid this confusion it might be better not to have that builtin integer.


> - It's cumbersome to find out if a time value is a whole number
>    of seconds.

If (time-equal-p X (time-convert X 'integer)) is too cumbersome, we 
could add a function to do that.


> Of course, maybe we shouldn't care, and just presume that the slow path
> which does a GMP mul+div is plenty fast anyway.

Yes, the goal is that this stuff "just works" without people having to 
worry overmuch about speed.

[-- Attachment #2: 0001-Decode-time-conses-before-floats.patch --]
[-- Type: text/x-patch, Size: 1563 bytes --]

From 15aba5e64496414ec4659118f910516d2dc5e8b4 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 14 Aug 2022 13:48:11 -0700
Subject: [PATCH 1/2] Decode time conses before floats

* src/timefns.c (decode_lisp_time): Test for conses before floats,
as conses are more common.
---
 src/timefns.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/src/timefns.c b/src/timefns.c
index edfd73e9b8..b9d9a4ed97 100644
--- a/src/timefns.c
+++ b/src/timefns.c
@@ -822,17 +822,6 @@ decode_lisp_time (Lisp_Object specified_time, bool decode_secs_only,
 
   if (NILP (specified_time))
     form = TIMEFORM_NIL;
-  else if (FLOATP (specified_time))
-    {
-      double d = XFLOAT_DATA (specified_time);
-      if (!isfinite (d))
-	time_error (isnan (d) ? EDOM : EOVERFLOW);
-      if (result)
-	decode_float_time (d, result);
-      else
-	*dresult = d;
-      return TIMEFORM_FLOAT;
-    }
   else if (CONSP (specified_time))
     {
       high = XCAR (specified_time);
@@ -872,6 +861,17 @@ decode_lisp_time (Lisp_Object specified_time, bool decode_secs_only,
       if (! INTEGERP (low))
 	form = TIMEFORM_INVALID;
     }
+  else if (FLOATP (specified_time))
+    {
+      double d = XFLOAT_DATA (specified_time);
+      if (!isfinite (d))
+	time_error (isnan (d) ? EDOM : EOVERFLOW);
+      if (result)
+	decode_float_time (d, result);
+      else
+	*dresult = d;
+      return TIMEFORM_FLOAT;
+    }
 
   int err = decode_time_components (form, high, low, usec, psec,
 				    result, dresult);
-- 
2.34.1


[-- Attachment #3: 0002-Improve-timefns-speed-on-integers.patch --]
[-- Type: text/x-patch, Size: 1890 bytes --]

From a64fe6f31987ca4a2c8ac6b96b97c6a440aeb2a2 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 14 Aug 2022 13:48:11 -0700
Subject: [PATCH 2/2] Improve timefns speed on integers

* src/timefns.c (decode_lisp_time) [FASTER_TIMEFNS]:
Speed up when SPECIFIED_TIME is an integer.
(time_cmp) [FASTER_TIMEFNS]: Speed up when comparing integers.
---
 src/timefns.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/src/timefns.c b/src/timefns.c
index b9d9a4ed97..eed2edf1cc 100644
--- a/src/timefns.c
+++ b/src/timefns.c
@@ -861,6 +861,11 @@ decode_lisp_time (Lisp_Object specified_time, bool decode_secs_only,
       if (! INTEGERP (low))
 	form = TIMEFORM_INVALID;
     }
+  else if (FASTER_TIMEFNS && INTEGERP (specified_time))
+    {
+      decode_ticks_hz (specified_time, make_fixnum (1), result, dresult);
+      return form;
+    }
   else if (FLOATP (specified_time))
     {
       double d = XFLOAT_DATA (specified_time);
@@ -1206,10 +1211,16 @@ time_cmp (Lisp_Object a, Lisp_Object b)
     return 0;
 
   /* Compare (X . Z) to (Y . Z) quickly if X and Y are fixnums.
-     Do not inspect Z, as it is OK to not signal if A and B are invalid.  */
-  if (FASTER_TIMEFNS && CONSP (a) && CONSP (b) && BASE_EQ (XCDR (a), XCDR (b))
-      && FIXNUMP (XCAR (a)) && FIXNUMP (XCAR (b)))
-    return XFIXNUM (XCAR (a)) - XFIXNUM (XCAR (b));
+     Do not inspect Z, as it is OK to not signal if A and B are invalid.
+     Also, compare X to Y quickly if X and Y are fixnums.  */
+  if (FASTER_TIMEFNS)
+    {
+      Lisp_Object x = a, y = b;
+      if (CONSP (a) && CONSP (b) && BASE_EQ (XCDR (a), XCDR (b)))
+	x = XCAR (a), y = XCAR (b);
+      if (FIXNUMP (x) && FIXNUMP (y))
+	return XFIXNUM (x) - XFIXNUM (y);
+    }
 
   /* Compare (ATICKS . AZ) to (BTICKS . BHZ) by comparing
      ATICKS * BHZ to BTICKS * AHZ.  */
-- 
2.34.1