all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Paul Eggert <eggert@cs.ucla.edu>
To: Kaushal Modi <kaushal.modi@gmail.com>
Cc: Emacs developers <emacs-devel@gnu.org>
Subject: Re: Timer errors in last few days (bignump change?)
Date: Wed, 5 Sep 2018 16:28:43 -0700	[thread overview]
Message-ID: <76ddfcc7-4852-b6de-b4bc-90bdd3e6a4a5@cs.ucla.edu> (raw)
In-Reply-To: <CAFyQvY3rwqLSTRONQjmK9iYBqJGwcdkBDqE7REFsErFDNb4rEg@mail.gmail.com>

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

Kaushal Modi wrote:

> Sorry for your wasted time. It was my mistake.. I redefined current-time.

No problem. In penance you might want to take a look at the attached, which 
fixes a rounding error in that area that I noticed while looking into your report.

> hat value even though way in future is still valid, right?

Depends on the platform. It's not valid on platforms with 32-bit signed time_t; 
it is valid on almost every other platform.

It
> just happens that  (encode-time 0 0 0 21 12 2100) returns that 2 element
> time list.

Not if time_t is 32-bit signed.

> Is the issue with:
> - time list having 2 elements?
> - year being 2100?
> - Or the delta between that fake `current-time' and real current-time too
> big?

The issue is that encode-time returns a format that 
timer-next-integral-multiple-of-time does not grok. I'm not sure it's worth 
fixing this now, since we're about to change timestamp formats anyway and I'd 
rather not be twiddling much with the old one (it'll still be supported blah 
blah blah, I just don't want to be enhancing it).

[-- Attachment #2: 0001-Fix-timer.el-minor-rounding-error.patch --]
[-- Type: text/x-patch, Size: 2184 bytes --]

From 67475a59e95919e2dbe25ae950450578afdfd0dc Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@Penguin.CS.UCLA.EDU>
Date: Wed, 5 Sep 2018 16:19:47 -0700
Subject: [PATCH] Fix timer.el minor rounding error

* lisp/emacs-lisp/timer.el (timer-next-integral-multiple-of-time):
Fix rounding error by using integers rather than floats.
* test/lisp/emacs-lisp/timer-tests.el (timer-test-multiple-of-time):
New test.
---
 lisp/emacs-lisp/timer.el            | 10 +++++-----
 test/lisp/emacs-lisp/timer-tests.el |  5 +++++
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/lisp/emacs-lisp/timer.el b/lisp/emacs-lisp/timer.el
index 795554fec5..74d37b0eae 100644
--- a/lisp/emacs-lisp/timer.el
+++ b/lisp/emacs-lisp/timer.el
@@ -102,14 +102,14 @@ timer-next-integral-multiple-of-time
   "Yield the next value after TIME that is an integral multiple of SECS.
 More precisely, the next value, after TIME, that is an integral multiple
 of SECS seconds since the epoch.  SECS may be a fraction."
-  (let* ((trillion 1e12)
+  (let* ((trillion 1000000000000)
 	 (time-sec (+ (nth 1 time)
-		      (* 65536.0 (nth 0 time))))
+		      (* 65536 (nth 0 time))))
 	 (delta-sec (mod (- time-sec) secs))
-	 (next-sec (+ time-sec (ffloor delta-sec)))
-	 (next-sec-psec (ffloor (* trillion (mod delta-sec 1))))
+	 (next-sec (+ time-sec (floor delta-sec)))
+	 (next-sec-psec (floor (* trillion (mod delta-sec 1))))
 	 (sub-time-psec (+ (or (nth 3 time) 0)
-			   (* 1e6 (nth 2 time))))
+			   (* 1000000 (nth 2 time))))
 	 (psec-diff (- sub-time-psec next-sec-psec)))
     (if (and (<= next-sec time-sec) (< 0 psec-diff))
 	(setq next-sec-psec (+ sub-time-psec
diff --git a/test/lisp/emacs-lisp/timer-tests.el b/test/lisp/emacs-lisp/timer-tests.el
index 65e5dc9bde..fa92c1b64a 100644
--- a/test/lisp/emacs-lisp/timer-tests.el
+++ b/test/lisp/emacs-lisp/timer-tests.el
@@ -39,4 +39,9 @@
   (if (fboundp 'debug-timer-check)
       (should (debug-timer-check)) t))
 
+(ert-deftest timer-test-multiple-of-time ()
+  (should (equal
+           (timer-next-integral-multiple-of-time '(0 0 0 1) (1+ (ash 1 53)))
+           (list (ash 1 (- 53 16)) 1 0 0))))
+
 ;;; timer-tests.el ends here
-- 
2.17.1


      parent reply	other threads:[~2018-09-05 23:28 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-05 17:54 Timer errors in last few days (bignump change?) Kaushal Modi
2018-09-05 18:39 ` Kaushal Modi
2018-09-05 18:43   ` Noam Postavsky
2018-09-05 18:45   ` Eli Zaretskii
2018-09-05 18:49     ` Kaushal Modi
2018-09-05 18:53       ` Kaushal Modi
2018-09-05 19:23         ` Eli Zaretskii
2018-09-05 22:34 ` Paul Eggert
2018-09-05 22:44   ` Kaushal Modi
2018-09-05 22:55     ` Kaushal Modi
2018-09-05 23:28     ` Paul Eggert [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=76ddfcc7-4852-b6de-b4bc-90bdd3e6a4a5@cs.ucla.edu \
    --to=eggert@cs.ucla.edu \
    --cc=emacs-devel@gnu.org \
    --cc=kaushal.modi@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.