manual - explain optimizations

doc/manual.org

@@ -19,11 +19,11 @@ All of these are optional, but recommended for the best experience.
 
 * Explanation
 :PROPERTIES:
-:DESCRIPTION: Descriptions of the design, implementation, and history
+:DESCRIPTION: The design, the implementation, and a little history
 :END:
 ** Design goals
 :PROPERTIES:
-:DESCRIPTION: Some vague ideas which guided the project
+:DESCRIPTION: Some vague objectives which guided the project
 :END:
     1. Don't make assumptions about the user's profession
        - e.g. timeclock seems to assume you're using it for a 9-to-5/contractor job
@@ -66,6 +66,35 @@ All three components use timers to keep their buffers updated. [[file:../elisp/c
 
 Note - sometimes, when hacking or dealing with errors, timers may result in subtle bugs which are very hard to debug. Using =chronometrist-force-restart-timer= or restarting Emacs can fix them, so try that as a first sanity check.
 
+** Chronometrist
+:PROPERTIES:
+:DESCRIPTION: The primary command and its associated buffer.
+:END:
+
+*** Optimization
+It is of great importance that Chronometrist be responsive -
++ A responsive program is more likely to be used; recall our design goal of 'incentivizing use'.
++ Being an Emacs program, freezing the UI for any human-noticeable length of time is unacceptable - it prevents the user from working on anything in their environment.
+Thus, I have considered various optimization strategies, and so far implemented two.
+
+**** Prevent excess creation of file watchers
+One of the earliest 'optimizations' of great importance turned out to simply be a bug - turns out, if you run an identical call to [[elisp:(describe-function 'file-notify-add-watch)][=file-notify-add-watch=]] twice, you create /two/ file watchers and your callback will be called /twice./ We were creating a file watcher /each time the chronometrist command was run./ 🤦 This was causing humongous slowdowns each time the file changed. 😅
++ It was fixed in v0.2.2 by making the watch creation conditional, using [[file:../elisp/chronometrist-common.el::defvar chronometrist--fs-watch ][=chronometrist--fs-watch=]] to store the watch object.
+
+**** Preserve state
+The next one was released in v0.5. Till then, any time the [[file:../elisp/chronometrist-custom.el::defcustom chronometrist-file (][=chronometrist-file=]] was modified, we'd clear the [[file:../elisp/chronometrist-events.el::defvar chronometrist-events (][=chronometrist-events=]] hash table and read data into it again. The reading itself is nearly-instant, even with ~2 years' worth of data [fn:1] (it uses Emacs' [[elisp:(describe-function 'read)][=read=]], after all), but the splitting of [[* Midnight-spanning events][midnight-spanning events]] is the real performance killer.
+
+After the optimization...
+1. Two backend functions ([[file:../elisp/chronometrist-sexp.el::cl-defun chronometrist-sexp-new (][=chronometrist-sexp-new=]] and [[file:../elisp/chronometrist-sexp.el::defun chronometrist-sexp-replace-last (][=chronometrist-sexp-replace-last=]]) were modified to set a flag ([[file:../elisp/chronometrist.el::defvar chronometrist--inhibit-read-p ][=chronometrist--inhibit-read-p=]]) before saving the file.
+2. If this flag is non-nil, [[file:../elisp/chronometrist.el::defun chronometrist-refresh-file (][=chronometrist-refresh-file=]] skips the expensive calls to =chronometrist-events-populate=, =chronometrist-tasks-from-table=, and =chronometrist-tags-history-populate=, and resets the flag.
+3. Instead, the aforementioned backend functions modify the relevant variables - =chronometrist-events=, =chronometrist-task-list=, and =chronometrist-tags-history= - via...
+   * =chronometrist-events-add= / =chronometrist-events-replace-last=
+   * =chronometrist-task-list-add=, and
+   * =chronometrist-tags-history-add= / =chronometrist-tags-history-replace-last=, respectively.
+
+There are still some operations which [[file:../elisp/chronometrist.el::defun chronometrist-refresh-file (][=chronometrist-refresh-file=]] runs unconditionally - which is to say there is scope for further optimization, if or when required.
+
+[fn:1] As indicated by exploratory work in the =parsimonious-reading= branch, where I made a loop to only =read= and collect s-expressions from the file. It was near-instant...until I added event splitting to it.
 ** Midnight-spanning events
 :PROPERTIES:
 :DESCRIPTION: Events starting on one day and ending on another
@@ -73,12 +102,15 @@ Note - sometimes, when hacking or dealing with errors, timers may result in subt
 A unique problem in working with Chronometrist, one I had never foreseen, was tasks which start on one day and end on another. These mess up data consumption (especially interval calculations and acquiring data for a specific date) in all sorts of unforeseen ways.
 
 There are a few different approaches of dealing with them. (Currently, Chronometrist uses #3.)
-*** (timeclock format) When the code of the first event in the day is "o", it's a midnight-spanning event.
-     + Advantage - very simple to detect
-     + Disadvantage - "in" and "out" events must be represented separately
+*** Check the code of the first event of the day (timeclock format)
+:PROPERTIES:
+:DESCRIPTION: When the code of the first event in the day is "o", it's a midnight-spanning event.
+:END:
+  + Advantage - very simple to detect
+  + Disadvantage - "in" and "out" events must be represented separately
 *** Split them at the file level
      + Advantage - operation is performed only once for each such event + simpler data-consuming code + reduced post-parsing load.
-     + What happens when the user changes their day-start-time? The split-up events are now split wrongly, and the second event may get split _again._
+     + What happens when the user changes their day-start-time? The split-up events are now split wrongly, and the second event may get split /again./
        Possible solutions -
        1. Add function to check if, for two events A and B, the :stop of A is the same as the :start of B, and that all their other tags are identical. Then we can re-split them according to the new day-start-time.
        2. Add a :split tag to split events. It can denote that the next event was originally a part of this one.
@@ -121,26 +153,26 @@ A quick description, starting from the first time [[file:../elisp/chronometrist-
 :END:
 [[file:../elisp/chronometrist-key-values.el][chronometrist-key-values.el]] deals with adding additional information to events, in the form of key-values and tags.
 
-Key-values are stored as plist keywords and values. The user can add any keywords except =:name=, =:tags=, =:start=, and =:stop=. [fn:1] Values can be any readable Lisp values.
+Key-values are stored as plist keywords and values. The user can add any keywords except =:name=, =:tags=, =:start=, and =:stop=. [fn:2] Values can be any readable Lisp values.
 
 Similarly, tags are stored using a =:tags (<tag>*)= keyword-value pair. The tags themselves (the elements of the list) can be any readable Lisp value.
 
-[fn:1] To remove this restriction, I had briefly considered making a keyword called =:user=, whose value would be another plist containing all user-defined keyword-values. But in practice, this hasn't been a big enough issue yet to justify the work.
+[fn:2] To remove this restriction, I had briefly considered making a keyword called =:user=, whose value would be another plist containing all user-defined keyword-values. But in practice, this hasn't been a big enough issue yet to justify the work.
 *** User input
-The entry points are [[file:../elisp/chronometrist-key-values.el::defun%20chronometrist-kv-add (][=chronometrist-kv-add=]] and [[file:../elisp/chronometrist-key-values.el::defun%20chronometrist-tags-add (][=chronometrist-tags-add=]]. The user adds these to the desired hooks, and they prompt the user for tags/key-values.
+The entry points are [[file:../elisp/chronometrist-key-values.el::defun chronometrist-kv-add (][=chronometrist-kv-add=]] and [[file:../elisp/chronometrist-key-values.el::defun chronometrist-tags-add (][=chronometrist-tags-add=]]. The user adds these to the desired hooks, and they prompt the user for tags/key-values.
 
 Both have corresponding functions to create a prompt -
 + [[file:../elisp/chronometrist-key-values.el::defun chronometrist-key-prompt (][=chronometrist-key-prompt=]],
 + [[file:../elisp/chronometrist-key-values.el::defun chronometrist-value-prompt (][=chronometrist-value-prompt=]], and
 + [[file:../elisp/chronometrist-key-values.el::defun chronometrist-tags-prompt (][=chronometrist-tags-prompt=]].
 
-[[file:../elisp/chronometrist-key-values.el::defun%20chronometrist-kv-add (][=chronometrist-kv-add=]]'s way of reading key-values from the user is somewhat different from most Emacs prompts - it creates a new buffer, and uses the minibuffer to alternatingly ask for keys and values in a loop. Key-values are inserted into the buffer as the user enters/selects them. The user can break out of this loop with an empty input (the keys to accept an empty input differ between completion systems, so we try to let the user know about them using [[file:../elisp/chronometrist-key-values.el::defun chronometrist-kv-completion-quit-key (][=chronometrist-kv-completion-quit-key=]]). After exiting the loop, they can edit the key-values in the buffer, and use the commands [[file:../elisp/chronometrist-key-values.el::defun chronometrist-kv-accept (][=chronometrist-kv-accept=]] to accept the key-values (which uses [[file:../elisp/chronometrist-key-values.el::defun chronometrist-append-to-last (][=chronometrist-append-to-last=]] to add them to the last plist in =chronometrist-file=) or [[file:../elisp/chronometrist-key-values.el::defun chronometrist-kv-reject (][=chronometrist-kv-reject=]] to discard them.
+[[file:../elisp/chronometrist-key-values.el::defun chronometrist-kv-add (][=chronometrist-kv-add=]]'s way of reading key-values from the user is somewhat different from most Emacs prompts - it creates a new buffer, and uses the minibuffer to alternatingly ask for keys and values in a loop. Key-values are inserted into the buffer as the user enters/selects them. The user can break out of this loop with an empty input (the keys to accept an empty input differ between completion systems, so we try to let the user know about them using [[file:../elisp/chronometrist-key-values.el::defun chronometrist-kv-completion-quit-key (][=chronometrist-kv-completion-quit-key=]]). After exiting the loop, they can edit the key-values in the buffer, and use the commands [[file:../elisp/chronometrist-key-values.el::defun chronometrist-kv-accept (][=chronometrist-kv-accept=]] to accept the key-values (which uses [[file:../elisp/chronometrist-key-values.el::defun chronometrist-append-to-last (][=chronometrist-append-to-last=]] to add them to the last plist in =chronometrist-file=) or [[file:../elisp/chronometrist-key-values.el::defun chronometrist-kv-reject (][=chronometrist-kv-reject=]] to discard them.
 
 *** History
 All prompts suggest past user inputs. These are queried from three history hash tables -
-+ [[file:../elisp/chronometrist-key-values.el::defvar%20chronometrist-key-history%20(][=chronometrist-key-history=]],
-+ [[file:../elisp/chronometrist-key-values.el::defvar%20chronometrist-value-history%20(][=chronometrist-value-history=]], and
-+ [[file:../elisp/chronometrist-key-values.el::defvar%20chronometrist-tags-history%20(][=chronometrist-tags-history=]].
++ [[file:../elisp/chronometrist-key-values.el::defvar chronometrist-key-history (][=chronometrist-key-history=]],
++ [[file:../elisp/chronometrist-key-values.el::defvar chronometrist-value-history (][=chronometrist-value-history=]], and
++ [[file:../elisp/chronometrist-key-values.el::defvar chronometrist-tags-history (][=chronometrist-tags-history=]].
 
 Each of these has a corresponding function to clear it and fill it with values -
 + [[file:../elisp/chronometrist-key-values.el::defun chronometrist-key-history-populate (][=chronometrist-key-history-populate=]]

elisp/chronometrist-sexp.el

@@ -112,7 +112,14 @@ were none."
     (chronometrist-sexp-delete-list)
     (chronometrist-plist-pp plist (current-buffer))
     (chronometrist-events-replace-last plist)
-    ;; We assume here that this function will always be used to replace something with the same :name. At the time of writing, this is indeed the case. The reason for this is that if the replaced plist is the only one in `chronometrist-file' with that :name, the :name should be removed from `chronometrist-task-list', but to ascertain that condition we would have to either read the entire file or map over the hash table, defeating the optimization. Thus, we don't update `chronometrist-task-list' here (unlike `chronometrist-sexp-new')
+    ;; We assume here that this function will always be used to
+    ;; replace something with the same :name. At the time of writing,
+    ;; this is indeed the case. The reason for this is that if the
+    ;; replaced plist is the only one in `chronometrist-file' with that :name, the
+    ;; :name should be removed from `chronometrist-task-list', but to ascertain
+    ;; that condition we would have to either read the entire file or
+    ;; map over the hash table, defeating the optimization. Thus, we
+    ;; don't update `chronometrist-task-list' here (unlike `chronometrist-sexp-new')
     (chronometrist-tags-history-replace-last plist)
     (setq chronometrist--inhibit-read-p t)
     (save-buffer)))