Restructure zk--id-list to take fuller advantage of passed zk-alist #61
Conversation
|
I can't find any cases where 'zk--id-list' is called with zk-alist but not str. Do you have some examples of where this improves performance? There used to be several such cases, but they were all removed in 5f40cae. edit: It actually seems that the zk-alist option in 'zk--id-list' is vestigial and could be safely removed. WDYT? |
|
I started down this rabbit hole because of |
| @@ -234,7 +234,7 @@ To quickly change this setting, call `zk-desktop-add-toggle'." | |||
| ;; replace titles | |||
| (goto-char (point-min)) | |||
| (let* ((zk-alist (zk--alist)) | |||
| (ids (zk--id-list))) | |||
| (ids (zk--id-list nil zk-alist))) | |||
There was a problem hiding this comment.
But if str is nil, then zk--alist function is never called, and not used; the function just goes straight to zk--parse-file, right? Am I missing something?
There was a problem hiding this comment.
That was the point of d6be688: if zk-alist is passed to zk--id-list, no matter whether str is nil or not, the function uses the passed zk-alist rather than redoing all the work with zk--parse-file.
This is just a slight change in the logic, so nothing has changed aside from big performance improvement when ZK-ALIST is passed but STR is not.
boyechko
force-pushed
the
feat/zk--id-list-performance-boost
branch
from
1c13fb1
to
46efaaf
Compare
A lot of these are very difficult to benchmark, but considering the following benchmarks, there is potential to considerably speed up various zk-desktop and zk-index commands. The change to `zk-index-query-files` will likely result in greatest benefit, since we go from running `zk--alist` three times to running it once. `zk--parse-id` did not pass on its zk-alist to `zk--id-list` either. ``` === bm/zk--id-list+use-zk-alist (10 reps) at 2023-07-16 15:26 === ((zk--id-list/orig)) => 3049 results in 0.97 sec (inc. 0.56 sec for 0 GCs) ((zk--id-list)) => 3049 results in 0.95 sec (inc. 0.56 sec for 0 GCs) ((zk--id-list/orig nil zk-alist)) => 3049 results in 0.98 sec (inc. 0.58 sec for 0 GCs) ((zk--id-list nil zk-alist)) => 3049 results in 0.03 sec (inc. 0.02 sec for 0 GCs) ```
|
A lot of the commands are interactive and difficult to benchmark, but considering the following benchmarks, there is potential to considerably speed up various zk-desktop and zk-index commands. The change to |
This is just a slight change in the logic, so nothing has changed aside from big performance improvement when ZK-ALIST is passed but STR is not.