Not sure how to help this, except by making completions as fast as possible

The newline is pretty odd, seems like a client issue

About the server performance, it should not be slow, we improved performance recently on that, maybe we Introduced a new issue? c/c @U07M2C8TT

@U0BUV7XSA curious, how much time completion and how many elements are coming? we have this answer on server logs included recently

@UKFSJSM38 yeah the client is getting confused because the lsp completion is so slow. it's taking about 700ms for 3-50 completions pretty consistently. definitely feels like a regression (this is on master).

yes, sounds like a regression, I think I noticed some performance issues on completion last week and didn't give the proper attention thinking it was something with my config, good to confirm that.

It may be related with some change on queries / completion we did recently @U07M2C8TT

Not much has changed in completions lately. We switched to the q/filter-project-analysis-xf transducer a few days ago. About a month back we added the practicalli snippets. I don’t see much else that’s different. I’ll plan to hook up the flamegraphs and see if I can track anything down.

curious @UKFSJSM38, how does lsp-mode handle late completions? If you put a 2s sleep in the completion handler, what happens if you have a completion item selected when they show up? @U07M2C8TT That was the first handler I wrote, looking at the code and there's gotta be redundancies in there. I'll bet we're iterating through analysis 3-4 times.

@U0BUV7XSA lsp-mode only show the completion popup when server returns

oh, so you don't see other sources?

I don't think so

I think it depends a lot on how you have completions configured. That suggests that there’s a nice way to configure completions, which is completely false. Completion configuration in Emacs is a disaster, IMO. In my—admittedly probably broken—configuration the editor and clojure-lsp compete for who gets to manage the completions. I still haven’t really figured out when one takes priority. @UKFSJSM38 helped me figure out a hack that prevents CIDER from stealing the completion priority when you start a REPL, but that was also a pain point for me for a long time.

yeah, emacs completion package (company) has the concept of backends (and frontends, but not relevant for now), so one could have a lsp-backend snippets-backend which would bring both completion items, but I'm not sure what happens when one takes more time than other, if company just wait all backends provide items

that means, it's possible to have both cider and clojure-lsp completions teorically, but that would probably cause lots of duplications and performance issues

> I’ll bet we’re iterating through analysis 3-4 times At one point we were iterating hundreds of times, although that was fixed in https://github.com/clojure-lsp/clojure-lsp/pull/764 It looks like you’re in a CLJS project @U0BUV7XSA, which has to return both clojure.core and cljs.core completions, both of which involve an expensive iteration. Out of curiosity, is this problem worse in CLJS projects?

Oh, another question @U0BUV7XSA… does your completion start after 1 character? Or worse, 0 characters? I made a series of PRs that speeds up clojure.core completion when there are lots of matches, but it can still be slow.

yeah, for emacs/lsp-mode 2 chars it's the recommended for best performance/UX tradeoff

(0 characters would be completion from a blank line or something like that. You’d notice log lines with huge numbers of items :completion 1000ms - total items: 899 )

Yup, that was a cljs project, in a .cljs file. Getting 300-1500ms in clojure-lsp, even at 5 chars. Seems like 400ms standard.

in the cljs project, 700-800ms seems to be consistent

-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
ClojureScript                    8            129              3           1143
ClojureC                         3             45              0            411
Clojure                          3             33              0            227
-------------------------------------------------------------------------------
SUM:                            14            207              3           1781
-------------------------------------------------------------------------------

Hmm, not really that big of a project

(What LOC tool do you use?)

http://github.com/AlDanial/cloc

@UKFSJSM38 do you remember how to change the number of characters you need to type before completion starts in Emacs?

yes: (setq company-minimum-prefix-length 2)

thanks

@U0BUV7XSA I’m having trouble reproducing times that long. I have a CLJS project of a similar size (1900 lines). I’ve adjusted completion to start after 1 character. For me, completion is generally in the 200-300ms range, occasionally getting up toward 400ms. Still not immediate, but closer to what I recall the performance being.

Will you paste your server-info?

@UKFSJSM38 I’m noticing that both :classpath and :settings :source-paths contain duplicates. I’m not sure that’s related to this problem, but it can’t be good for performance

hum, not aware of that, could you paste a example?

Odd, I see only the src twice, we should save that as a set or fix the duplication

Yes, src twice in :source-paths, but many other things are duplicated in :classpath —for example cemerick/url. Both of them should be sets

Oh that's pretty bad and should be fixed, yeah, making a set would easily solve that but I wonder why that is happening

here's all my dep setup and server info. Something I realized is by having shadow handle one classpath and lein the other, the dep trees will have conflicting versions when we mash them together (see slf4j-api) https://gist.github.com/snoe/3375ccc12bf28f366f0a6b01b6ed022d

I also see the same version of medley due to this mashing.

Just to keep track, I create https://github.com/clojure-lsp/clojure-lsp/issues/908 for that @U07M2C8TT

