zk icon indicating copy to clipboard operation
zk copied to clipboard

Restructure zk--id-list to take fuller advantage of passed zk-alist

Open boyechko opened this issue 2 years ago • 3 comments

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 avatar Jun 28 '23 16:06 boyechko

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 5f40caea8cbe09d93c6f8f6eca33a0dbc241e5a2.

edit: It actually seems that the zk-alist option in 'zk--id-list' is vestigial and could be safely removed. WDYT?

localauthor avatar Jun 28 '23 19:06 localauthor

I started down this rabbit hole because of zk-desktop-make-buttons, which ought to be calling zk--id-list with ZK-ALIST but not STR (another commit to this pull request forthcoming). There is also potential to take advantage of this in zk-index-query-files, though that might be a separate PR down the line.

boyechko avatar Jun 28 '23 21:06 boyechko

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 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, so that would add further performance enhancements.

=== 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)

boyechko avatar Jul 16 '23 23:07 boyechko