Skip to content
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

Parallelize referred-syms-by-file&fullname #320

Merged
merged 2 commits into from
Jul 9, 2021
Merged

Parallelize referred-syms-by-file&fullname #320

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ccf9398
Parallelize `referred-syms-by-file&fullname`
vemv Jul 8, 2021
02bc6d3
PR feedback
vemv Jul 9, 2021
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
## Unreleased

#### Changes

* [(Part of #230)](https://github.com/clojure-emacs/refactor-nrepl/issues/230): Parallelize various functionality
* This will have a noticeable improvement in e.g. clj-refactor.el's `cljr-slash` performance.
* [#291](https://github.com/clojure-emacs/refactor-nrepl/issues/291): The `:ignore-errors` option will be honored in more places, making refactor-nrepl more robust in face of files not particularly meant to be part of the AST corpus.
* Examples: WIP files, Moustache template files, scripts.
* Upgrade Orchard
Expand All @@ -15,6 +18,7 @@
* Honor internal `future-cancel` calls, improving overall responsiveness and stability.

### Bugs fixed

* [#289](https://github.com/clojure-emacs/refactor-nrepl/issues/289): Fix an edge-case with involving keywords that caused find-symbol to crash.
* [#305](https://github.com/clojure-emacs/refactor-nrepl/issues/305): Don't put `:as` or `:refer` on their own lines in the ns form, when the libspec is so long it causes the line to wrap.
* [clojure-emacs/clj-refactor.el#459](https://github.com/clojure-emacs/clj-refactor.el/issues/459): `clean-ns` should conform to the style guide: `(:require` in the ns form should be followed by a newline.
Expand Down
24 changes: 16 additions & 8 deletions src/refactor_nrepl/core.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,15 +99,20 @@
(defn find-in-dir
"Searches recursively under dir for files matching (pred ^File file).

Note that files which are non-existant, hidden or build-artifacts
Note that files which are non-existent, hidden or build-artifacts
are pruned by this function."
[pred dir]
(->> dir
file-seq
(filter (every-pred fs/exists?
(complement fs/hidden?)
pred
(complement build-artifact?)))))
(->> dir
file-seq
;; `pmap` performs better in large projects.
(pmap (fn [f]
(when ((every-pred fs/exists?
(complement fs/hidden?)
pred
(complement build-artifact?))
f)
f)))
(filter identity)))

(defn read-ns-form
([path]
Expand Down Expand Up @@ -184,7 +189,10 @@
(defn find-in-project
"Return the files in the project satisfying (pred ^File file)."
[pred]
(-> find-in-dir (partial pred) (mapcat (dirs-on-classpath)) distinct))
(->> (dirs-on-classpath)
(pmap (partial find-in-dir pred))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I chose pmap for simplicity, it's better than some tend to say

(because pmap won't create one thread per item... it takes the CPU count in account)

...it would be inefficient for very small inputs, but it's safe to say that most projects have multiple dirs in the classpath (especially in monorepos), plenty of namespaces, etc.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good choice! The small workloads that will get slightly slower will still be more than fast enough in the face of some fork/join overhead, I think :)

(apply concat)
distinct))

(defn throw-unless-clj-file [file-path]
(when-not (re-matches #".+\.clj$" file-path)
Expand Down
7 changes: 4 additions & 3 deletions src/refactor_nrepl/ns/libspecs.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,21 +77,22 @@

(defn referred-syms-by-file&fullname
"Return a map of filename to a map of sym fullname to sym
the sym itself
the sym itself.

Example:
{:clj {\"/home/someuser/projects/some.clj\" [\"example.com/foobar\" foobar]}
:cljs}"
([]
(referred-syms-by-file&fullname false))
([ignore-errors?]
;; `pmap` is used as it has proved to be more efficient, both for cached and non-cached cases.
{:clj (->> (core/find-in-project (util/with-suppressed-errors
(some-fn core/clj-file? core/cljc-file?)
ignore-errors?))
(map (juxt identity (partial get-libspec-from-file-with-caching :clj)))
(pmap (juxt identity (partial get-libspec-from-file-with-caching :clj)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd add a note about the use of pmap, as people tend to "optimize" it by removing it. :D

sym-by-file&fullname)
:cljs (->> (core/find-in-project (util/with-suppressed-errors
(some-fn core/cljs-file? core/cljc-file?)
ignore-errors?))
(map (juxt identity (partial get-libspec-from-file-with-caching :cljs)))
(pmap (juxt identity (partial get-libspec-from-file-with-caching :cljs)))
sym-by-file&fullname)}))