Yeah that's badd too @U0BUV7XSA 😂 not sure how to solve that, maybe consider only the most recent?

Probably

Or rather I think there's stuff in java to take a classpath and produce a package list perhaps. only the first package wins in classpath order

The :classpath also often has duplicates because of relative paths: /Users/case/dev/secret/cross/src and cross/src . Is that expected @UKFSJSM38?

Same problem with relative paths in :source-paths too, actually

I have a theory for why I see duplicate :source-paths in my CLJS project. I have a deps.edn file, with :path ["src"]. I also have a shadow-cljs.edn file, with :deps true, which in shadow terms means “load deps from deps.edn”. So, when we ask both dependency managers for paths, they both include "src" .

the :source-paths should be always absolute, I thought we did that somewhere

but yeah, I think lein returns relative and deps.edn absolute or something like that, we should always "absolutize" so we can set safely

BTW, almost sure we set that somewhere before sending to kondo, but probably we should do it earlier right after getting the classpath/source-paths

@U07M2C8TT is the code to produce the flame graph still there? I'm curious to see on this project.

yeah, there is a commit/branch somewhere that @U07M2C8TT linked it in a recent PR

https://github.com/ptaoussanis/tufte is what I used when building out the parser. It was nice to see only the call times I was interested in

@U0BUV7XSA there are some instructions for getting a profiling build here https://clojure-lsp.io/development/#profiling. I’ll dig up one of the commits @UKFSJSM38 mentioned

@U0BUV7XSA the first commit in https://github.com/clojure-lsp/clojure-lsp/pull/795/commits includes code for constructing flamegraphs

Since I can’t reproduce the poor performance you’re seeing, I was going to suggest that we get your environment set up to do some profiling. Let me know how I can help. I can jump on a call if that’s easiest

@U07M2C8TT here's the flamegraph, I kinda hate sampling profilers, so maybe you can get better insight than I. The first is a single run of a completion, the second is dotimes [n 100] of the same call. You can see times are consistently 700ms. What is a typical time for a completion for you in the clojure-lsp project?

For context: Originally, completion ran synchronously as you typed, and I know that at one point I had it down below 30ms on this machine with a project with 10x the deps.

before graal/kondo

Ah, those flamgraphs are very interesting… jar-file? shows up three different times and accounts for about 50% of the time. That’s new code. Looks like maybe an inefficient regex. Could that function just be (string/includes? % ".jar") @UKFSJSM38?

Completion is taking 200-400ms for me at the moment in clojure-lsp, with completion starting after 1 character

We have some work to do before we’re back down to the tens of milliseconds. It’s not impossible but it’ll take some work

> That’s new code. Looks like maybe an inefficient regex. Could that function just be (string/includes? % ".jar") @UKFSJSM38? Yep, probably that's the culprit :)

yeah, IMO completion is more consistent than before but slower indeed probably related with the analysis iteration. For now I'll improve the jar-file? fixing that specific issue

@UKFSJSM38 class-file? could also be a string/ends-with? . That wouldn’t speed up completion, but might be worth changing at the same time.

that reminds me that is pretty hard to make some code that could not side effect other features, we should try in the future to have performance tests just like kondo has

yes, I was not aware those functions could casue that slowness, will do some checking on all of them profiling it and let you know

> we should try in the future to have performance tests Yeah, good idea. Want to start an Issue where we can discuss that?

yep, feel free to create one regarding that

BTW, I think we will have to use includes? instead of ends-with? as we can have cases like: /foo/bar.jar!/baz.clj

Yeah, I was expecting includes? for jar-file?, and ends-with? for class-file?

Tried something naive for jar-file?

