From c709ed0b4436939dd7c33abcfa994b76f8b7b668 Mon Sep 17 00:00:00 2001 From: Jamie English Date: Sat, 4 Apr 2026 20:41:47 +0100 Subject: [PATCH] perf(gen_build): cache ns declarations, fix #?@ reader conditional handling Co-Authored-By: Claude Opus 4.6 (1M context) --- src/rules_clojure/gen_build.clj | 77 ++++++++++---------- src/rules_clojure/namespace/find.clj | 29 ++++---- test/rules_clojure/gen_build_test.clj | 100 +++++++++++++++++++++----- 3 files changed, 136 insertions(+), 70 deletions(-) diff --git a/src/rules_clojure/gen_build.clj b/src/rules_clojure/gen_build.clj index 3ff8a46..5c80f17 100644 --- a/src/rules_clojure/gen_build.clj +++ b/src/rules_clojure/gen_build.clj @@ -303,24 +303,37 @@ (s/def ::src-ns->label (s/map-of symbol? string?)) -(defn ->src-ns->label [{:keys [basis deps-edn-dir] :as args}] - {:pre [(-> basis map?) (-> basis :classpath map?) deps-edn-dir] - :post [(map? %)]} - (->> basis - :classpath - (map (fn [[path {:keys [path-key]}]] - (when path-key - (let [nses (->> - (concat (find/find-namespaces [(fs/path->file path)] find/clj) - (find/find-namespaces [(fs/path->file path)] find/cljs)) - distinct)] - (->> nses - (map (fn [n] - [n (-> (resolve-src-location path n) - (#(src-path->label (select-keys args [:deps-edn-dir]) %)))])) - (into {})))))) - (filter identity) - (apply merge))) +(defn build-src-index + "Scan source dirs once and return both src-ns->label and a ns-decl cache. + Returns {:src-ns->label {symbol label-str}, :ns-decl-cache {[Path platform-kw] ns-decl}}" + [{:keys [basis deps-edn-dir] :as args}] + {:pre [(-> basis map?) (-> basis :classpath map?) deps-edn-dir]} + (let [label-args (select-keys args [:deps-edn-dir]) + file-decls (into [] + (comp (filter (fn [[_ {:keys [path-key]}]] path-key)) + (mapcat (fn [[path _]] + (let [dir (fs/path->file path)] + [[path dir :clj find/clj] [path dir :cljs find/cljs]]))) + (mapcat (fn [[path dir platform find-platform]] + (map (fn [[f decl]] [path f platform decl]) + (find/find-ns-decls-with-files-in-dir dir find-platform))))) + (:classpath basis)) + ns-decl-cache + (into {} + (map (fn [[_ ^java.io.File f platform decl]] + [[(.toPath f) platform] decl])) + file-decls) + src-ns->label + (into {} + (comp (map (fn [[root _ _ decl]] + [root (parse/name-from-ns-decl decl)])) + (distinct) + (map (fn [[root n]] + [n (src-path->label label-args + (resolve-src-location root n))]))) + file-decls)] + {:src-ns->label src-ns->label + :ns-decl-cache ns-decl-cache})) (s/def ::class->jar (s/map-of symbol? fs/path?)) (s/fdef ->class->jar :args (s/cat :b ::basis) :ret ::class->jar) @@ -391,22 +404,6 @@ (when label {:deps [label]}))) -(defn get-ns-decl [^Path path platform] - (try - (with-open [rdr (java.io.PushbackReader. (io/reader (.toFile path)))] - (loop [] - (let [form (clojure.core/read {:read-cond :allow - :features #{platform} - :eof ::eof} - rdr)] - (cond - (= ::eof form) nil - (and (sequential? form) (= 'ns (first form))) form - :else (recur))))) - (catch Exception e - (throw (ex-info (str "while reading " path) {:path path - :platform platform} e))))) - (defn get-ns-meta "return any metadata attached to the namespace, or nil" [ns-decl] @@ -561,6 +558,7 @@ cljc? (some cljc-path? paths) js? (some js-path? paths) + ns-decl-cache (:ns-decl-cache args) ns-decl-platforms (->> (filter clj*-path? paths) (mapcat (fn [path] @@ -570,7 +568,8 @@ (cljc-path? path) #{:clj :cljs})] (->> file-platforms (map (fn [platform] - [(get-ns-decl path platform) platform]))))))) + [(get ns-decl-cache [path platform]) + platform]))))))) ns-decls (map first ns-decl-platforms) ns-name (-> ns-decls first second) path (first paths) @@ -783,10 +782,12 @@ deps-repo-tag: Bazel workspace repo for deps, typically `@deps` " [{:keys [deps-edn-path deps-bazel deps-repo-tag basis jar->lib aliases] :as args}] - (let [args (merge args - {:src-ns->label (->src-ns->label args) + (let [{:keys [src-ns->label ns-decl-cache]} (build-src-index args) + args (merge args + {:src-ns->label src-ns->label :dep-ns->label (->dep-ns->label args) - :jar->lib jar->lib})] + :jar->lib jar->lib + :ns-decl-cache ns-decl-cache})] (gen-source-paths- args (source-paths (select-keys args [:aliases :basis :deps-edn-dir :deps-bazel]))))) (s/fdef gen-deps-build :args (s/cat :a (s/keys :req-un [::repository-dir ::dep-ns->label ::deps-build-dir ::deps-repo-tag ::jar->lib ::lib->jar ::lib->deps ::deps-bazel]))) diff --git a/src/rules_clojure/namespace/find.clj b/src/rules_clojure/namespace/find.clj index 6dc43f2..81e1bdb 100644 --- a/src/rules_clojure/namespace/find.clj +++ b/src/rules_clojure/namespace/find.clj @@ -71,20 +71,23 @@ (->> (find-files-in-dir dir platform) (map slurp))) -(defn find-ns-decls-in-dir- +(def find-ns-decls-with-files-in-dir "Searches dir recursively for (ns ...) declarations in Clojure - source files; returns the unevaluated ns declarations." - {:added "0.2.0"} - ([dir platform] - {:pre [(string? dir)]} - (keep #(ignore-reader-exception - (file/read-file-ns-decl % (:read-opts platform))) - (find-files-in-dir (io/file dir) platform)))) - -(def find-ns-decls-in-dir (memoize (fn - [dir platform] - {:pre [(instance? java.io.File dir)]} - (find-ns-decls-in-dir- (str dir) platform)))) + source files; returns [File ns-decl] pairs." + (memoize + (fn [^File dir platform] + (vec + (keep (fn [^File file] + (when-let [decl (ignore-reader-exception + (file/read-file-ns-decl file (:read-opts platform)))] + [file decl])) + (find-files-in-dir dir platform)))))) + +(defn find-ns-decls-in-dir + "Like find-ns-decls-with-files-in-dir but returns only the ns declarations." + [^File dir platform] + {:pre [(instance? java.io.File dir)]} + (mapv second (find-ns-decls-with-files-in-dir dir platform))) ;;; Finding namespaces in JAR files diff --git a/test/rules_clojure/gen_build_test.clj b/test/rules_clojure/gen_build_test.clj index 5c9f646..d81b243 100644 --- a/test/rules_clojure/gen_build_test.clj +++ b/test/rules_clojure/gen_build_test.clj @@ -3,6 +3,8 @@ [clojure.test :refer [deftest is testing]] [clojure.java.io :as io] [rules-clojure.gen-build :as gb] + [rules-clojure.namespace.file :as ns-file] + [rules-clojure.namespace.parse :as parse] [rules-clojure.fs :as fs])) (defn- make-temp-dir @@ -17,19 +19,47 @@ (spit f content) (fs/->path (.getAbsolutePath f)))) +(defn- build-ns-decl-cache + "Build a ns-decl cache for the given paths, mimicking build-src-index." + [paths] + (into {} + (mapcat (fn [path] + (let [file (.toFile ^java.nio.file.Path path) + s (str path)] + (cond + (str/ends-with? s ".clj") + (when-let [decl (ns-file/read-file-ns-decl file parse/clj-read-opts)] + {[path :clj] decl}) + + (str/ends-with? s ".cljs") + (when-let [decl (ns-file/read-file-ns-decl file parse/cljs-read-opts)] + {[path :cljs] decl}) + + (str/ends-with? s ".cljc") + (merge + (when-let [decl (ns-file/read-file-ns-decl file parse/clj-read-opts)] + {[path :clj] decl}) + (when-let [decl (ns-file/read-file-ns-decl file parse/cljs-read-opts)] + {[path :cljs] decl})))))) + paths)) + (defn- minimal-args - "Build a minimal args map for ns-rules with the given dep-ns->label map." - [dir dep-ns->label] - (let [dir-path (fs/->path (.getAbsolutePath dir))] - {:src-ns->label {} - :dep-ns->label dep-ns->label - :jar->lib {} - :class->jar {} - :deps-repo-tag "@deps" - :deps-bazel {} - :deps-edn-dir dir-path - :basis {:paths ["src"] - :classpath {}}})) + "Build a minimal args map for ns-rules with the given dep-ns->label map. + Provide paths to build the ns-decl-cache that ns-rules requires." + ([dir dep-ns->label] + (minimal-args dir dep-ns->label [])) + ([dir dep-ns->label paths] + (let [dir-path (fs/->path (.getAbsolutePath dir))] + {:src-ns->label {} + :dep-ns->label dep-ns->label + :jar->lib {} + :class->jar {} + :deps-repo-tag "@deps" + :deps-bazel {} + :deps-edn-dir dir-path + :ns-decl-cache (build-ns-decl-cache paths) + :basis {:paths ["src"] + :classpath {}}}))) (defn- extract-deps "Given ns-rules output, extract the deps list from the first clojure_library rule." @@ -50,7 +80,7 @@ 'reagent.core "ns_reagent_reagent_reagent_core"} :cljs {'reitit.core "metosin_reitit_core" 'reagent.core "reagent_reagent"}} - args (minimal-args dir dep-ns->label) + args (minimal-args dir dep-ns->label [clj-path cljs-path]) result (gb/ns-rules args [clj-path cljs-path]) deps (extract-deps result)] (try @@ -77,7 +107,7 @@ "(ns example.shared\n (:require [clojure.string :as str]))") dep-ns->label {:clj {'clojure.string "ns_org_clojure_clojure_clojure_string"} :cljs {'clojure.string "org_clojure_clojurescript"}} - args (minimal-args dir dep-ns->label) + args (minimal-args dir dep-ns->label [cljc-path]) result (gb/ns-rules args [cljc-path]) deps (extract-deps result)] (try @@ -89,6 +119,38 @@ (finally (fs/rm-rf (.toPath dir))))))) +(deftest ns-rules-cljc-splicing-reader-conditional + (testing ".cljc with #?@ splicing reader conditional should resolve platform-specific requires" + (let [dir (make-temp-dir) + cljc-path (write-file dir "machine.cljc" + (str "(ns example.machine\n" + " (:require #?@(:clj [[clojure.core.memoize]\n" + " [example.logging :as log]]\n" + " :cljs [[taoensso.timbre :as log]])\n" + " [clojure.set :as set]))")) + dep-ns->label {:clj {'clojure.core.memoize "ns_org_clojure_core_memoize_clojure_core_memoize" + 'clojure.set "ns_org_clojure_clojure_clojure_set" + 'taoensso.timbre "ns_com_taoensso_timbre_taoensso_timbre"} + :cljs {'clojure.set "org_clojure_clojurescript" + 'taoensso.timbre "com_taoensso_timbre"}} + args (minimal-args dir dep-ns->label [cljc-path]) + result (gb/ns-rules args [cljc-path]) + deps (extract-deps result)] + (try + ;; CLJ branch: clojure.core.memoize + (is (some #(= "@deps//:ns_org_clojure_core_memoize_clojure_core_memoize" %) deps) + "CLJ splicing branch should resolve clojure.core.memoize") + ;; CLJS branch: taoensso.timbre + (is (some #(= "@deps//:com_taoensso_timbre" %) deps) + "CLJS splicing branch should resolve taoensso.timbre") + ;; Common require resolved via both platforms + (is (some #(= "@deps//:ns_org_clojure_clojure_clojure_set" %) deps) + "common require should resolve via CLJ dep map") + (is (some #(= "@deps//:org_clojure_clojurescript" %) deps) + "common require should resolve via CLJS dep map") + (finally + (fs/rm-rf (.toPath dir))))))) + (defn- find-rule "Find a rule in ns-rules output by type keyword (e.g. :clojure_binary). Returns attrs map." [rules-output type-kw] @@ -101,7 +163,7 @@ (str "(ns example.benchmark\n" " {:bazel/clojure_binary {:jvm_flags [\"-Djdk.attach.allowAttachSelf\"]}}\n" " (:gen-class))")) - args (-> (minimal-args dir {:clj {} :cljs {}}) + args (-> (minimal-args dir {:clj {} :cljs {}} [clj-path]) (assoc :deps-bazel {:clojure_library {:jvm_flags ["-Dclojure.main.report=stderr"]}})) result (gb/ns-rules args [clj-path]) attrs (find-rule result :clojure_binary)] @@ -120,7 +182,7 @@ (let [dir (make-temp-dir) clj-path (write-file dir "core.clj" "(ns example.core\n (:gen-class))") - args (minimal-args dir {:clj {} :cljs {}}) + args (minimal-args dir {:clj {} :cljs {}} [clj-path]) result (gb/ns-rules args [clj-path]) attrs (find-rule result :clojure_binary)] (try @@ -134,7 +196,7 @@ (str "(ns example.server\n" " {:bazel/clojure_binary {:main_class \"example.server\"}}\n" " (:gen-class))")) - args (minimal-args dir {:clj {} :cljs {}}) + args (minimal-args dir {:clj {} :cljs {}} [clj-path]) result (gb/ns-rules args [clj-path]) attrs (find-rule result :clojure_binary)] (try @@ -147,7 +209,7 @@ (let [dir (make-temp-dir) clj-path (write-file dir "post_test.clj" "(ns example.post-test\n {:bazel/clojure_test {:timeout :long}}\n (:require [clojure.test :refer [deftest]]))") - args (minimal-args dir {:clj {'clojure.test "org_clojure_clojure"} :cljs {}}) + args (minimal-args dir {:clj {'clojure.test "org_clojure_clojure"} :cljs {}} [clj-path]) result (gb/ns-rules args [clj-path]) attrs (find-rule result :clojure_test)] (try @@ -162,7 +224,7 @@ (let [dir (make-temp-dir) clj-path (write-file dir "large_test.clj" "(ns example.large-test\n {:bazel/clojure_test {:size :large}}\n (:require [clojure.test :refer [deftest]]))") - args (minimal-args dir {:clj {'clojure.test "org_clojure_clojure"} :cljs {}}) + args (minimal-args dir {:clj {'clojure.test "org_clojure_clojure"} :cljs {}} [clj-path]) result (gb/ns-rules args [clj-path]) attrs (find-rule result :clojure_test)] (try