jiri/org-ql
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Change/Fix: (--select-cached) Query-cache key and narrowing

Browse source 
1.  It seems that caching was broken in
3adaf4e5fc, because I changed what key
was used to retrieve from the cache but not the key that was used to
store in the cache.  I guess I hadn't noticed the performance
difference.  It should be fixed now.

2.  While making this fix, the test suite caught a bug in the new,
almost-fixed version that was caused by not including the preamble in
the key, and it only did so because the tests ran sequentially in the
same Emacs, making use of caching.  If each test were independent and
ran with a clean cache, it wouldn't have caught the bug, and it would
undoubtedly have been quite a head-scratcher, or even a hair-puller,
at some future time.

3.  And I would have pushed a version including that bug if I didn't
have a git pre-push hook that runs the tests, because somehow I missed
running the tests on that particular change.

So, lessons reinforced: tests are good; automated tests are better;
enforced tests are best.

4.  Narrowed queries are now cached using point-min/max in the buffer.
This will be especially helpful for the WIP recursive queries feature.
I do wonder if the overhead of caching might be a drawback in some
cases, however some simple benchmarks of recursive queries that return
about 2,000 results shows a large improvement from caching, reducing
runtime from 3.12 to 0.24 seconds, so it's probably worth it, overall.
This commit is contained in:
Adam Porter 2019-08-30 09:34:34 -05:00
parent 802bc462d6
commit 8d57a69125
1 changed files with 12 additions and 7 deletions
Download patch file Download diff file Expand all files Collapse all files

19
org-ql.el
View file

@ -540,15 +540,20 @@ replace the clause with a preamble."
"Return results for ARGS and current buffer using cache." "Return results for ARGS and current buffer using cache."
;; MAYBE: Timeout cached queries. Probably not necessarily since they will be removed when a ;; MAYBE: Timeout cached queries. Probably not necessarily since they will be removed when a
;; buffer is closed, or when a query is run after modifying a buffer. ;; buffer is closed, or when a query is run after modifying a buffer.
;; FIXME: Narrowed queries will probably conflict in the cache, because the region is not (-let* (((&plist :query :preamble-re :action :narrow) args)
;; stored. We should either not cache narrow queries, or store the region with it. (query-cache-key
(-let (((&plist :query query :action action :narrow narrow) args)) ;; The key must include the preamble, because some queries are replaced by
;; the preamble, leaving a nil query, which would make the key ambiguous.
(list :query query :preamble-re preamble-re :action action
(if narrow
;; Use bounds of narrowed portion of buffer.
(cons (point-min) (point-max))
nil))))
(if-let* ((buffer-cache (gethash (current-buffer) org-ql-cache)) (if-let* ((buffer-cache (gethash (current-buffer) org-ql-cache))
(query-cache (cadr buffer-cache)) (query-cache (cadr buffer-cache))
(modified-tick (car buffer-cache)) (modified-tick (car buffer-cache))
(buffer-unmodified-p (eq (buffer-modified-tick) modified-tick)) (buffer-unmodified-p (eq (buffer-modified-tick) modified-tick))
(cache-key (list query action narrow)) (cached-result (gethash query-cache-key query-cache)))
(cached-result (gethash cache-key query-cache)))
(pcase cached-result (pcase cached-result
('org-ql-nil nil) ('org-ql-nil nil)
(_ cached-result)) (_ cached-result))
@ -558,10 +563,10 @@ replace the clause with a preamble."
(puthash (current-buffer) (puthash (current-buffer)
(list (buffer-modified-tick) (list (buffer-modified-tick)
(let ((table (make-hash-table :test 'org-ql-hash-test))) (let ((table (make-hash-table :test 'org-ql-hash-test)))
(puthash args (or new-result 'org-ql-nil) table) (puthash query-cache-key (or new-result 'org-ql-nil) table)
table)) table))
org-ql-cache)) org-ql-cache))
(t (puthash args (or new-result 'org-ql-nil) query-cache))) (t (puthash query-cache-key (or new-result 'org-ql-nil) query-cache)))
new-result)))) new-result))))
(cl-defun org-ql--select (&key preamble-re predicate action narrow &allow-other-keys) (cl-defun org-ql--select (&key preamble-re predicate action narrow &allow-other-keys)