Speed up analysis of gitlibs using git sha, resolve protocol methods (#509)

This significantly speeds up analysis for gitlibs by using the git sha as a hash (thus treating them as immutable) instead of handling them on a per-form level.

Also resolve protocol methods to the defining protocol, which would previously not be detected.

Lastly drop the location cache which is no longer needed.

Author: mk

CHANGELOG.md

@@ -18,6 +18,14 @@ Changes can be:
 
     Use these features to build a new welcome page that gives more useful information, including links to potential notebooks in the project.
 
+* ⚡️ Speed up analysis of gitlibs using git sha, resolve protocol methods
+
+    This significantly speeds up analysis for gitlibs by using the git sha as a hash (thus treating them as immutable) instead of handling them on a per-form level.
+
+    Also resolve protocol methods to the defining protocol, which would previously not be detected.
+
+    Lastly drop the location cache which is no longer needed.
+
 * 🍕 `clerk/fragment` for splicing a seq of values into the document as if it were produced by results of individual cells. Useful when programmatically generating content.
 
 * 🚨 Change `nextjournal.clerk.render/clerk-eval` to not recompute the currently shown document when using the 1-arity version. Added a second arity that takes an opts map with a `:recompute?` key.

notebooks/emmy.clj

@@ -1,33 +1,39 @@
-(ns emmy-repro
+(ns emmy
   (:require [emmy.env :as e]
             [emmy.mechanics.lagrange]))
 
-;; ## BUG 1:
+;; `emmy.env` is importing vars via potemkin. This notebook exists to
+;; test that the analysis finds the correct locations for imported
+;; vars.
 
-;; This notebook takes close to 2 seconds to evaluate:
-
-;; Clerk evaluated '/Users/sritchie/code/clj/clerk-demo/notebooks/emmy_repro.clj' in 1853.674042ms.
-
-;; For the final Langrangian in generalized coordinates (the angles of each
-;; segment) by composing `L-rect` with a properly transformed `angles->rect`
-;; coordinate transform!
-
-;; ## BUG 2:
-;;
-;; The following form:
-
-#_
 (let [L (emmy.mechanics.lagrange/L-pendulum 'g 'm 'l)]
   (((e/Lagrange-equations L)
     (e/literal-function 'theta_1))
    't))
 
-;; Evaluates to this:
-(e/literal-number
- '(- (* 1/2 m 2 l (((expt D 2) theta_1) t) l) (* g m l (- (sin (theta_1 t))))))
+(comment
+  (require '[nextjournal.clerk.analyzer :as analyzer]
+           '[clj-async-profiler.core :as prof])
+
+  
+  (analyzer/unhashed-deps (:->analysis-info @nextjournal.clerk.webserver/!doc))
+
+  (analyzer/find-location 'emmy.util.stopwatch/repr)
+  (analyzer/find-location 'emmy.value/zero?)
+  (meta #'emmy.value/zero?)
+  
+  (meta #'emmy.value/Value)
+
+  (def parsed
+    (nextjournal.clerk.parser/parse-file "notebooks/emmy.clj"))
 
+  (def analyzed
+    (analyzer/analyze-doc parsed))
+  
+  (time (do
+          (prof/profile (dotimes [i 10] (analyzer/build-graph analyzed)))
+          :done))
 
-;; But if I include it in a notebook, I get this:
+  )
 
-;; Execution error (NullPointerException) at clojure.tools.analyzer.jvm.utils/members* (utils.clj:272).
-;; Cannot invoke "java.lang.Class.getName()" because the return value of "clojure.lang.IFn.invoke(Object)" is null
+#_(nextjournal.clerk/clear-cache!)

notebooks/profile.clj

@@ -10,6 +10,8 @@
 (def parsed
   (parser/parse-file "notebooks/rule_30.clj"))
 
+
+
 (def analyzed
   (analyzer/analyze-doc parsed))
 
@@ -19,6 +21,8 @@
 
 
 
+(prof/profile (analyzer/build-graph analyzed))
+
 (prof/profile (analyzer/build-graph analyzed))
 
 (if-not @prof.ui/current-server

src/nextjournal/clerk.clj

@@ -526,8 +526,6 @@
   * the form of an anonymous expression"
   ([]
    (swap! webserver/!doc dissoc :blob->result)
-   (reset! analyzer/!file->analysis-cache {})
-   (reset! analyzer/!ns->loc-cache {})
    (if (fs/exists? config/cache-dir)
      (do (fs/delete-tree config/cache-dir)
          (prn :cache-dir/deleted config/cache-dir))

src/nextjournal/clerk/analyzer.clj

@@ -145,6 +145,10 @@
                          (form->ex-data form)
                          e)))))))
 
+(defn ^:private var->protocol [v]
+  (or (:protocol (meta v))
+      v))
+
 (defn analyze [form]
   (let [!deps      (atom #{})
         mexpander (fn [form env]
@@ -173,7 +177,7 @@
                                        (when-not (= op :the-var)
                                          (list `deref (symbol var))))))
                          nodes)
-        deps (set/union (set/difference (into #{} (map symbol) @!deps) vars)
+        deps (set/union (set/difference (into #{} (map (comp symbol var->protocol)) @!deps) vars)
                         deref-deps
                         (class-deps analyzed)
                         (when (var? form) #{(symbol form)}))
@@ -227,7 +231,6 @@
 #_(type (first (:deps (analyze #'analyze-form))))
 #_(-> (analyze '(proxy [clojure.lang.ISeq] [])))
 
-
 (defn- circular-dependency-error? [e]
   (-> e ex-data :reason #{::dep/circular-dependency}))
 
@@ -371,12 +374,12 @@
 (defn analyze-file
   ([file]
    (let [current-file-sha (sha1-base58 (fs/read-all-bytes file))]
-            (or (when-let [{:as cached-analysis :keys [file-sha]} (@!file->analysis-cache file)]
-                  (when (= file-sha current-file-sha)
-                    cached-analysis))
-                (let [analysis (analyze-doc {:file-sha current-file-sha} (parser/parse-file {} file))]
-                  (swap! !file->analysis-cache assoc file analysis)
-                  analysis))))
+     (or (when-let [{:as cached-analysis :keys [file-sha]} (@!file->analysis-cache file)]
+           (when (= file-sha current-file-sha)
+             cached-analysis))
+         (let [analysis (analyze-doc {:file-sha current-file-sha} (parser/parse-file {} file))]
+           (swap! !file->analysis-cache assoc file analysis)
+           analysis))))
   ([state file]
    (analyze-doc state (parser/parse-file {} file))))
 
@@ -470,8 +473,8 @@
               (ns->jar ns))
           (symbol->jar sym)))))
 
-
 #_(find-location `inc)
+#_(find-location `*print-dup*)
 #_(find-location '@nextjournal.clerk.webserver/!doc)
 #_(find-location `dep/depend)
 #_(find-location 'java.util.UUID)
@@ -480,25 +483,6 @@
 #_(find-location 'io.methvin.watcher.hashing.FileHasher/DEFAULT_FILE_HASHER)
 #_(find-location 'String)
 
-(defn find-location+cache [!ns->loc sym]
-  (if (deref? sym)
-    (find-location+cache !ns->loc (second sym))
-    (if-let [ns-sym (and (qualified-symbol? sym) (-> sym namespace symbol))]
-      (or (@!ns->loc ns-sym)
-          (when-let [loc (find-location sym)]
-            (swap! !ns->loc assoc ns-sym loc)
-            loc))
-      (find-location sym))))
-
-(def !ns->loc-cache (atom {}))
-
-#_(reset! !ns->loc-cache {})
-
-(defn find-location-cached [sym]
-  (find-location+cache !ns->loc-cache sym))
-
-#_(find-location-cached `inc)
-
 (def hash-jar
   (memoize (fn [f]
              {:jar f :hash (sha1-base58 (io/input-stream f))})))
@@ -530,22 +514,24 @@
              (assoc :counter 0))]
     (let [unhashed (unhashed-deps ->analysis-info)
           loc->syms (apply dissoc
-                           (group-by find-location-cached unhashed)
+                           (group-by find-location unhashed)
                            analyzed-file-set)]
       (if (and (seq loc->syms) (< counter 10))
         (recur (-> (reduce (fn [g [source symbols]]
-                             (if (or (nil? source)
-                                     (str/ends-with? source ".jar"))
-                               (update g :->analysis-info merge (into {} (map (juxt identity (constantly (if source (hash-jar source) {})))) symbols))
-                               (-> g
-                                   (update :analyzed-file-set conj source)
-                                   (merge-analysis-info (analyze-file source)))))
+                             (let [jar? (or (nil? source)
+                                            (str/ends-with? source ".jar"))
+                                   gitlib-hash (and (not jar?)
+                                                    (second (re-find #".gitlibs/libs/.*/(\b[0-9a-f]{5,40}\b)/" (fs/unixify source))))]
+                               (if (or jar? gitlib-hash)
+                                 (update g :->analysis-info merge (into {} (map (juxt identity (constantly (if source (or gitlib-hash (hash-jar source)) {})))) symbols))
+                                 (-> g
+                                     (update :analyzed-file-set conj source)
+                                     (merge-analysis-info (analyze-file source))))))
                            state
                            loc->syms)
                    (update :counter inc)))
         (dissoc state :analyzed-file-set :counter)))))
 
-
 (comment
   (def parsed (parser/parse-file {:doc? true} "src/nextjournal/clerk/webserver.clj"))
   (def analysis (time (-> parsed analyze-doc build-graph)))

test/nextjournal/clerk/analyzer_test.clj

@@ -75,7 +75,11 @@
     (is (every? symbol? (:deps (ana/analyze '(.hashCode clojure.lang.Compiler)))))
 
     (is (every? symbol? (:deps (ana/analyze '(defprotocol MyProtocol
-                                               (-check [_]))))))))
+                                               (-check [_])))))))
+
+  (testing "protocol methods are resolved to protocol in deps"
+    (is (= '#{nextjournal.clerk.analyzer/BoundedCountCheck}
+           (:deps (ana/analyze 'nextjournal.clerk.analyzer/-exceeds-bounded-count-limit?))))))
 
 (deftest read-string-tests
   (testing "read-string should read regex's such that value equalility is preserved"
@@ -194,16 +198,6 @@
   (testing "weavejester.dependency/graph"
     (is (re-find #"dependency-.*\.jar" (ana/find-location 'weavejester.dependency/graph)))))
 
-(deftest find-location+cache
-  (let [!ns->loc (atom {})]
-    (testing "populates the cache"
-      (ana/find-location+cache !ns->loc 'clojure.core/inc)
-      (is (= (@!ns->loc 'clojure.core) (ana/find-location 'clojure.core/inc))))
-
-    (testing "doesn't call `find-location` with cache populated"
-      (with-redefs [ana/find-location (fn [_] :bogus/value)]
-        (is (re-find #"clojure-1\..*\.jar" (ana/find-location+cache !ns->loc 'clojure.core/inc)))))))
-
 (defn analyze-string [s]
   (-> (parser/parse-clojure-string {:doc? true} s)
       ana/analyze-doc