bug#54930: [PATCH] Avoid possibly unnecessary lisp_time_struct call

bug#54930: [PATCH] Avoid possibly unnecessary lisp_time_struct call

[Philip Kaludercic]

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

Tags: patch


Reading through time_cmp, I didn't understand why the lisp_time struct
was calculated for the first argument before checking if both are eq.
From what I see lisp_time struct is not a destructive function, so it
should be possible to call the function when calculating the lisp_time
struct for the second argument.

In GNU Emacs 29.0.50 (build 4, x86_64-pc-linux-gnu, GTK+ Version 3.24.30, cairo version 1.16.0)
 of 2022-04-10 built on viero
Repository revision: 0023cfdc4e6d348519198cf75553d1afad0ed153
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12011000
System Description: Guix System

Configured using:
 'configure
 PKG_CONFIG_PATH=/home/philip/.guix-profile/lib/pkgconfig:/home/philip/.guix-profile/share/pkgconfig'


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Avoid-possibly-unnecessary-lisp_time_struct-call.patch --]
[-- Type: text/patch, Size: 1248 bytes --]

From 1a94c8fc3256c8d73e4b7f4d8a16aab52bb952d9 Mon Sep 17 00:00:00 2001
From: Philip Kaludercic <philipk@posteo.net>
Date: Thu, 14 Apr 2022 11:00:07 +0200
Subject: [PATCH] Avoid possibly unnecessary lisp_time_struct call
* timefns.c (time_cmp): Defer the calculation of the time struct, in
  case A and B are eq to one another.
---
 src/timefns.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/timefns.c b/src/timefns.c
index 9e8592d35a..6cfb787af8 100644
--- a/src/timefns.c
+++ b/src/timefns.c
@@ -1219,8 +1219,6 @@ time_cmp (Lisp_Object a, Lisp_Object b)
       return da < db ? -1 : da != db;
     }
 
-  struct lisp_time ta = lisp_time_struct (a, 0);
-
   /* Compare nil to nil correctly, and handle other eq values quicker
      while we're at it.  Compare here rather than earlier, to handle
      NaNs and check formats.  */
@@ -1229,6 +1227,7 @@ time_cmp (Lisp_Object a, Lisp_Object b)
 
   /* Compare (ATICKS . AZ) to (BTICKS . BHZ) by comparing
      ATICKS * BHZ to BTICKS * AHZ.  */
+  struct lisp_time ta = lisp_time_struct (a, 0);
   struct lisp_time tb = lisp_time_struct (b, 0);
   mpz_t const *za = bignum_integer (&mpz[0], ta.ticks);
   mpz_t const *zb = bignum_integer (&mpz[1], tb.ticks);
-- 
2.34.0


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


-- 
	Philip Kaludercic

bug#54930: [PATCH] Avoid possibly unnecessary lisp_time_struct call

[Lars Ingebrigtsen]

Philip Kaludercic <philipk@posteo.net> writes:

> Reading through time_cmp, I didn't understand why the lisp_time struct
> was calculated for the first argument before checking if both are eq.
>>From what I see lisp_time struct is not a destructive function, so it
> should be possible to call the function when calculating the lisp_time
> struct for the second argument.

Looks good to me; please go ahead and push.

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