Skip to content

perf(gen_build): cache ns declarations, fix #?@ reader conditional handling #93

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
english wants to merge 1 commit into
base: main
Choose a base branch
from
Open

perf(gen_build): cache ns declarations, fix #?@ reader conditional handling #93

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 39 additions & 38 deletions src/rules_clojure/gen_build.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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]
Expand All @@ -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)
Expand Down Expand Up @@ -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])))
Expand Down
29 changes: 16 additions & 13 deletions src/rules_clojure/namespace/find.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
100 changes: 81 additions & 19 deletions test/rules_clojure/gen_build_test.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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."
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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]
Expand All @@ -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)]
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down