Limit var analysis #1674
Limit var analysis #1674
Conversation
If you set {:output {:analysis {:var-definitions {:shallow true}}}}, the
names (and optionally metadata and arglists) of vars will be analyzed, but
their bodies will be skipped.
This is useful to analyze the skeleton of a namespace, without getting
any details. It is an expert option, and if you use it, you should not
expect linters to behave correctly anymore.
More specifically, the bodies of these forms are skipped:
- def
- defn
- defrecord
- defmethod
- plumatic schema; fn, def, defn, defmethod, defrecord
- protocol-impls
- fn
- letfn
- fn*
- bound-fn
src/clj_kondo/core.clj
Outdated
| @@ -18,7 +18,12 @@ | |||
| subject to change." | |||
| [{:keys [:config :findings :summary :analysis]}] | |||
| (let [output-cfg (:output config) | |||
| fmt (or (:format output-cfg) :text)] | |||
| fmt (or (:format output-cfg) :text) | |||
There was a problem hiding this comment.
It's unclear to me why this code was moved.
There was a problem hiding this comment.
Sorry, I'll put this back... it was part of an intermediate refactoring where the logic for when to show the analysis got complicated and I didn't want to duplicate it. Now, the presence or absence of analysis is all that's needed so it'll be ok to duplicate.
| :interop? interop? | ||
| :resolved-core? resolved-core? | ||
| :in-def (:in-def ctx)})) | ||
| (when (:analyze-var-usages? ctx) |
There was a problem hiding this comment.
This doesn't make sense to me. The :analyze-*? flags are intended for "is clj-kondo used with the analysis option and which one exactly?". But this option is usually off, when clj-kondo is not being called from clojure-lsp. However, reg-var-usage! is necessary for linting.
There was a problem hiding this comment.
:analyze-var-usages? is true by default, so this is really a check for whether the user has explicitly turned off var-usages. That means reg-var-usage! will still be run when linting. If you'd prefer a different name for this flag, let me know... I picked :analyze-var-usages? because it's consistent with the other existing :analyze-*? flags.
There was a problem hiding this comment.
Maybe we can make this more clearer because I'm pretty it will confuse me in the future. What about (not (:shallow-var-usages? ctx))? Or (:non-shallow-var-analysis? ctx)?
There was a problem hiding this comment.
I'm trying of a better name, but trying to avoid (:analyze-...? ctx) since that kind of implies that :analysis true was used.
There was a problem hiding this comment.
Maybe (not (:skip-var-usage-analysis? ctx)) would be clearest? ("shallow" applies to var-defs, not var-usages.)
| @@ -1876,7 +1883,8 @@ | |||
| expanded (assoc expanded :visited [resolved-namespace resolved-name])] | |||
| ;;;; This registers the original call when the new node does not | |||
| ;;;; refer to the same call, so we still get arity linting | |||
| (when-not same-call? | |||
| (when (and (:analyze-var-usages? ctx) | |||
| :config config | ||
| :expr %))) | ||
| opt-expr-children) | ||
| (when (:analyze-var-usages? ctx) |
| ;; save some memory | ||
| :expr (when-not dependencies expr) | ||
| :resolved-core? resolved-core?}))))) | ||
| (when (:analyze-var-usages? ctx) |
Per PR feedback.
This decreases uncached startup time by 60-70%. See clj-kondo/clj-kondo#1674. This also triples the batch size of the dep analysis, since clj-kondo seems to analyze about 3 times as many files in the same amount of time. I don't have numbers on how much memory was being saved by batching analysis, so that needs to be double-checked. This also skips all linting, and analysis of var-usages for deps since we don't use those. The performance gain is almost exclusively from skipping var bodies, however, so that's the interesting part.
This decreases uncached startup time by 60-70%. See clj-kondo/clj-kondo#1674. This also triples the batch size of the dep analysis, since clj-kondo seems to analyze about 3 times as many files in the same amount of time. I don't have numbers on how much memory was being saved by batching analysis, so that needs to be double-checked. This also skips all linting, and analysis of var-usages for deps since we don't use those. The performance gain is almost exclusively from skipping var bodies, however, so that's the interesting part.
* Skip var bodies when analyzing deps This decreases uncached startup time by 60-70%. See clj-kondo/clj-kondo#1674. This also triples the batch size of the dep analysis, since clj-kondo seems to analyze about 3 times as many files in the same amount of time. I don't have numbers on how much memory was being saved by batching analysis, so that needs to be double-checked. This also skips all linting, and analysis of var-usages for deps since we don't use those. The performance gain is almost exclusively from skipping var bodies, however, so that's the interesting part. * Add clojure.test and re-frame details to full-project and referenced-file analysis Co-authored-by: Eric Dallo <ericdallo06@hotmail.com>
This PR adds config options to limit the amount of analysis that is done.
:var-usages falseexcludes var-usages from the analysis output:var-definitions {:shallow true}includes var-definitions, but skips analysis of their bodiesIf both options are set analysis is a little over 80% faster. This will be helpful for clojure-lsp, which can use them when scanning deps.
This PR also moves the
:analysisconfig option to the top-level of:config, out of:output. Docs and tests have been updated to reflect this new location. To maintain backward compatibility, the code still accepts:analysisin the:outputconfig.Finally, this PR clarifies the meanings of
:analysisand:skip-lint. The output will include:analysisonly when the config includes:analysis. The:findingswill be non-empty only if the config doesn't include:skip-lint. The code tries to minimize the work performed based on these settings.I experimented with a
:var-definitions falsesetting. It was easy to implement, and I have the code if you'd like it, but it doesn't affect performance much. The true performance gain comes from skipping the definition bodies, not from skipping their signatures.Please answer the following questions and leave the below in as part of your PR.
I have read the developer documentation.
This PR correponds to an issue with a clear problem statement. See Slack discussion starting here and continuing here.
This PR contains a test to prevent against future regressions
I have updated the CHANGELOG.md file with a description of the addressed issue.