`org-clock--oldest-date` performance

`org-clock--oldest-date` performance

[Jack Henahan]

Hiya,

I've run into a performance issue in `org-clock` which I've narrowed
down to being caused by the calculation in the defconst for
`org-clock--oldest-date`. In particular, invoking `org-clock-in` or
eagerly loading `org-clock` on init incurs a 21(!) second delay while
calculating the constant. If I inline the value (`(-1034058236842
-45726)`, in my case), the delay vanishes.

So, context out of the way (just in case someone else already knows an
easier fix), I'd like to spend some spare cycles finding a better way to
go about the functionality this is meant to provide. If I've read the
source correctly, it's meant to provide a view of all the clocks by
showing all clocks between some time way in the past until now.

I've read GNU Emacs bug #27706 [1] and attempted to reproduce to ensure
that it's not just the libc issue again, but none of the apparently
problematic examples cause any sort of hang.

This leads me to believe that it's the `dichotomy` algorithm getting a
bit out of control when `most-negative-fixnum` is -2^61.

Right now I'm just `el-patch`ing the defconst and
`org-clock-special-range`, but a more permanent solution would be
desirable.

I'll be looking at it this weekend to see if I can figure something out,
but I thought it likely that someone who understands `org-clock` better
than I might be able to chime in, or at least see if I'm the only one
hitting this issue.

Thanks,
==
Jack Henahan
Org mode version 9.1.6 (release_9.1.6-369-g3604f2) on macOS 10.13.2

Re: `org-clock--oldest-date` performance

[Nicolas Goaziou]

Hello,

Jack Henahan <jhenahan@me.com> writes:

> I've run into a performance issue in `org-clock` which I've narrowed
> down to being caused by the calculation in the defconst for
> `org-clock--oldest-date`. In particular, invoking `org-clock-in` or
> eagerly loading `org-clock` on init incurs a 21(!) second delay while
> calculating the constant. If I inline the value (`(-1034058236842
> -45726)`, in my case), the delay vanishes.
>
> So, context out of the way (just in case someone else already knows an
> easier fix), I'd like to spend some spare cycles finding a better way to
> go about the functionality this is meant to provide. If I've read the
> source correctly, it's meant to provide a view of all the clocks by
> showing all clocks between some time way in the past until now.

A correct fix would be to remove `org-clock--oldest-date', which is used
only in one place, and replace it with nil. Then all
`org-clock-special-range' callers need to be updated to handle this nil
start value.

Regards,

-- 
Nicolas Goaziou

Re: [PATCH] `org-clock--oldest-date` performance

[Jack Henahan]

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


To that end, I've attached a patch for review which removes
`org-clock--oldest-date`, replacing its only use with `nil`, and
altering the logic where it's actually used to account for this case and
give a sensible-ish value of the year -50000 for the start time in
`org-special-range`. Before the dawn of humanity seemed like a
reasonable limit, but I'm taking suggestions. :D

I'm not certain if this hits the "modify 15 lines" threshold since it's
mainly deletion, but I'll start getting the paperwork in order and write
a Changelog entry.

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> Hello,
>
> Jack Henahan <jhenahan@me.com> writes:
>
>> I've run into a performance issue in `org-clock` which I've narrowed
>> down to being caused by the calculation in the defconst for
>> `org-clock--oldest-date`. In particular, invoking `org-clock-in` or
>> eagerly loading `org-clock` on init incurs a 21(!) second delay while
>> calculating the constant. If I inline the value (`(-1034058236842
>> -45726)`, in my case), the delay vanishes.
>>
>> So, context out of the way (just in case someone else already knows an
>> easier fix), I'd like to spend some spare cycles finding a better way to
>> go about the functionality this is meant to provide. If I've read the
>> source correctly, it's meant to provide a view of all the clocks by
>> showing all clocks between some time way in the past until now.
>
> A correct fix would be to remove `org-clock--oldest-date', which is used
> only in one place, and replace it with nil. Then all
> `org-clock-special-range' callers need to be updated to handle this nil
> start value.
>
> Regards,

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Set-untilnow-to-use-the-year-50000-rather-than-the-e.patch --]
[-- Type: text/x-patch, Size: 2840 bytes --]

