Skip to content

Improve clojure-ts--node-child-skip-metadata performance by ~6x#132

Merged
bbatsov merged 3 commits into
clojure-emacs:mainfrom
kommen:node-child-skip-metadata-perf
Mar 17, 2026
Merged

Improve clojure-ts--node-child-skip-metadata performance by ~6x#132
bbatsov merged 3 commits into
clojure-emacs:mainfrom
kommen:node-child-skip-metadata-perf

Conversation

@kommen
Copy link
Copy Markdown
Contributor

@kommen kommen commented Mar 16, 2026

I noticed a severe lag when invoking imenu in larger clojure namespaces with clojure-ts-mode.
Traced it down to an inefficeny in clojure-ts--node-child-skip-metadata. With this fix invoking imenu gets very snappy. As this function is also used by indendation and font-lock, it should also improve the performance there as well.

While at it, I added correctness tests for it. I found the docstring a bit confusing, I hope I didn't missunderstand it?

Using this to benchmark shows a 6.5x improvement on my machine:

(let ((old-impl (lambda (node n)
                    (let ((value-nodes
                           (seq-filter
                            (lambda (child)
                              (string= (treesit-node-field-name child) "value"))
                            (treesit-node-children node t))))
                      (seq-elt value-nodes n))))
        (iterations 500))
    (with-clojure-ts-buffer "(defn ^:private foo [x] x)"
      (let* ((node (clojure-ts-test--first-list-node))
             (bench (lambda (fn)
                      (garbage-collect)
                      (let ((start (float-time)))
                        (dotimes (_ iterations)
                          (funcall fn node 0)
                          (funcall fn node 1))
                        (- (float-time) start))))
             (median (lambda (times)
                       (let ((sorted (sort (copy-sequence times) #'<)))
                         (nth (/ (length sorted) 2) sorted)))))
        ;; Warm up
        (funcall bench old-impl)
        (funcall bench #'clojure-ts--node-child-skip-metadata)
        ;; Collect samples
        (let* ((old-times (mapcar (lambda (_) (funcall bench old-impl))
                                  (number-sequence 1 7)))
               (new-times (mapcar (lambda (_) (funcall bench #'clojure-ts--node-child-skip-metadata))
                                  (number-sequence 1 7)))
               (old-median (funcall median old-times))
               (new-median (funcall median new-times)))
          (message (concat "node-child-skip-metadata benchmark (%d iterations):\n"
                           "  old: median=%.3fs samples=%s\n"
                           "  new: median=%.3fs samples=%s\n"
                           "  speedup: %.1fx")
                   iterations
                   old-median (mapcar (lambda (t) (format "%.3f" t)) old-times)
                   new-median (mapcar (lambda (t) (format "%.3f" t)) new-times)
                   (/ old-median new-median))))))

Before submitting a PR mark the checkboxes for the items you've done (if you
think a checkbox does not apply, then leave it unchecked):

  • The commits are consistent with our contribution guidelines.
  • You've added tests (if possible) to cover your change(s). Bugfix, indentation, and font-lock tests are extremely important!
  • You've run M-x checkdoc and fixed any warnings in the code you've written.
  • You've updated the changelog (if adding/changing user-visible functionality).
  • You've updated the readme (if adding/changing user-visible functionality).

Thanks!

Comment thread clojure-ts-mode.el
@bbatsov bbatsov merged commit 54fda49 into clojure-emacs:main Mar 17, 2026
5 checks passed
@bbatsov
Copy link
Copy Markdown
Member

bbatsov commented Mar 17, 2026

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants