Skip to content

Commit

Permalink
Optimize namespace-aliases
Browse files Browse the repository at this point in the history
Parallelizes it, and shrinks the corpus by removing non source/test dirs from the classpath.

Testing it over refactor-nrepl itself, I got a performance gain from 70ms (cached case) to 20ms (cached case).

The performance gains can be larger as projects scale. It also can plausibly ameliorate the mistery slowness for the :cljs branch.
  • Loading branch information
vemv committed Sep 17, 2021
1 parent eb77344 commit 1399da2
Show file tree
Hide file tree
Showing 7 changed files with 167 additions and 58 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
* This increases the chances that a namespace will be found, which in turns makes refactor-nrepl more complete/accurate.
* Replace Cheshire with `clojure.data.json`
* Build ASTs more robustly (by using locks, `require`, and ruling out certain namespaces like refactor-nrepl itself)
* Improve `namespace-aliases` performance and make it return more accurate results.
* Honor internal `future-cancel` calls, improving overall responsiveness and stability.

### Bugs fixed
Expand Down
90 changes: 60 additions & 30 deletions src/refactor_nrepl/core.clj
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
(ns refactor-nrepl.core
(:require [clojure.java.io :as io]
[clojure.string :as str]
[clojure.tools.namespace.parse :as parse]
[clojure.tools.reader.reader-types :as readers]
[orchard.java.classpath :as cp]
[orchard.misc :as misc]
[me.raynes.fs :as fs]
[refactor-nrepl.util :as util :refer [normalize-to-unix-path]]
[refactor-nrepl.s-expressions :as sexp]
[refactor-nrepl.config :as config])
(:import [java.io File FileReader PushbackReader StringReader]))
(:require
[clojure.java.io :as io]
[clojure.string :as str]
[clojure.tools.namespace.parse :as parse]
[clojure.tools.reader.reader-types :as readers]
[me.raynes.fs :as fs]
[orchard.java.classpath :as cp]
[orchard.misc :as misc]
[refactor-nrepl.config :as config]
[refactor-nrepl.s-expressions :as sexp]
[refactor-nrepl.util :as util :refer [normalize-to-unix-path]])
(:import
(java.io File FileReader PushbackReader StringReader)))

(defn version []
(let [v (-> (or (io/resource "refactor-nrepl/refactor-nrepl/project.clj")
Expand Down Expand Up @@ -57,6 +59,18 @@
(when (.isDirectory ^File f) f)))
(remove (comp ignore-dir-on-classpath? str))))

(defn source-dirs-on-classpath
"Like `#'dirs-on-classpath`, but restricted to dirs that look like
(interesting) source/test dirs."
[]
(->> (dirs-on-classpath)
(remove (fn [^File f]
(let [s (-> f .toString)]
(or (-> s (.contains "resources"))
(-> s (.contains "target"))
(-> s (.contains ".gitlibs"))))))
(remove util/dir-outside-root-dir?)))

(defn project-root
"Return the project root directory.
Expand Down Expand Up @@ -141,34 +155,32 @@
{:read-cond :allow :features #{dialect}})
(catch Exception _ nil))))))

