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.

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

cljr-slash is very slow #230

Closed
kommen opened this issue Jul 20, 2018 · 10 comments · Fixed by #328
Closed

cljr-slash is very slow #230

kommen opened this issue Jul 20, 2018 · 10 comments · Fixed by #328
Assignees
Labels

Comments

@kommen
Copy link

kommen commented Jul 20, 2018

Expected behavior

Typing a / is fast

Actual behavior

Typing a / makes emacs hang every time for about 0.5 to ~1 second.

Steps to reproduce the problem

  • have a significantly sized clojure project
  • type / in clojure file

Environment & Version information

spacemacs (current develop branch)

clj-refactor.el and refactor-nrepl version information

clj-refactor 2.4.0-SNAPSHOT (package: 20180708.57), refactor-nrepl 2.4.0-SNAPSHOT

CIDER version information

;; CIDER 0.18.0snapshot (package: 20180719.542), nREPL 0.2.13
;; Clojure 1.9.0, Java 1.8.0_102

Emacs version

GNU Emacs 26.1 (build 1, x86_64-apple-darwin17.5.0, NS appkit-1561.40 Version 10.13.4 (Build 17E199)) of 2018-05-28

Operating system

macOS 10.13.6

Profiler

- command-execute                                                1715  77%
 - call-interactively                                            1715  77%
  - funcall-interactively                                        1269  57%
   - cljr-slash                                                  1234  55%
    - cljr--magic-requires-lookup-alias                          1233  55%
     - cljr--get-aliases-from-middleware                         1233  55%
      - cljr--clj-context-p                                       762  34%
       - cljr--prompt-user-for                                    762  34%
        - completing-read                                         762  34%
         + #<compiled 0x50071a75>                                 762  34%
      - cljr--call-middleware-for-namespace-aliases                471  21%
       - cljr--call-middleware-sync                               354  15%
        + cider-nrepl-send-sync-request                           354  15%
       + edn-read                                                 117   5%
    + cljr--in-keyword-sans-alias-p                                 1   0%
@expez
Copy link
Member

expez commented Jul 20, 2018

Always or only the first time?

It builds up a cache the first time this command is called. Parsing all namespaces in the project to find the most likely candidates can obv take some time i large projects.

@kommen
Copy link
Author

kommen commented Jul 21, 2018

@expez just verified that in .clj files it is actually fast as expected, but in .cljs and .cljc files it is always slow.

@expez
Copy link
Member

expez commented Jul 21, 2018

Maybe the cache isn't working as it should for those dialects. Thanks for investigating.

@expez expez added the bug label Jul 21, 2018
@vemv
Copy link
Member

vemv commented Mar 28, 2019

I wonder if addressing this one would be a matter of parallelising the workload (hypothesis: cljs workloads tend to be larger for some reason, hence the performance difference)

If accepted, the same technique suggested in #246 could be applied here.

@expez
Copy link
Member

expez commented Mar 28, 2019

If it's fast in clj files but slow in the other, then it's most likely an issue with the caching. Even if the workload on a cljc file is larger, it should only be performed once. From there on only the files that have been modified are re-examined.

@vemv vemv self-assigned this Jul 3, 2021
@vemv
Copy link
Member

vemv commented Jul 3, 2021

I self-assigned this one

IME cljr-slash is generally fast for .clj, for not so much for very large codebases where I experienced completion times of up to 7s. Parallelizing it fixed it

No idea about .cljs, will review as well

@expez
Copy link
Member

expez commented Jul 4, 2021

IME cljr-slash is generally fast for .clj, for not so much for very large codebases where I experienced completion times of up to 7s.

I use this as at work on a clojure codebase that's ~100kloc. I've never experienced it being slow, not even prior to the cache warming up.

Parallelizing it fixed it

Glad to hear this made it a lot faster!

Did you see if the caching was broken, though?

@vemv
Copy link
Member

vemv commented Jul 4, 2021

I use this as at work on a clojure codebase that's ~100kloc. I've never experienced it being slow, not even prior to the cache warming up.

That's weird, the biggest codebase I work on has a comparable size.

Did you see if the caching was broken, though?

Will check it as part of this task

@vemv
Copy link
Member

vemv commented Jul 8, 2021

(note to self, mostly)

On caching, there's no difference between the clj and cljs paths.

However one can observe that the caching is based on .lastModified. Having worked heavily on other tooling projects, I no longer trust .lastModified as a completely accurate source of truth.

So a plausible hypothesis is that in cljs usages, the build system, compilation model or other usage patterns render lastModified ~useless.

So there would be a couple lines of investigation:

  • check if indeed lastModified cannot be trusted for real-world cljs usages
  • use a mixture of lastModified and deriving SHAs out of file contents
    • slurping/SHAing is somewhat expensive so it should be only invoked if lastModified hasn't changed
    • (this technique was necessary and useful in the past for me)

@vemv
Copy link
Member

vemv commented Sep 16, 2021

I have a good reason to believe the cljs part of the problem will be solved by #328 : lein-cljsbuild copies unnecessary .cljs files around, and refactor-nrepl would inspect those by default.

#328 stops inspecting folders such as resources/. Between that and other optimizations, performance should become much better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants