Clojurians Log v2

Patching the run! to get the pod working should be ok

@dominicm what's the issue with pods then? the tests seem to work:

Pod test
Cloning: 
Checking out:  at 47e55fe5e728578ff4dbf7d2a2caf00efea87b1e

Running tests in #{"pod-test"}
Downloading: org/clojure/clojure/1.11.3/clojure-1.11.3.pom from central
Downloading: org/clojure/clojure/1.11.3/clojure-1.11.3.jar from central

Testing clj-kondo.pod-test

Ran 1 tests containing 2 assertions.
0 failures, 0 errors.

I fixed them: https://github.com/clj-kondo/clj-kondo/pull/2643/files#diff-e6479bf8a0a80eb355c04e5185351707cdd12e5a377b8e3cfc1b9ea0d6536275R281-R288

there is an ns-group in your kondo config.edn, creating a test group!

oh right!

I measured this difference on metabase with JVM: 22785ms 18397ms

metabase quite heavily uses ns-groups

Nice! We have 8 of them.

I get an additional improvement of:

16558ms

oh wow, that's really interesting. I did wonder if you could pre-group all the nss rather than doing it on the fly like that.

I get:

17052ms

when I remove the pre-calculated patterns:

16753ms

OK, so not too much in it really. Interesting. I expected memoize to be a bit slower than that.

if you memoize on string arguments, it can be really fast

but memoization on collections can be very slow

my suggestion would be: change the PR to use a localized memoize just for something like this:

(memoize (fn [pat filename]
             (re-find (re-pattern pat) filename)))

localized I mean: created per run! invocation, so we don't hold on to memory

and then re-measure

How would it be isolated to a single run! invocation?

stuff it into the context I think

I'd need to pass the ns-groups function around, or otherwise update the callers to it to pass...

ah 🙂

OK

we could also use a dynamic var but stuffing it into the context has my preference

Don't know of any reason yet why context wouldn't be perfect.

@dominicm pushed one here: https://github.com/clj-kondo/clj-kondo/compare/master...ns-groups-perf

going to try the native now

without optimization, metabase takes 44 seconds...

with optimization 27307ms. nice

merged

cc @ericdallo a pretty impressive perf improvement was just merged - when people are using ns-groups, this will help

Oh wow, thanks! I'm absolutely pooped and wasn't going to get to this for a while. This will massively improve our lsp start time which has become very painful.

I'm going to look for more usages of re-pattern

oh that will affect nubank projects startup which use that as well, nice! will bump clojure-lsp ASAP

My colleagues will be buzzing 😁

🚀

hmm, memoize uses a seq of strings as a cache key now. perhaps I can make that faster by using a nested map. let's try

Pushed clojure-lsp, available as nightly build in ~5mins

nice speedup to be had here, memoize specialized for 2 args seems to be twice as fast:

(defn memoize2
  "Returns a memoized version of a referentially transparent function. The
  memoized version of the function keeps a cache of the mapping from arguments
  to results and, when calls with the same arguments are repeated often, has
  higher performance at the expense of higher memory use."
  {:added "1.0"
   :static true}
  [f]
  (let [mem (atom {})]
    (fn [a b]
      (let [args [a b]]
        (if-let [e (find @mem args)]
          (val e)
          (let [ret (f a b)]
            (swap! mem assoc args ret)
            ret))))))

by avoiding apply probably

ah well, this didn't help at all in the grand scheme of things, so I'm just going to use memoize

so, just use the clj-kondo pushed to master already, this is it. I made an issue for speeding up other regex calls.

Thanks again both, this is great ❤️

And thank you for mentioning this low hanging fruit optimization!

how did you discover it btw?

I scanned the code to see how filename-pattern was applied to check it was re-find (meaning .* is unnecessary in patterns) - so that I could optimise our regexs to speed things up (I saved 40s by not capturing unnecessary data!). I noticed the call to re-pattern and I know from a history of reading about the very cool FSM implementation behind it that it's expensive to generate. And also from the fact clojure added #"" to make it seamless to automatically optimise and type.

I wonder if your manual regex optimization would still speed up clj-kondo after this optimization

probably not due to memoization right?

I think it will. The regex construction alone wasn't expensive, it was also expensive to scan the whole string for a substring. Anchoring to the last 4 characters using $ means the regex engine knows to jump to the end and work backwards (kinda. It probably jumps to -4, and works forward)

That saves a lot of cycles on string matching.

instead of dozens of times

Ah, I forgot about that layer of optimization. Yeah, it probably won't be as noticeable.

hmm, what's not optimal in the current memoization setup is that regex creation is done over and over again for each regex / filename combo

wonder if it makes a huge difference if I optimize that... gonna try

only a small local change:

(let [re-pattern-memo (memoize re-pattern)]
                   (memoize (fn [pattern-str file-str]
                              (re-find (re-pattern-memo pattern-str) file-str))))

that doesn't seem to help that much

@ericdallo is the restriction of not renaming unqualified keywords already dropped in lsp? then I might upgrade

(projectile-replace did the job for now)

hum not really, no issue for that tho, would be nice to have one

https://github.com/clojure-lsp/clojure-lsp/issues/2139

now pushed lots more cached re-finds to master. no significant speedups on metabase though

for sure!

diff --git a/src/clj_kondo/impl/config.clj b/src/clj_kondo/impl/config.clj
index 356801ef..d9b9043a 100644
--- a/src/clj_kondo/impl/config.clj
+++ b/src/clj_kondo/impl/config.clj
@@ -476,10 +476,10 @@
   (keep (fn [{:keys [pattern
                      filename-pattern
                      name]}]
-          (when (or (and (string? pattern) (symbol? name)
-                         (re-find (re-pattern pattern) (str ns-name)))
-                    (and (string? filename-pattern) (symbol? name)
-                         (re-find (re-pattern filename-pattern) filename)))
+          (when (or (and pattern (symbol? name)
+                         (re-find pattern (str ns-name)))
+                    (and filename-pattern (symbol? name)
+                         (re-find filename-pattern filename)))
             name))
         (:ns-groups config)))
 
diff --git a/src/clj_kondo/impl/core.clj b/src/clj_kondo/impl/core.clj
index d0e3a67c..2ca5118d 100644
--- a/src/clj_kondo/impl/core.clj
+++ b/src/clj_kondo/impl/core.clj
@@ -183,7 +183,16 @@
                     (process-cfg-dir extra-config-dir))]
                  ;; command line config
                  (map read-config configs)))
-        config (config/expand-ignore config)]
+        config (config/expand-ignore config)
+        config (update config :ns-groups (fn [ns-groups]
+                                           (mapv
+                                            (fn [{:keys [filename-pattern pattern] :as ns-group}]
+                                              (cond-> ns-group
+                                                filename-pattern
+                                                (update :filename-pattern re-pattern)
+                                                pattern
+                                                (update :pattern re-pattern)))
+                                            ns-groups)))]
     (cond-> config
       cfg-dir (assoc :cfg-dir (.getCanonicalPath cfg-dir)
                      :use-import-dir (or import-dir-exists

If there's a mac binary I'm happy to compare performance on our codebase. I'm feeling quite unwell/tired today, otherwise I'd probably be installing graal and having a go.

do you happen to have sdkman installed?

I don't, but I could probably make it installed.

it's pretty easy to download a graalvm and then compile clj-kondo. download graalvm manually, untar/gz it and then set GRAALVM_HOME=... and then run script/compile in the clj-kondo repo if you use sdkman it's pretty easy to download graalvm too, but tbh, just curling it also works fine. you can find urls here: https://www.graalvm.org/downloads/

but if it's way faster on the JVM, I suppose it's going to help the binary too. JVM is a supported target for clj-kondo too, so worth it on its own

I expect the effect will be more pronounced on graal.

can you make an issue + PR and describe the solution (in any order)?

is avoiding calling re-pattern what makes it faster?

Calling it only once per regex, rather than for each file

Can you make a PR of this?

then we can download the mac binary and you can test it as well

https://github.com/clj-kondo/clj-kondo/pull/2643 done 🙂

nvm, managed to build using the http://graalvm.org download, much faster than I expected based on prior experiences. Although the gu command was missing.

My branch build of clj-kondo is 38s, running with the release clj-kondo is 46s.

My change does break pod usage because regex isn't edn safe, so we'll need to do something about that.

❯ hyperfine -i '/Users/dominicmonroe/src/github.com/borkdude/clj-kondo/clj-kondo --lint src:test --parallel' 'clj-kondo --lint src:test --parallel'
Benchmark 1: /Users/dominicmonroe/src/github.com/borkdude/clj-kondo/clj-kondo --lint src:test --parallel
  Time (mean ± σ):     33.922 s ±  0.886 s    [User: 38.404 s, System: 2.599 s]
  Range (min … max):   32.689 s … 35.191 s    10 runs
 
  Warning: Ignoring non-zero exit code.
 
Benchmark 2: clj-kondo --lint src:test --parallel
  Time (mean ± σ):     39.051 s ±  1.053 s    [User: 45.098 s, System: 2.646 s]
  Range (min … max):   37.819 s … 40.829 s    10 runs
 
  Warning: Ignoring non-zero exit code.
 
Summary
  /Users/dominicmonroe/src/github.com/borkdude/clj-kondo/clj-kondo --lint src:test --parallel ran
    1.15 ± 0.04 times faster than clj-kondo --lint src:test --parallel

I guess we could memorize re-pattern as an alternative solution

Typo by my phones spelling correction

Ha, don't worry my brain filled in the gap. I did consider that, but figured if we could avoid memoize then that would be faster. Are there a lot of places the config is printed and that would be a concern?

Probably pod is the main one