(defn- data-file?
"True of f is named like a clj file but represents data.
E.g. true for data_readers.clj"
[path-or-file]
(let [path (.getPath (io/file path-or-file))
data-files #{"data_readers.clj" "project.clj" "boot.clj"}]
(reduce (fn [acc data-file] (or acc (.endsWith path data-file)))
false
data-files)))
(defn cljc-extension? [^String path]
(.endsWith path ".cljc"))

(defn cljc-file?
[path-or-file]
(let [path (.getPath (io/file path-or-file))]
(and (.endsWith path ".cljc")
(and (cljc-extension? path)
(read-ns-form path))))

(defn cljs-extension? [^String path]
(.endsWith path ".cljs"))

(defn cljs-file?
[path-or-file]
(let [path (.getPath (io/file path-or-file))]
(and (.endsWith path ".cljs")
(and (cljs-extension? path)
(read-ns-form path))))

(defn clj-extension? [^String path]
(.endsWith path ".clj"))

(defn clj-file?
[path-or-file]
(let [path (.getPath (io/file path-or-file))]
(and (not (data-file? path-or-file))
(.endsWith path ".clj")
(and (not (util/data-file? path-or-file))
(clj-extension? path)
(read-ns-form path))))

(defn source-file?
Expand All @@ -194,11 +206,29 @@

(defn find-in-project
"Return the files in the project satisfying (pred ^File file)."
[pred]
(->> (dirs-on-classpath)
(pmap (partial find-in-dir pred))
(apply concat)
distinct))
([pred]
(find-in-project pred (dirs-on-classpath)))
([pred dirs]
(->> dirs
(pmap (partial find-in-dir pred))
(apply concat)
distinct)))

(defn source-files-with-clj-like-extension
"Finds files with .clj* extension in the project, without inspecting them.
Meant as a particularly fast operation (as it doesn't slurp files)."
([ignore-errors?]
(source-files-with-clj-like-extension ignore-errors? (source-dirs-on-classpath)))
([ignore-errors? dirs]
(find-in-project (util/with-suppressed-errors
(comp (some-fn clj-extension?
cljc-extension?
cljs-extension?)
(fn [^File f]
(.getPath f)))
ignore-errors?)
dirs)))

(defn throw-unless-clj-file [file-path]
(when-not (re-matches #".+\.clj$" file-path)
Expand Down
43 changes: 28 additions & 15 deletions src/refactor_nrepl/ns/libspecs.clj
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
(ns refactor-nrepl.ns.libspecs
(:require [refactor-nrepl.core :as core]
[refactor-nrepl.ns.ns-parser :as ns-parser]
[refactor-nrepl.util :as util])
(:import [java.io File]))
(:require
[refactor-nrepl.core :as core]
[refactor-nrepl.ns.ns-parser :as ns-parser]
[refactor-nrepl.util :as util])
(:import
(java.io File)))

;; The structure here is {path {lang [timestamp value]}}
;; where lang is either :clj or :cljs
Expand Down Expand Up @@ -43,23 +45,34 @@
(put-cached-libspec f lang)))

(defn namespace-aliases
"Return a map of file type to a map of aliases to namespaces
"Returns a map of file type to a map of aliases to namespaces
{:clj {util com.acme.util str clojure.string
:cljs {gstr goog.str}}}"
([]
(namespace-aliases false))
([ignore-errors?]
{:clj (->> (core/find-in-project (util/with-suppressed-errors
(some-fn core/clj-file? core/cljc-file?)
ignore-errors?))
(map (partial get-libspec-from-file-with-caching :clj))
aliases-by-frequencies)
:cljs (->> (core/find-in-project (util/with-suppressed-errors
(some-fn core/cljs-file? core/cljc-file?)
ignore-errors?))
(map (partial get-libspec-from-file-with-caching :cljs))
aliases-by-frequencies)}))
(namespace-aliases ignore-errors? (core/source-dirs-on-classpath)))
([ignore-errors? dirs]
(let [;; fetch the file list just once (as opposed to traversing the project once for each dialect)
files (core/source-files-with-clj-like-extension ignore-errors? dirs)
;; pmap parallelizes a couple things:
;; - `pred`, which is IO-intentive
;; - `aliases-by-frequencies`, which is moderately CPU-intensive
[clj-files cljs-files] (pmap (fn [[dialect pred] corpus]
(->> corpus
(filter pred)
(map (partial get-libspec-from-file-with-caching dialect))
aliases-by-frequencies))
[[:clj (util/with-suppressed-errors
(some-fn core/clj-file? core/cljc-file?)
ignore-errors?)]
[:cljs (util/with-suppressed-errors
(some-fn core/cljs-file? core/cljc-file?)
ignore-errors?)]]
(repeat files))]
{:clj clj-files
:cljs cljs-files})))

(defn- unwrap-refer
[file {:keys [ns refer]}]
Expand Down
39 changes: 37 additions & 2 deletions src/refactor_nrepl/util.clj
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
(ns refactor-nrepl.util
(:require [clojure.string :as string])
(:import java.util.regex.Pattern))
(:require
[clojure.java.io :as io]
[clojure.string :as string])
(:import
(java.io File)
(java.util.regex Pattern)))

(defn normalize-to-unix-path
"Replace use / as separator and lower-case."
Expand Down Expand Up @@ -99,3 +103,34 @@
;; in other places that already are using `some-fn`, `every-pred`, etc
([_]
(.isInterrupted (Thread/currentThread))))

(defn dir-outside-root-dir?
"Dirs outside the root dir often represent uninteresting dirs to examine, e.g.
Lein checkouts, .gitlibs from deps.edn, or other extraneous source paths
which aren't directly related to the project.
By excluding them, one gets better and more accurate performance."
[^File f]
{:pre [(-> f .isDirectory)]}
(let [f (-> f .getCanonicalPath File.)
root-dir (File. (System/getProperty "user.dir"))
parent-dirs (->> f
(iterate (fn [^File f]
(some-> f .getCanonicalPath File. .getParent File.)))
(take-while some?)
(set))]
(not (parent-dirs root-dir))))

(defn data-file?
"True of f is named like a clj file but represents data.
E.g. true for data_readers.clj"
[path-or-file]
(let [path (.getPath (io/file path-or-file))
data-files #{"data_readers.clj" "project.clj" "boot.clj"}]
(reduce (fn [acc data-file]
(let [v (or acc (.endsWith path data-file))]
(cond-> v
v reduced)))
false
data-files)))
13 changes: 10 additions & 3 deletions test/refactor_nrepl/core_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@
(:require
[clojure.test :refer [are deftest is testing]]
[refactor-nrepl.config :as config]
[refactor-nrepl.core :refer [ignore-dir-on-classpath? read-ns-form]])
[refactor-nrepl.core :as sut])
(:import
(java.io File)))

(defmacro assert-ignored-paths
[paths pred]
`(doseq [p# ~paths]
(is (~pred (ignore-dir-on-classpath? p#)))))
(is (~pred (sut/ignore-dir-on-classpath? p#)))))

(deftest test-ignore-dir-on-classpath?
(let [not-ignored ["/home/user/project/test"
Expand All @@ -32,9 +32,16 @@
(are [input expected] (testing input
(assert (-> input File. .exists))
(is (= expected
(read-ns-form input)))
(sut/read-ns-form input)))
true)
"test-resources/readable_file_incorrect_aliases.clj" nil
"testproject/src/com/example/one.clj" '(ns com.example.one
(:require [com.example.two :as two :refer [foo]]
[com.example.four :as four]))))

(deftest source-files-with-clj-like-extension-test
(let [result (sut/source-files-with-clj-like-extension true)]
(doseq [extension [".clj" ".cljs" ".cljc"]]
(is (pos? (count (filter (fn [^File f]
(-> f .getPath (.endsWith extension)))
result)))))))
13 changes: 7 additions & 6 deletions test/refactor_nrepl/ns/namespace_aliases_test.clj
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
(ns refactor-nrepl.ns.namespace-aliases-test
(:require [clojure.test :refer [deftest is]]
[refactor-nrepl.core :as core]
[refactor-nrepl.ns.libspecs :as sut]
[refactor-nrepl.util :as util]
[refactor-nrepl.unreadable-files :refer [ignore-errors?]]))
(:require
[clojure.test :refer [deftest is]]
[refactor-nrepl.core :as core]
[refactor-nrepl.ns.libspecs :as sut]
[refactor-nrepl.unreadable-files :refer [ignore-errors?]]
[refactor-nrepl.util :as util]))

(defn finds [selector alias libspec]
(let [aliases (selector (sut/namespace-aliases ignore-errors?))]
(let [aliases (selector (sut/namespace-aliases ignore-errors? (core/dirs-on-classpath)))]
(some (fn [[k vs]]
(and (= k alias)
(some #{libspec} vs)))
Expand Down
26 changes: 24 additions & 2 deletions test/refactor_nrepl/util_test.clj
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
(ns refactor-nrepl.util-test
(:require [clojure.test :refer [deftest is]]
[refactor-nrepl.util :as sut]))
(:require
[clojure.test :refer [are deftest is]]
[refactor-nrepl.util :as sut])
(:import
(java.io File)))

(deftest with-additional-ex-data-test
(try
Expand All @@ -10,3 +13,22 @@
(catch clojure.lang.ExceptionInfo e
(let [{:keys [foo]} (ex-data e)]
(is (= foo :bar))))))

(deftest dir-outside-root-dir?-test
(are [input expected] (= expected
(sut/dir-outside-root-dir? input))
(File. (System/getProperty "user.dir")) false
(File. ".") false
(File. "src") false
(File. "/") true))

(deftest data-file?-test
(are [input expected] (= expected
(sut/data-file? input))
"project.clj" true
"boot.clj" true
"data_readers.clj" true
"project.cljs" false
"boot.cljs" false
"data_readers.cljs" false
"a.clj" false))

0 comments on commit 1399da2

Please sign in to comment.