(def ^:private jar-file-with-filename-regex-fast #"\.jar:.*")
(defn jar-file? [filename]
  (or (boolean (re-find jar-file-with-filename-regex-fast filename))
      (boolean (string/ends-with? filename ".jar")))) 

(defn jar-file? [^String filename]
  (or (boolean (.contains filename ".jar:"))
      (boolean (string/ends-with? filename ".jar"))))

Changing with-core-items to not call var-defs-in-ns-named cuts time to ~200ms - it's just used for :kind so that should be hardcoded into common-sym

Yeah, sounds like a valid improvement

in completion, we make external-ns-elements and other-ns-elements and then immediately (concat external-ns-elements other-ns-elements) and do nothing else with them. I believe the concats are probably a mistake (we don't want other project's aliases), but as it stands all that code can be simplified to all-other-analysis (mapcat val (dissoc analysis filename))

that takes it down to ~60ms

@U0BUV7XSA thanks, I'll take a look and check if those improvements are valid, it probably is, just to make sure we are not missing anything

thanks @UKFSJSM38 ya, I probably won't have time to make the pr, just putting it here to look into.

Np, that is already really valuable info

Those are great observations @U0BUV7XSA. I didn’t realize that the only benefit of looking up all those clojure.core items was getting :kind. If that can be hard-coded, that trims off a huge amount of time. I agree that we don’t need external aliases but we might want external keywords, for libs that export specs or re-frame events, right? If we do, I’d accept using the full analysis—and getting unwanted aliases—just to get better performance. Several days ago @UKFSJSM38 was seeing the filters for internal/external filenames take upwards of 100ms, even before this performance regression. In the long run, we need to be able to separate internal and external files quickly, as well as pick out files that modify particular namespaces. We’ve been talking about keeping a cache of file metadata for this case among others. Any thoughts on that?

Hrm, so, in that project there's 40k files in (keys analysis) . 18k .java & 21k .class. Seems kinda crazy to be the java stuff in the same analysis to me. But this shows that external can be 2-4 order of magnitude bigger than project (even without the java deps, so many projects just balloon like this). So I agree it's necessary to separate the two (I'd argue this shows java stuff is a third). However, I think this clearly shows that walking through external is expensive. So external keywords and stuff, should probably be an opt in setting if they can't be narrowed down to a particular file based on the cursor-value. I can't think why they can't follow the same rules as we do for namespace functions: if the alias/ns is a perfect match show all the functions. Caches for external stuff makes sense to me, if the deps change clear and recalc once. For project stuff I think it's real tricky and should be avoided beyond "these files are in the project" or "this is a separate project analysis". I used to analyze source-paths separately from classpath for this reason.

we still analyze source-paths separately from classpath, and I agree if we would have a cache for project analysis the consistency needs to be perfect, I think starting with external deps is a huge step and if it works well we can think about how that would work for project in the future. But basically the naive idea is: if my cache says file /a.clj has SHA 123, and analysis {...}, next time, if sha matches I don't want to analyze that again, unless kondo config/version changed and etc, needs some research yet tho

> in completion, we make external-ns-elements and other-ns-elements and then immediately (concat external-ns-elements other-ns-elements) and do nothing else with them. I believe the concats are probably a mistake (we don't want other project's aliases), but as it stands all that code can be simplified to all-other-analysis (mapcat val (dissoc analysis filename)) You are right, that's probably a regression related to the multiple refactors that function already received 😅 I'm working on the completion improvements discussed here, and managed to make take ~120ms with a empty completion with >1000 items 😄 It's really notable the improement on the editor

https://github.com/clojure-lsp/clojure-lsp/commit/cd3300e93349dabe3689815df5107c7aafbd8b38 improves the performance, but it may be a good idea @U0BUV7XSA test in that huge project of yours. Testing on clojure-lsp, I noticed the perofmrnace improvent was from 199ms -> 110ms, but probably before the change huge projects with a lot of analysis should grow exponentially that number

ooh… I’m excited to try this @UKFSJSM38. I was also noticing that some vars were added in Clojure 1.10 and 1.11 that haven’t made it into clojure-lsp. Some of them are new and some of them have moved from cljs to clj. Are you already working on adding those? If not, I can take that

nope, I noticed the same, but didn't touch yet, we should add missing ones like update-val etc indeed

With that patch, completion is consistently in the 100-200ms range for me. Awesome!

I'm down to 150 - 250ms - the late pop-in virtually gone so this a great step.

thanks @UKFSJSM38

Great, we can probably focus on minor improvements now as I don't think/didn't easily found some code that takes most of the time, a flamegraph profiler would help with that probably

@U07M2C8TT thanks for the debug-perf-cli build, I wonder if there's a reason to keep it separate from debug. I also think it might be nice to have a ./dev/clojure-lsp/debug.clj file with snippets and instructions to run a profile or a criterium.

Yeah, both ideas sounds good to me, usually there are user.clj namespaces on lein projects, maybe we could have a performance one or something

Yeah, agreed. @UKFSJSM38 I think you suggested merging the perf deps into the main debug build when I first proposed it. I can’t remember why I was hesitant then. In retrospect, it doesn’t seem like it would hurt

what exactly? find definition for npm you mean?

that is not supported yet along with java completion. we intend to add that soon on clj-kondo so we can make clojure-lsp work

Calva does JS completions when the REPL is connected. Not working for you, @U037TPXKBGS?

@UKFSJSM38 yeah, for installed and required/imported npm modules/packages. And yeah that makes sense! @U0ETXRFEW For arbitrary third party npm libraries? I've seen it for standard js stuff like console log and such so far, but not third party ones. Function parameter help would be nice too!

yeah, that is something cool we would like to add in the future

Like so for a require like

(:require ["@snowplow/browser-plugin-link-click-tracking" :as link-tracking]
            ["@snowplow/browser-tracker" :as sbt]

Not sure where the limits go for this approach, but the project to contribute to for improving it is: https://github.com/clojure-emacs/clj-suitable

Huh, i don't think i get that consistently. It is complicated because our work codebase is a big monorepo so calvas single repl limitation plus my frequently working across the monorepo might affect that