* `org-clock--oldest-date` performance @ 2018-01-20 6:52 Jack Henahan 2018-01-20 11:14 ` Nicolas Goaziou 0 siblings, 1 reply; 11+ messages in thread From: Jack Henahan @ 2018-01-20 6:52 UTC (permalink / raw) To: emacs-orgmode 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: `org-clock--oldest-date` performance 2018-01-20 6:52 `org-clock--oldest-date` performance Jack Henahan @ 2018-01-20 11:14 ` Nicolas Goaziou 2018-01-20 17:00 ` [PATCH] " Jack Henahan 0 siblings, 1 reply; 11+ messages in thread From: Nicolas Goaziou @ 2018-01-20 11:14 UTC (permalink / raw) To: Jack Henahan; +Cc: emacs-orgmode 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] `org-clock--oldest-date` performance 2018-01-20 11:14 ` Nicolas Goaziou @ 2018-01-20 17:00 ` Jack Henahan 2018-01-20 17:05 ` Jack Henahan 0 siblings, 1 reply; 11+ messages in thread From: Jack Henahan @ 2018-01-20 17:00 UTC (permalink / raw) To: emacs-orgmode [-- 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] `org-clock--oldest-date` performance 2018-01-20 17:00 ` [PATCH] " Jack Henahan @ 2018-01-20 17:05 ` Jack Henahan 2018-01-20 17:09 ` Jack Henahan 0 siblings, 1 reply; 11+ messages in thread From: Jack Henahan @ 2018-01-20 17:05 UTC (permalink / raw) To: emacs-orgmode [-- 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] `org-clock--oldest-date` performance 2018-01-20 17:05 ` Jack Henahan @ 2018-01-20 17:09 ` Jack Henahan 2018-01-20 18:18 ` Nicolas Goaziou 0 siblings, 1 reply; 11+ messages in thread From: Jack Henahan @ 2018-01-20 17:09 UTC (permalink / raw) To: emacs-orgmode [-- 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] `org-clock--oldest-date` performance 2018-01-20 17:09 ` Jack Henahan @ 2018-01-20 18:18 ` Nicolas Goaziou 2018-01-20 20:32 ` Jack Henahan 0 siblings, 1 reply; 11+ messages in thread From: Nicolas Goaziou @ 2018-01-20 18:18 UTC (permalink / raw) To: Jack Henahan; +Cc: emacs-orgmode 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] `org-clock--oldest-date` performance 2018-01-20 18:18 ` Nicolas Goaziou @ 2018-01-20 20:32 ` Jack Henahan 2018-01-20 21:06 ` Jack Henahan 2018-01-21 9:42 ` Nicolas Goaziou 0 siblings, 2 replies; 11+ messages in thread From: Jack Henahan @ 2018-01-20 20:32 UTC (permalink / raw) To: emacs-orgmode 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] `org-clock--oldest-date` performance 2018-01-20 20:32 ` Jack Henahan @ 2018-01-20 21:06 ` Jack Henahan 2018-01-21 9:42 ` Nicolas Goaziou 1 sibling, 0 replies; 11+ messages in thread From: Jack Henahan @ 2018-01-20 21:06 UTC (permalink / raw) To: emacs-orgmode [-- 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] `org-clock--oldest-date` performance 2018-01-20 20:32 ` Jack Henahan 2018-01-20 21:06 ` Jack Henahan @ 2018-01-21 9:42 ` Nicolas Goaziou 2018-01-21 16:04 ` Jack Henahan 1 sibling, 1 reply; 11+ messages in thread From: Nicolas Goaziou @ 2018-01-21 9:42 UTC (permalink / raw) To: Jack Henahan; +Cc: emacs-orgmode 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] `org-clock--oldest-date` performance 2018-01-21 9:42 ` Nicolas Goaziou @ 2018-01-21 16:04 ` Jack Henahan 2018-01-21 22:30 ` Nicolas Goaziou 0 siblings, 1 reply; 11+ messages in thread From: Jack Henahan @ 2018-01-21 16:04 UTC (permalink / raw) To: emacs-orgmode [-- 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] `org-clock--oldest-date` performance 2018-01-21 16:04 ` Jack Henahan @ 2018-01-21 22:30 ` Nicolas Goaziou 0 siblings, 0 replies; 11+ messages in thread From: Nicolas Goaziou @ 2018-01-21 22:30 UTC (permalink / raw) To: Jack Henahan; +Cc: emacs-orgmode 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-01-21 22:30 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-01-20 6:52 `org-clock--oldest-date` performance Jack Henahan 2018-01-20 11:14 ` Nicolas Goaziou 2018-01-20 17:00 ` [PATCH] " Jack Henahan 2018-01-20 17:05 ` Jack Henahan 2018-01-20 17:09 ` Jack Henahan 2018-01-20 18:18 ` Nicolas Goaziou 2018-01-20 20:32 ` Jack Henahan 2018-01-20 21:06 ` Jack Henahan 2018-01-21 9:42 ` Nicolas Goaziou 2018-01-21 16:04 ` Jack Henahan 2018-01-21 22:30 ` Nicolas Goaziou
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).