Repository: clojure-lsp/clojure-lsp
Pull request #243 (2021-01-09 16:43:47)

Performance improvement
* Fix the performance regression introduced after #211, when crawling source/jars.
* Some minor performance improvements following https://tech.redplanetlabs.com/2020/09/02/clojure-faster/

Before the cljc duplicated usages fix:
```clojure
Evaluation count : 6 in 6 samples of 1 calls.
             Execution time mean : 847.996168 ms
    Execution time std-deviation : 42.774901 ms
   Execution time lower quantile : 801.961730 ms ( 2.5%)
   Execution time upper quantile : 913.601543 ms (97.5%)
                   Overhead used : 1.947168 ns
```

[Before](https://github.com/clojure-lsp/clojure-lsp/blob/master/src/clojure_lsp/parser.clj#L1131-L1157) the performance fix (after this PR).
```clojure
Evaluation count : 6 in 6 samples of 1 calls.
             Execution time mean : 3.575516 sec
    Execution time std-deviation : 224.909583 ms
   Execution time lower quantile : 3.304419 sec ( 2.5%)
   Execution time upper quantile : 3.841209 sec (97.5%)
                   Overhead used : 1.947168 ns
```

[With the performance fix](https://github.com/clojure-lsp/clojure-lsp/pull/243)
```clojure
             Execution time mean : 877.753781 ms
    Execution time std-deviation : 12.686530 ms
   Execution time lower quantile : 857.340680 ms ( 2.5%)
   Execution time upper quantile : 890.014269 ms (97.5%)
                   Overhead used : 1.947168 ns
```

This function is used every time we need to crawl a code in a file to parse and understand the symbols, since we could have a lot of file for multiple jars/source-dirs, this should improve the performance as it was before :)

note: I did the performance tests using [criterium](https://github.com/hugoduncan/criterium) benchmark tool.


Patch:
diff --git a/src/clojure_lsp/crawler.clj b/src/clojure_lsp/crawler.clj
index 5bd76f8a..116e61eb 100644
--- a/src/clojure_lsp/crawler.clj
+++ b/src/clojure_lsp/crawler.clj
@@ -174,7 +174,7 @@
                                            reverse
                                            (map io/file)
                                            (map #(vector (get-cp-entry-type %) %))
-                                           (group-by first)
+                                           (group-by (fn [classpath] (nth classpath 0 nil)))
                                            (reduce-kv (fn [m k v]
                                                         (assoc m k (map second v))) {}))
             jars (:file classpath-entries-by-type)
diff --git a/src/clojure_lsp/db.clj b/src/clojure_lsp/db.clj
index 3c60e0ec..d921003f 100644
--- a/src/clojure_lsp/db.clj
+++ b/src/clojure_lsp/db.clj
@@ -32,12 +32,12 @@
   (try
     (with-open [conn (jdbc/get-connection (make-spec project-root))]
       (let [project-row
-            (->> (jdbc/execute! conn
+            (-> (jdbc/execute! conn
                                 ["select root, hash, classpath, jar_envs from project where root = ? and version = ?"
                                  (str project-root)
                                  (str version)]
                                 {:builder-fn rs/as-unqualified-lower-maps})
-                 (first))]
+                 (nth 0))]
         {:jar-envs (edn/read-string (:jar_envs project-row))
          :classpath (edn/read-string (:classpath project-row))
          :project-hash (:hash project-row)}))
diff --git a/src/clojure_lsp/feature/code_actions.clj b/src/clojure_lsp/feature/code_actions.clj
index 3898787f..e39adb95 100644
--- a/src/clojure_lsp/feature/code_actions.clj
+++ b/src/clojure_lsp/feature/code_actions.clj
@@ -14,7 +14,7 @@
         line (dec row)
         character (dec col)
         has-unknown-ns? (some #(= (compare "unresolved-namespace" (some-> % .getCode .get)) 0) diagnostics)
-        unresolved-symbol (first (filter #(= (compare "unresolved-symbol" (some-> % .getCode .get)) 0) diagnostics))
+        unresolved-symbol (some #(= (compare "unresolved-symbol" (some-> % .getCode .get)) 0) diagnostics)
         known-refer? (when unresolved-symbol
                        (get r.transform/common-refers->info (z/sexpr zloc)))
         missing-require (when (or has-unknown-ns? known-refer?)
diff --git a/src/clojure_lsp/feature/references.clj b/src/clojure_lsp/feature/references.clj
index 5582b153..16a27ca7 100644
--- a/src/clojure_lsp/feature/references.clj
+++ b/src/clojure_lsp/feature/references.clj
@@ -1,11 +1,12 @@
 (ns clojure-lsp.feature.references
-  (:require [clojure-lsp.shared :as shared]
-            [clojure.set :as set]
-            [clojure-lsp.db :as db]
-            [clojure-lsp.parser :as parser]
-            [clojure-lsp.feature.diagnostics :as f.diagnostic]
-            [clojure.core.async :as async]
-            [clojure.tools.logging :as log]))
+  (:require
+   [clojure-lsp.db :as db]
+   [clojure-lsp.feature.diagnostics :as f.diagnostic]
+   [clojure-lsp.parser :as parser]
+   [clojure-lsp.shared :as shared]
+   [clojure.core.async :as async]
+   [clojure.set :as set]
+   [clojure.tools.logging :as log]))
 
 (defn find-after-cursor [line column env file-type]
   (let [file-types (if (= :cljc file-type)
@@ -21,12 +22,12 @@
   (let [file-types (if (= :cljc file-type)
                      #{:clj :cljs}
                      #{file-type})]
-    (->> env
-         (filter (comp #(set/subset? % file-types) :file-type))
-         (filter (comp #{:within} (partial shared/check-bounds line column)))
-         ;; Pushes keywords last
-         (sort-by (comp keyword? :sym))
-         (first))))
+    (-> (->> env
+             (filter (comp #(set/subset? % file-types) :file-type))
+             (filter (comp #{:within} (partial shared/check-bounds line column)))
+           ;; Pushes keywords last
+             (sort-by (comp keyword? :sym)))
+        (nth 0 nil))))
 
 (defn reference-usages [doc-id line column]
   (let [file-envs (:file-envs @db/db)
@@ -49,7 +50,8 @@
            macro-defs (get-in @db/db [:settings :macro-defs])
            excluded-unused-ns-declarations (get-in @db/db [:settings :linters :unused-namespace-declarations] #{})
            references (cond->> (parser/find-usages uri text file-type macro-defs)
-                        remove-private? (filter (fn [{:keys [tags]}] (and (:public tags) (:declare tags)))))]
+                        remove-private? (filter (comp #(and (:public %)
+                                                            (:declare %)) :tags)))]
        (when diagnose?
          (async/put! db/diagnostics-chan
                      {:uri uri
diff --git a/src/clojure_lsp/parser.clj b/src/clojure_lsp/parser.clj
index b071d040..b7703587 100644
--- a/src/clojure_lsp/parser.clj
+++ b/src/clojure_lsp/parser.clj
@@ -573,12 +573,12 @@
 (defn- arglists-to-signatures
   [arglists]
   (cond
-    (= 'quote (first arglists))
+    (= 'quote (nth arglists 0 nil))
     (let [sexprs (eval arglists)]
       {:sexprs (seq sexprs)
        :strings (map str sexprs)})
 
-    (string? (first arglists))
+    (string? (nth arglists 0 nil))
     {:sexprs (map (comp z/sexpr z/of-string) arglists)
      :strings arglists}
 
@@ -1128,33 +1128,20 @@
         (recur (z/next loc))
         (vary-meta (z/skip z/prev #(z/prev %) loc) assoc ::zm/end? false)))))
 
-(defn ^:private same-usage?
-  [a-usage b-usage]
-  (and (= (:tags a-usage) (:tags b-usage))
-       (= (:sym a-usage) (:sym b-usage))
-       (= (:row a-usage) (:row b-usage))
-       (= (:col a-usage) (:col b-usage))
-       (= (:end-row a-usage) (:end-row b-usage))
-       (= (:end-col a-usage) (:end-col b-usage))))
-
 (defn ^:private merge-by-file-types
-  "Merge usages by file-type checking clj and cljs same usages."
+  "Merge usages by file-type to avoid duplicated usages for cljc files."
   [usages]
-  (let [usages-by-file-type (group-by :file-type usages)]
-    (->> usages
-         (map (fn [{:keys [tags file-type] :as usage}]
-                (let [inverse-file-type (if (= file-type :cljs) :clj :cljs)]
-                  (cond
-                    (and tags (contains? tags :norename))
-                    (assoc usage :file-type #{file-type})
-
-                    (not-any? #(same-usage? usage %) (inverse-file-type usages-by-file-type))
-                    (assoc usage :file-type #{file-type})
-
-                    (= file-type :clj)
-                    (assoc usage :file-type #{:clj :cljs})))))
-         (remove nil?)
-         vec)))
+  (->> usages
+       (group-by (juxt :sym :tags :row :col :end-row :end-col))
+       (map (fn [[_k v]]
+              (->> (map #(assoc % :file-type #{(:file-type %)}) v)
+                   (reduce (fn [a b]
+                             (assoc b :file-type (set/union (:file-type a)
+                                                            (:file-type b))))
+                           {})
+                   )))
+       (sort-by (juxt :row :col :end-row :end-col count))
+       vec))
 
 (defn ^:private find-raw-usages
   [code-loc file-type client-macro-defs uri]


Test patch:
diff --git a/test/clojure_lsp/handlers_test.clj b/test/clojure_lsp/handlers_test.clj
index 7a3c905d..b932f700 100644
--- a/test/clojure_lsp/handlers_test.clj
+++ b/test/clojure_lsp/handlers_test.clj
@@ -391,6 +391,7 @@
               {:label "bazz" :detail "alpaca.ns/bazz" :data "alpaca.ns/bazz"}]
              (handlers/completion "file://e.clj" 1 38))))
     (testing "complete-core-stuff"
+      (get-in @db/db [:file-envs "file://b.clj"])
       (reset! db/db (update-in db-state [:file-envs "file://b.clj" 4] merge {:sym 'freq :str "freq"}))
       (is (= [{:label "frequencies" :data "clojure.core/frequencies"}]
              (handlers/completion "file://b.clj" 3 18)))
@@ -545,7 +546,8 @@
                 :data ["file://a.clj" 3 8]}
                {:range
                 {:start {:line 4 :character 6} :end {:line 4 :character 9}}
-                :data ["file://a.clj" 5 7]})
+                :data ["file://a.clj" 5 7]}
+               )
              (handlers/code-lens "file://a.clj"))))))
 
 (deftest test-code-lens-resolve
diff --git a/test/clojure_lsp/parser_test.clj b/test/clojure_lsp/parser_test.clj
index e6e8389a..408729ec 100644
--- a/test/clojure_lsp/parser_test.clj
+++ b/test/clojure_lsp/parser_test.clj
@@ -213,21 +213,21 @@
 
 (deftest find-references-destructuring-test
   (let [code "(let [{:keys [a] b :b} {}] a b)"
-        [_ _a-kw a-param _b-kw b-param a-usage b-usage :as usages] (parser/find-usages code :clj {})]
+        [_ _a-kw a-param b-param _b-kw a-usage b-usage :as usages] (parser/find-usages code :clj {})]
     (is (= 7 (count usages)))
     (is (= #{:declare :param} (:tags a-param)))
     (is (= (namespace (:sym a-usage)) (namespace (:sym a-param))))
     (is (= #{:declare :param} (:tags b-param)))
     (is (= (namespace (:sym b-usage)) (namespace (:sym b-param)))))
   (let [code "(fn [y {:keys [a] b :b}] a b)"
-        [_ _ _a-kw a-param _b-kw b-param a-usage b-usage :as usages] (parser/find-usages code :clj {})]
+        [_ _ _a-kw a-param b-param _b-kw a-usage b-usage :as usages] (parser/find-usages code :clj {})]
     (is (= 8 (count usages)))
     (is (= #{:declare :param} (:tags a-param)))
     (is (= (namespace (:sym a-param)) (namespace (:sym a-usage))))
     (is (= #{:declare :param} (:tags b-param)))
     (is (= (namespace (:sym b-param)) (namespace (:sym b-usage)))))
   (let [code "(fn myname [y {:keys [a] b :b}] a b)"
-        [_ name-ref _ _a-kw a-param _b-kw b-param a-usage b-usage :as usages] (parser/find-usages code :clj {})]
+        [_ name-ref _ _a-kw a-param b-param _b-kw a-usage b-usage :as usages] (parser/find-usages code :clj {})]
     (is (= 9 (count usages)))
     (is (not= "user" (namespace (:sym name-ref))))
     (is (= #{:declare} (:tags name-ref)))
@@ -337,8 +337,8 @@
           usages (parser/find-usages code :cljc {})]
       (is (= 3 (count usages)))
       (is (= 'clojure.core/ns (:sym (nth usages 0 nil))))
-      (is (= 'cljs.core/ns (:sym (nth usages 2 nil))))
-      (is (= "hi" (:str (nth usages 1 nil))))))
+      (is (= 'cljs.core/ns (:sym (nth usages 1 nil))))
+      (is (= "hi" (:str (nth usages 2 nil))))))
   (testing "cljs and clj"
     (let [code       "#?(:cljs x) #?(:clj y) #?@(:clj [a b] :cljs [c d])"
           usages     (parser/find-usages code :cljc {})
@@ -354,7 +354,7 @@
           usages (parser/find-usages code :cljc {})]
       (is (= 6 (count usages)))
       (is (= ["def" "hello" "comment" "hello"] (map :str (filter (comp #(contains? % :clj) :file-type) usages))))
-      (is (= ["hello" "hello" "def" "comment"] (map :str (filter (comp #(contains? % :cljs) :file-type) usages)))))))
+      (is (= ["def""hello" "comment" "hello"] (map :str (filter (comp #(contains? % :cljs) :file-type) usages)))))))
 
 (deftest if-let-test
   (let [code "(if-let [] a b)"
@@ -407,7 +407,7 @@
     (let [code "(def GET) (GET \"/my-route/:id\" {{:keys [id]} :params} id)"
           usages (parser/find-usages code :clj {'user/GET [:_ :params :bound-elements]})]
       (is (= 7 (count usages)))
-      (is (= (:sym (nth usages 5 nil)) (:sym (nth usages 6 nil))))))
+      (is (= (:sym (nth usages 4 nil)) (:sym (nth usages 6 nil))))))
   (testing "for-like"
     (let [code "(def dofor) (dofor [a [1 2 3] :let [b a]] b)"
           usages (parser/find-usages code :clj {'user/dofor [:bindings :bound-elements]})]
@@ -584,7 +584,7 @@
   (testing "destructures param"
     (let [code "(ns user (:require [schema.core :as s])) (s/defn a :- A \"Docs\" [{b :b} :- Long c :- [S/Str]] b)"
           usages (parser/find-usages code :clj {})
-          [_ u _ s _ a _ _ _ b _ c _ b2] usages]
+          [_ u _ s _ a _ _ b _ _ c _ b2] usages]
       (is (= #{:declare :public} (:tags a)))
       (is (= 'user/a (:sym a)))
       (is (= "Docs" (:doc a)))
@@ -600,7 +600,7 @@
   (testing "handles complex return type"
     (let [code "(ns user (:require [schema.core :as s])) (s/defn a :- [A] \"Docs\" [{b :b} :- Long c :- [S/Str]] b)"
           usages (parser/find-usages code :clj {})
-          [_ u _ s _ a _ _ _ b _ c _ b2] usages]
+          [_ u _ s _ a _ _ b _ _ c _ b2] usages]
       (is (= #{:declare :public} (:tags a)))
       (is (= 'user/a (:sym a)))
       (is (= "Docs" (:doc a)))