What's the project size just for curiosity? Clojure-lsp is pretty fast nowadays :thinking_face:

The project is pretty big… It would take a bit of work to get an exact number but my rough find is:

find . -type f -name "*.clj*" | wc -l    
   12595

or 12.5k files

Seems a lot indeed, so maybe it's expected. What you can do to help debug what is taking time, is pass a --verbose flag and paste here

I can run that, but I guess my bigger question is, since we’re only formatting the files given, is it necessary to analyze the project?

I think for format and clean-ns we don't do that anymore, only for diagnostics

The log should tell that

okay, let me run it and get back to you 👀

clojure-lsp clean-ns --verbose --filenames file
[ 10%] Finding cache               [INFO] [DB] Reading transit analysis cache from .lsp/.cache/db.transit.json db took 313ms
[INFO] [Startup] Using cached db for project root /project
[INFO] [Startup] Using given source-paths: [<TRUNCATED but maybe 12 paths>]
[ 15%] Copying kondo configs       [INFO] Copying kondo configs from classpath to project if any...
Configs copied:
- .clj-kondo/funcool/promesa
[INFO] Copied kondo configs, took 601ms secs.
[ 20%] Analyzing project files     [INFO] [Startup] Analyzing source paths for project root /project
[INFO] [Startup] Project only paths analyzed by clj-depend, took 0ms
[ 99%] Analyzing project files     No configs copied.
[INFO] Linting whole project for unused-public-var took 379ms
[INFO] [Startup] Project only paths analyzed by clj-kondo, took 35719ms
[100%] Project analyzed            
Checking namespaces...
[INFO] :execute-command 29ms
Cleaned <ns>

This is expected, it's taking 35s to analyze all the 12k files We don't analyze the classpath, but we do always analyze the project

We could stop doing that for format only, but not for clean-ns and even so, not sure it's worth

35s doesn't sound that much for 12k files to be honest

It doesn’t seem to persist though, I ran it a second time and it took another 35s, which isn’t really acceptable for a commit hook

I’m curious, can you go into some detail about why analysis is necessary for clean-ns? Shouldn’t there be enough info in the ns alone to know if it’s used within that namespace?

What is persisted is only the external classpath one, not the project code analysis for consistency

Clean-ns relies on kondo analysis about unused alias and refers and other diagnostics

I see, and there’s no way to persist this cache so that analysis doesn’t take long after the first time?

ATM no, but me and @U07M2C8TT had some discussions about that, maybe we could add a feature flag setting to smart cache files like we do for external deps

Ah I see. I’ll keep an eye out for that then (or if there is a way to not analyze the whole file for clean-ns, that would be even nicer 😄). For now I’ll stick with our current commands. Thanks Eric!

@slack1338 your statement “Shouldn’t there be enough info in the ns alone to know if it’s used within that namespace” hides a lot of complexity. You’re saying, I think, that it should be possible to read a file, parse it, then look through it to figure out which namespaces are used and which aren’t. That’s true, but that is what clj-kondo is doing. That is, when we run clj-kondo to get analysis, we’re doing the work you’re describing. Now granted, we run analysis on every project file, including ones that haven’t changed since the last time you ran clean-ns. That’s extra work, but let me explain why we haven’t tried caching project analysis yet. First, as @ericdallo said, we do cache analysis for external dep files. We can do that because we don’t expect them to change much. We can analyze them and cache them at startup. But project files are a different story. Not only do they change frequently, but often (since clojure-lsp is usually interacting with an editor, not with the CLI) the version that clojure-lsp has isn’t even the same as what’s saved on disk. It’s very bad to have incorrect analysis. If you do, refactorings like clean-ns can create syntax errors, or worse, create syntactically valid but incorrect code. So, we’ve shied away from caching analysis, for fear that we’d cache the wrong thing, or that it would get out of date. So, when interacting with an editor we have all these unsaved versions of files floating around, and know that bad things would happen if we cached their analysis. That’s why we haven’t wanted to try to cache project analysis—too much can go wrong. Running refactorings on the CLI is a bit of a different story though. The CLI looks at what’s on disk, not ephemeral unsaved versions, so it’d be safer to cache the analysis it does (along with timestamps or shas or whatever to easily detect changed files). Perhaps we could do it, but the CLI is a bit of a second-class citizen. @ericdallo has done some refactoring specifically to make the CLI faster, but it doesn’t happen often. Much more effort goes into making the editor interaction as good as it can be. Technically it is possible to cache analysis even as files change. clj-kondo does a good job of this itself, although it only caches a tiny portion of the analysis it generates. We’d need to cache a lot more. Anyway, all of this is to say we might do this someday, but I wouldn’t hold my breath. In the meantime, I recommend using the CLI version of clean-ns less frequently (perhaps in a pre-push hook instead of a commit hook) or using your existing commands.

I think I may be misunderstanding then, since as far as I can understand, the analysis is useful but not necessary for cleaning a namespace. You should be able to look at all bindings, function calls, etc. in a particular file and check against the require d things in the ns, and if one is required but not used, it can be removed. Is this wrong?

It sounds like your image of analysis is different from mine. Will you explain what you think it is? That’ll help me explain

If I were to explain it, I’d say clj-kondo defines “analysis” as the process of looking through a file for (among other things) all the namespaces in the require, then looking for all the var usages, then resolving their aliases to figure out which namespaces they use. There’s a second step, which clj-kondo calls “findings” or “diagnostics” which is the process of deciding that based on the prior step, some namespaces in the require aren’t actually used. Or to put it another way, you’re saying “analysis is useful but not necessary; you should be able to do [all the work of analysis] instead.” See why that’s confusing? :)

I think I see. Thanks for explaining in more detail 🙂 Maybe a different way of looking at it (or a different way of forming the question): clojure-lsp allows for --filenames, is there sufficient information from clj-kondo --lint or another command that only works on specific filenames to perform the cleaning? I don’t know either tools nearly well enough to know the answer, and maybe the answer is “no” :thinking_face: I think the way I understand analysis is the same or similar to the way you’re describing it, but I also wonder if there are ways to just get a single file analysis (shallow analysis?) without analyzing the whole project for certain cases like cleaning the ns. For things like references I think it’s necessary but I think for other cases, at least on the cli, it might be okay to not do an entire analysis. However I don’t know if kondo even supports this idea (or if it’s desirable)

That’s a good point… at the moment the CLI runs analysis on the entire project, then cleans the namespaces of the --filenames. But in the particular case of clean-ns the CLI should (I think) have enough information if we were to analyze only the --filenames. ( The same is not true for diagnostics or rename, which definitely need full analysis.) We already have tools to lint analyze and lint a subset of files, so that part is built already. @ericdallo do you have a sense of whether the CLI’s analysis could be changed to focus on a small set of files? That would be less intrusive than caching. I want to be cautious though—you may actually need the full project analysis to run clean-ns. It’s hard to keep in your head all the times that some feature uses the full analysis. Sometimes the intelligence of the system surpasses that of its builders.

yeah, I think it's possible to lint only files from --filenames if this flag is provided, I guess clj-kondo analysis is already smart enough for that — it can check if a require is being used on a file even not knowing the definition of that required namespace in the project. It may worth checking if that is possible, but I'd suggest not spend that much effort, as I said, we should know the tradeoff of tuning performance and keeping it easy enough to maintain/evolve