From ba4f38b8337c83330f303e10e3fbf1a251a58fea Mon Sep 17 00:00:00 2001
From: Jack Henahan <jack.henahan@uvmhealth.org>
Date: Sat, 20 Jan 2018 11:35:33 -0500
Subject: [PATCH] Set `untilnow` to use the year -50000, rather than the
 earliest representable date.

---
 lisp/org-clock.el | 42 +++++++-----------------------------------
 1 file changed, 7 insertions(+), 35 deletions(-)

diff --git a/lisp/org-clock.el b/lisp/org-clock.el
index 496c4310a..519b1563b 100644
--- a/lisp/org-clock.el
+++ b/lisp/org-clock.el
@@ -468,38 +468,6 @@ to add an effort property.")
 (defvar org-clock-stored-resume-clock nil
   "Clock to resume, saved by `org-clock-load'")
 
-(defconst org-clock--oldest-date
-  (let* ((dichotomy
-	  (lambda (min max pred)
-	    (if (funcall pred min) min
-	      (cl-incf min)
-	      (while (> (- max min) 1)
-		(let ((mean (+ (ash min -1) (ash max -1) (logand min max 1))))
-		  (if (funcall pred mean) (setq max mean) (setq min mean)))))
-	    max))
-	 (high
-	  (funcall dichotomy
-		   most-negative-fixnum
-		   0
-		   (lambda (m)
-                     ;; libc in macOS 10.6 hangs when decoding times
-                     ;; around year -2**31.  Limit `high' not to go
-                     ;; any earlier than that.
-                     (unless (and (eq system-type 'darwin)
-                                  (string-match-p
-                                   "10\\.6\\.[[:digit:]]"
-                                   (shell-command-to-string
-                                    "sw_vers -productVersion"))
-                                  (<= m -1034058203135))
-                       (ignore-errors (decode-time (list m 0)))))))
-	 (low
-	  (funcall dichotomy
-		   most-negative-fixnum
-		   0
-		   (lambda (m) (ignore-errors (decode-time (list high m)))))))
-    (list high low))
-  "Internal time for oldest date representable on the system.")
-
 ;;; The clock for measuring work time.
 
 (defvar org-mode-line-string "")
@@ -2260,7 +2228,7 @@ have priority."
     ;; Format start and end times according to AS-STRINGS.
     (let* ((start (pcase key
 		    (`interactive (org-read-date nil t nil "Range start? "))
-		    (`untilnow org-clock--oldest-date)
+		    (`untilnow nil)
 		    (_ (encode-time 0 m h d month y))))
 	   (end (pcase key
 		  (`interactive (org-read-date nil t nil "Range end? "))
@@ -2283,8 +2251,12 @@ have priority."
 	      (`interactive "(Range interactively set)")
 	      (`untilnow "now"))))
       (if (not as-strings) (list start end text)
-	(let ((f (cdr org-time-stamp-formats)))
-	  (list (format-time-string f start)
+	(let ((f (cdr org-time-stamp-formats))
+              (safe-start
+	       (if (not start)
+		   (encode-time 0 0 0 0 0 -50000)
+		 start)))
+	  (list (format-time-string f safe-start)
 		(format-time-string f end)
 		text))))))
 
-- 
2.15.1

Re: [PATCH] `org-clock--oldest-date` performance

[Jack Henahan]

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

Jack Henahan <jhenahan@me.com> writes:

Modified patch attached to use my list mail rather than my work one.

> To that end, I've attached a patch for review which removes
> `org-clock--oldest-date`, replacing its only use with `nil`, and
> altering the logic where it's actually used to account for this case and
> give a sensible-ish value of the year -50000 for the start time in
> `org-special-range`. Before the dawn of humanity seemed like a
> reasonable limit, but I'm taking suggestions. :D
>
> I'm not certain if this hits the "modify 15 lines" threshold since it's
> mainly deletion, but I'll start getting the paperwork in order and write
> a Changelog entry.
>
> Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:
>
>> Hello,
>>
>> Jack Henahan <jhenahan@me.com> writes:
>>
>>> I've run into a performance issue in `org-clock` which I've narrowed
>>> down to being caused by the calculation in the defconst for
>>> `org-clock--oldest-date`. In particular, invoking `org-clock-in` or
>>> eagerly loading `org-clock` on init incurs a 21(!) second delay while
>>> calculating the constant. If I inline the value (`(-1034058236842
>>> -45726)`, in my case), the delay vanishes.
>>>
>>> So, context out of the way (just in case someone else already knows an
>>> easier fix), I'd like to spend some spare cycles finding a better way to
>>> go about the functionality this is meant to provide. If I've read the
>>> source correctly, it's meant to provide a view of all the clocks by
>>> showing all clocks between some time way in the past until now.
>>
>> A correct fix would be to remove `org-clock--oldest-date', which is used
>> only in one place, and replace it with nil. Then all
>> `org-clock-special-range' callers need to be updated to handle this nil
>> start value.
>>
>> Regards,

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Set-untilnow-to-use-the-year-50000-rather-than-the-e.patch --]
[-- Type: text/x-patch, Size: 2829 bytes --]

From ba4f38b8337c83330f303e10e3fbf1a251a58fea Mon Sep 17 00:00:00 2001
From: Jack Henahan <jhenahan@me.com>
Date: Sat, 20 Jan 2018 11:35:33 -0500
Subject: [PATCH] Set `untilnow` to use the year -50000, rather than the
 earliest representable date.

---
 lisp/org-clock.el | 42 +++++++-----------------------------------
 1 file changed, 7 insertions(+), 35 deletions(-)

diff --git a/lisp/org-clock.el b/lisp/org-clock.el
index 496c4310a..519b1563b 100644
--- a/lisp/org-clock.el
+++ b/lisp/org-clock.el
@@ -468,38 +468,6 @@ to add an effort property.")
 (defvar org-clock-stored-resume-clock nil
   "Clock to resume, saved by `org-clock-load'")
 
-(defconst org-clock--oldest-date
-  (let* ((dichotomy
-	  (lambda (min max pred)
-	    (if (funcall pred min) min
-	      (cl-incf min)
-	      (while (> (- max min) 1)
-		(let ((mean (+ (ash min -1) (ash max -1) (logand min max 1))))
-		  (if (funcall pred mean) (setq max mean) (setq min mean)))))
-	    max))
-	 (high
-	  (funcall dichotomy
-		   most-negative-fixnum
-		   0
-		   (lambda (m)
-                     ;; libc in macOS 10.6 hangs when decoding times
-                     ;; around year -2**31.  Limit `high' not to go
-                     ;; any earlier than that.
-                     (unless (and (eq system-type 'darwin)
-                                  (string-match-p
-                                   "10\\.6\\.[[:digit:]]"
-                                   (shell-command-to-string
-                                    "sw_vers -productVersion"))
-                                  (<= m -1034058203135))
-                       (ignore-errors (decode-time (list m 0)))))))
-	 (low
-	  (funcall dichotomy
-		   most-negative-fixnum
-		   0
-		   (lambda (m) (ignore-errors (decode-time (list high m)))))))
-    (list high low))
-  "Internal time for oldest date representable on the system.")
-
 ;;; The clock for measuring work time.
 
 (defvar org-mode-line-string "")
@@ -2260,7 +2228,7 @@ have priority."
     ;; Format start and end times according to AS-STRINGS.
     (let* ((start (pcase key
 		    (`interactive (org-read-date nil t nil "Range start? "))
-		    (`untilnow org-clock--oldest-date)
+		    (`untilnow nil)
 		    (_ (encode-time 0 m h d month y))))
 	   (end (pcase key
 		  (`interactive (org-read-date nil t nil "Range end? "))
@@ -2283,8 +2251,12 @@ have priority."
 	      (`interactive "(Range interactively set)")
 	      (`untilnow "now"))))
       (if (not as-strings) (list start end text)
-	(let ((f (cdr org-time-stamp-formats)))
-	  (list (format-time-string f start)
+	(let ((f (cdr org-time-stamp-formats))
+              (safe-start
+	       (if (not start)
+		   (encode-time 0 0 0 0 0 -50000)
+		 start)))
+	  (list (format-time-string f safe-start)
 		(format-time-string f end)
 		text))))))
 
-- 
2.15.1

Re: [PATCH] `org-clock--oldest-date` performance

[Jack Henahan]

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

Jack Henahan <jhenahan@me.com> writes:

Apologies again, didn't update the commit hash properly. I swear this is
the last one. :|

> Jack Henahan <jhenahan@me.com> writes:
>
> Modified patch attached to use my list mail rather than my work one.
>
>> To that end, I've attached a patch for review which removes
>> `org-clock--oldest-date`, replacing its only use with `nil`, and
>> altering the logic where it's actually used to account for this case and
>> give a sensible-ish value of the year -50000 for the start time in
>> `org-special-range`. Before the dawn of humanity seemed like a
>> reasonable limit, but I'm taking suggestions. :D
>>
>> I'm not certain if this hits the "modify 15 lines" threshold since it's
>> mainly deletion, but I'll start getting the paperwork in order and write
>> a Changelog entry.
>>
>> Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:
>>
>>> Hello,
>>>
>>> Jack Henahan <jhenahan@me.com> writes:
>>>
>>>> I've run into a performance issue in `org-clock` which I've narrowed
>>>> down to being caused by the calculation in the defconst for
>>>> `org-clock--oldest-date`. In particular, invoking `org-clock-in` or
>>>> eagerly loading `org-clock` on init incurs a 21(!) second delay while
>>>> calculating the constant. If I inline the value (`(-1034058236842
>>>> -45726)`, in my case), the delay vanishes.
>>>>
>>>> So, context out of the way (just in case someone else already knows an
>>>> easier fix), I'd like to spend some spare cycles finding a better way to
>>>> go about the functionality this is meant to provide. If I've read the
>>>> source correctly, it's meant to provide a view of all the clocks by
>>>> showing all clocks between some time way in the past until now.
>>>
>>> A correct fix would be to remove `org-clock--oldest-date', which is used
>>> only in one place, and replace it with nil. Then all
>>> `org-clock-special-range' callers need to be updated to handle this nil
>>> start value.
>>>
>>> Regards,

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Set-untilnow-to-use-the-year-50000-rather-than-the-e.patch --]
[-- Type: text/x-patch, Size: 2829 bytes --]

From 29969da1e47032f0b3691ba1dd14bd836488990d Mon Sep 17 00:00:00 2001
From: Jack Henahan <jhenahan@me.com>
Date: Sat, 20 Jan 2018 12:07:11 -0500
Subject: [PATCH] Set `untilnow` to use the year -50000, rather than the
 earliest representable date.

---
 lisp/org-clock.el | 42 +++++++-----------------------------------
 1 file changed, 7 insertions(+), 35 deletions(-)

diff --git a/lisp/org-clock.el b/lisp/org-clock.el
index 496c4310a..519b1563b 100644
--- a/lisp/org-clock.el
+++ b/lisp/org-clock.el
@@ -468,38 +468,6 @@ to add an effort property.")
 (defvar org-clock-stored-resume-clock nil
   "Clock to resume, saved by `org-clock-load'")
 
-(defconst org-clock--oldest-date
-  (let* ((dichotomy
-	  (lambda (min max pred)
-	    (if (funcall pred min) min
-	      (cl-incf min)
-	      (while (> (- max min) 1)
-		(let ((mean (+ (ash min -1) (ash max -1) (logand min max 1))))
-		  (if (funcall pred mean) (setq max mean) (setq min mean)))))
-	    max))
-	 (high
-	  (funcall dichotomy
-		   most-negative-fixnum
-		   0
-		   (lambda (m)
-                     ;; libc in macOS 10.6 hangs when decoding times
-                     ;; around year -2**31.  Limit `high' not to go
-                     ;; any earlier than that.
-                     (unless (and (eq system-type 'darwin)
-                                  (string-match-p
-                                   "10\\.6\\.[[:digit:]]"
-                                   (shell-command-to-string
-                                    "sw_vers -productVersion"))
-                                  (<= m -1034058203135))
-                       (ignore-errors (decode-time (list m 0)))))))
-	 (low
-	  (funcall dichotomy
-		   most-negative-fixnum
-		   0
-		   (lambda (m) (ignore-errors (decode-time (list high m)))))))
-    (list high low))
-  "Internal time for oldest date representable on the system.")
-
 ;;; The clock for measuring work time.
 
 (defvar org-mode-line-string "")
@@ -2260,7 +2228,7 @@ have priority."
     ;; Format start and end times according to AS-STRINGS.
     (let* ((start (pcase key
 		    (`interactive (org-read-date nil t nil "Range start? "))
-		    (`untilnow org-clock--oldest-date)
+		    (`untilnow nil)
 		    (_ (encode-time 0 m h d month y))))
 	   (end (pcase key
 		  (`interactive (org-read-date nil t nil "Range end? "))
@@ -2283,8 +2251,12 @@ have priority."
 	      (`interactive "(Range interactively set)")
 	      (`untilnow "now"))))
       (if (not as-strings) (list start end text)
-	(let ((f (cdr org-time-stamp-formats)))
-	  (list (format-time-string f start)
+	(let ((f (cdr org-time-stamp-formats))
+              (safe-start
+	       (if (not start)
+		   (encode-time 0 0 0 0 0 -50000)
+		 start)))
+	  (list (format-time-string f safe-start)
 		(format-time-string f end)
 		text))))))
 
-- 
2.15.1

Re: [PATCH] `org-clock--oldest-date` performance

[Nicolas Goaziou]

Jack Henahan <jhenahan@me.com> writes:

> Apologies again, didn't update the commit hash properly. I swear this is
> the last one. :|

Thank you. However, I'm surprised that `org-clock-special-range' callers
handle a nil start date. Have you tested it?

If that's true, we don't need the -50000 hack at all. Returning an empty
string might be enough.

Re: [PATCH] `org-clock--oldest-date` performance

[Jack Henahan]

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

I tested that `org-clock-display` and the clocktable work as expected
when `org-clock-display-default-range` is set to `untilnow`.
`org-clock-sum-custom` also appears to function as intended. If I'm
reading things correctly, the `untilnow` case is the only one that ought
to be affected, since it's the only one that used
`org-clock--oldest-date`. The behavior of `org-clock-special-range`
ought to be unchanged in all cases except where this symbol is
explicitly used, or the start time is nil for some other reason.

Functionally, this means that today `org-clock-special-range` produces a
range from the current time until the current time if `start` ends up
nil for whatever reason, but with this patch it will instead produce a
range from the year -50000 until now. The -50000 hack is entirely for
the benefit of `format-time-string`, since otherwise it just gives the
current time if its second argument is nil.

> Jack Henahan <jhenahan@me.com> writes:
>
>> Apologies again, didn't update the commit hash properly. I swear this is
>> the last one. :|
>
> Thank you. However, I'm surprised that `org-clock-special-range' callers
> handle a nil start date. Have you tested it?
>
> If that's true, we don't need the -50000 hack at all. Returning an empty
> string might be enough.

Re: [PATCH] `org-clock--oldest-date` performance

[Jack Henahan]

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

I've sent off for the FSF form so I can get that process started, and
I've amended my patch according to the guidelines (attached).

Jack Henahan <jhenahan@me.com> writes:

> I tested that `org-clock-display` and the clocktable work as expected
> when `org-clock-display-default-range` is set to `untilnow`.
> `org-clock-sum-custom` also appears to function as intended. If I'm
> reading things correctly, the `untilnow` case is the only one that ought
> to be affected, since it's the only one that used
> `org-clock--oldest-date`. The behavior of `org-clock-special-range`
> ought to be unchanged in all cases except where this symbol is
> explicitly used, or the start time is nil for some other reason.
>
> Functionally, this means that today `org-clock-special-range` produces a
> range from the current time until the current time if `start` ends up
> nil for whatever reason, but with this patch it will instead produce a
> range from the year -50000 until now. The -50000 hack is entirely for
> the benefit of `format-time-string`, since otherwise it just gives the
> current time if its second argument is nil.
>
>> Jack Henahan <jhenahan@me.com> writes:
>>
>>> Apologies again, didn't update the commit hash properly. I swear this is
>>> the last one. :|
>>
>> Thank you. However, I'm surprised that `org-clock-special-range' callers
>> handle a nil start date. Have you tested it?
>>
>> If that's true, we don't need the -50000 hack at all. Returning an empty
>> string might be enough.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-clock.el-Improve-untilnow-range-behavior-and-per.patch --]
[-- Type: text/x-patch, Size: 3388 bytes --]

From a4add4ef44c4f445b4c029a0f0a7ef6f3d5d606b Mon Sep 17 00:00:00 2001
From: Jack Henahan <jhenahan@me.com>
Date: Sat, 20 Jan 2018 12:07:11 -0500
Subject: [PATCH] org-clock.el: Improve `untilnow' range behavior and
 performance

* org-clock.el:
(org-clock-special-range): Set `untilnow' to use the year -50000,
rather than the earliest representable date.
(org-clock--oldest-date): Remove.

The `untilnow' range relied on the constant `org-clock--oldest-date`
to find the earliest representable date, which caused delays when
loading `org-clock' on systems where `most-negative-fixnum' is large.
This change removes that constant in favor of a simpler hack to
produce a range between the current time and before the dawn of human
civilization. If this breaks your workflow, please report to the Time
Police.
---
 lisp/org-clock.el | 42 +++++++-----------------------------------
 1 file changed, 7 insertions(+), 35 deletions(-)

diff --git a/lisp/org-clock.el b/lisp/org-clock.el
index 496c4310a..519b1563b 100644
--- a/lisp/org-clock.el
+++ b/lisp/org-clock.el
@@ -468,38 +468,6 @@ to add an effort property.")
 (defvar org-clock-stored-resume-clock nil
   "Clock to resume, saved by `org-clock-load'")
 
-(defconst org-clock--oldest-date
-  (let* ((dichotomy
-	  (lambda (min max pred)
-	    (if (funcall pred min) min
-	      (cl-incf min)
-	      (while (> (- max min) 1)
-		(let ((mean (+ (ash min -1) (ash max -1) (logand min max 1))))
-		  (if (funcall pred mean) (setq max mean) (setq min mean)))))
-	    max))
-	 (high
-	  (funcall dichotomy
-		   most-negative-fixnum
-		   0
-		   (lambda (m)
-                     ;; libc in macOS 10.6 hangs when decoding times
-                     ;; around year -2**31.  Limit `high' not to go
-                     ;; any earlier than that.
-                     (unless (and (eq system-type 'darwin)
-                                  (string-match-p
-                                   "10\\.6\\.[[:digit:]]"
-                                   (shell-command-to-string
-                                    "sw_vers -productVersion"))
-                                  (<= m -1034058203135))
-                       (ignore-errors (decode-time (list m 0)))))))
-	 (low
-	  (funcall dichotomy
-		   most-negative-fixnum
-		   0
-		   (lambda (m) (ignore-errors (decode-time (list high m)))))))
-    (list high low))
-  "Internal time for oldest date representable on the system.")
-
 ;;; The clock for measuring work time.
 
 (defvar org-mode-line-string "")
@@ -2260,7 +2228,7 @@ have priority."
     ;; Format start and end times according to AS-STRINGS.
     (let* ((start (pcase key
 		    (`interactive (org-read-date nil t nil "Range start? "))
-		    (`untilnow org-clock--oldest-date)
+		    (`untilnow nil)
 		    (_ (encode-time 0 m h d month y))))
 	   (end (pcase key
 		  (`interactive (org-read-date nil t nil "Range end? "))
@@ -2283,8 +2251,12 @@ have priority."
 	      (`interactive "(Range interactively set)")
 	      (`untilnow "now"))))
       (if (not as-strings) (list start end text)
-	(let ((f (cdr org-time-stamp-formats)))
-	  (list (format-time-string f start)
+	(let ((f (cdr org-time-stamp-formats))
+              (safe-start
+	       (if (not start)
+		   (encode-time 0 0 0 0 0 -50000)
+		 start)))
+	  (list (format-time-string f safe-start)
 		(format-time-string f end)
 		text))))))
 
-- 
2.15.1

Re: [PATCH] `org-clock--oldest-date` performance

[Nicolas Goaziou]

Hello,

Jack Henahan <jhenahan@me.com> writes:

> Functionally, this means that today `org-clock-special-range` produces a
> range from the current time until the current time if `start` ends up
> nil for whatever reason

That's not true. Currently, `org-clock-special-range' never returns
a nil start time. This is where I don't understand your patch:

> -		    (`untilnow org-clock--oldest-date)
> +		    (`untilnow nil)
>  		    (_ (encode-time 0 m h d month y))))
>  	   (end (pcase key
>  		  (`interactive (org-read-date nil t nil "Range end? "))
> @@ -2283,8 +2251,12 @@ have priority."
>  	      (`interactive "(Range interactively set)")
>  	      (`untilnow "now"))))
>        (if (not as-strings) (list start end text)
                                    ^^^^^
                              this can now be nil
> -	(let ((f (cdr org-time-stamp-formats)))
> -	  (list (format-time-string f start)
> +	(let ((f (cdr org-time-stamp-formats))
> +              (safe-start
> +	       (if (not start)
> +		   (encode-time 0 0 0 0 0 -50000)
> +		 start)))
> +	  (list (format-time-string f safe-start)
>  		(format-time-string f end)
>  		text))))))

Why not replacing `org-clock--oldest-date' with (encode-time
0 0 0 0 0 -50000) in the let binding above?

Regards,

-- 
Nicolas Goaziou

Re: [PATCH] `org-clock--oldest-date` performance

[Jack Henahan]

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

I wasn't altogether certain that it could never be nil, and my eyes seem
to have skipped over the expression you called out so I missed that case
in my testing. I have reworked the patch as you suggest. Now the
behavior should be entirely unchanged except in the `untilnow' case
where we use the new date, no extra nils.

Revised patch attached.

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> Hello,
>
> Jack Henahan <jhenahan@me.com> writes:
>
>> Functionally, this means that today `org-clock-special-range` produces a
>> range from the current time until the current time if `start` ends up
>> nil for whatever reason
>
> That's not true. Currently, `org-clock-special-range' never returns
> a nil start time. This is where I don't understand your patch:
>
>> -		    (`untilnow org-clock--oldest-date)
>> +		    (`untilnow nil)
>>  		    (_ (encode-time 0 m h d month y))))
>>  	   (end (pcase key
>>  		  (`interactive (org-read-date nil t nil "Range end? "))
>> @@ -2283,8 +2251,12 @@ have priority."
>>  	      (`interactive "(Range interactively set)")
>>  	      (`untilnow "now"))))
>>        (if (not as-strings) (list start end text)
>                                     ^^^^^
>                               this can now be nil
>> -	(let ((f (cdr org-time-stamp-formats)))
>> -	  (list (format-time-string f start)
>> +	(let ((f (cdr org-time-stamp-formats))
>> +              (safe-start
>> +	       (if (not start)
>> +		   (encode-time 0 0 0 0 0 -50000)
>> +		 start)))
>> +	  (list (format-time-string f safe-start)
>>  		(format-time-string f end)
>>  		text))))))
>
> Why not replacing `org-clock--oldest-date' with (encode-time
> 0 0 0 0 0 -50000) in the let binding above?
>
> Regards,

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-clock.el-Improve-untilnow-range-behavior-and.patch --]
[-- Type: text/x-patch, Size: 3007 bytes --]

From a0792b3900812b6a2385794f30ba7c057bc9fcf7 Mon Sep 17 00:00:00 2001
From: Jack Henahan <jhenahan@me.com>
Date: Sun, 21 Jan 2018 10:40:22 -0500
Subject: [PATCH] org-clock.el: Improve `untilnow' range behavior and 
 performance

* org-clock.el:
(org-clock-special-range): Set `untilnow' to use the year -50000,
rather than the earliest representable date.
(org-clock--oldest-date): Remove.

The `untilnow' range relied on the constant `org-clock--oldest-date`
to find the earliest representable date, which caused delays when
loading `org-clock' on systems where `most-negative-fixnum' is large.
This change removes that constant in favor of a simpler hack to
produce a range between the current time and before the dawn of human
civilization. If this breaks your workflow, please report to the Time
Police.
---
 lisp/org-clock.el | 35 ++---------------------------------
 1 file changed, 2 insertions(+), 33 deletions(-)

diff --git a/lisp/org-clock.el b/lisp/org-clock.el
index 496c4310a..d580ffa43 100644
--- a/lisp/org-clock.el
+++ b/lisp/org-clock.el
@@ -468,38 +468,6 @@ to add an effort property.")
 (defvar org-clock-stored-resume-clock nil
   "Clock to resume, saved by `org-clock-load'")
 
-(defconst org-clock--oldest-date
-  (let* ((dichotomy
-	  (lambda (min max pred)
-	    (if (funcall pred min) min
-	      (cl-incf min)
-	      (while (> (- max min) 1)
-		(let ((mean (+ (ash min -1) (ash max -1) (logand min max 1))))
-		  (if (funcall pred mean) (setq max mean) (setq min mean)))))
-	    max))
-	 (high
-	  (funcall dichotomy
-		   most-negative-fixnum
-		   0
-		   (lambda (m)
-                     ;; libc in macOS 10.6 hangs when decoding times
-                     ;; around year -2**31.  Limit `high' not to go
-                     ;; any earlier than that.
-                     (unless (and (eq system-type 'darwin)
-                                  (string-match-p
-                                   "10\\.6\\.[[:digit:]]"
-                                   (shell-command-to-string
-                                    "sw_vers -productVersion"))
-                                  (<= m -1034058203135))
-                       (ignore-errors (decode-time (list m 0)))))))
-	 (low
-	  (funcall dichotomy
-		   most-negative-fixnum
-		   0
-		   (lambda (m) (ignore-errors (decode-time (list high m)))))))
-    (list high low))
-  "Internal time for oldest date representable on the system.")
-
 ;;; The clock for measuring work time.
 
 (defvar org-mode-line-string "")
@@ -2260,7 +2228,8 @@ have priority."
     ;; Format start and end times according to AS-STRINGS.
     (let* ((start (pcase key
 		    (`interactive (org-read-date nil t nil "Range start? "))
-		    (`untilnow org-clock--oldest-date)
+                    ;; In theory, all clocks started after the dawn of humanity
+		    (`untilnow (encode-time 0 0 0 0 0 -50000))
 		    (_ (encode-time 0 m h d month y))))
 	   (end (pcase key
 		  (`interactive (org-read-date nil t nil "Range end? "))
-- 
2.15.1

Re: [PATCH] `org-clock--oldest-date` performance

[Nicolas Goaziou]

Hello,

Jack Henahan <jhenahan@me.com> writes:

> Revised patch attached.

Applied. Thank you.

I added TINYCHANGE at the end of the commit message. Please let me know
when the process with the FSF is over.

Regards,

-- 
Nicolas Goaziou