jshiell / checkstyle-idea

CheckStyle plug-in for IntelliJ IDEA
https://plugins.jetbrains.com/plugin/1065-checkstyle-idea
Other
890 stars 160 forks source link

Long "checkstyle is scanning" modal popup when committing non-checked files. #526

Open TWiStErRob opened 3 years ago

TWiStErRob commented 3 years ago

We have a hybrid Java & Kotlin Android app project. We use Detekt for Kotlin static analysis and PMD/Checkstyle for Java. When committing Kotlin files the Checkstyle popup blocks Android Studio for a long time, and it'll never find anything, because it's not checking Kotlin files. Is there a way to disable this, or is this a bug?

TWiStErRob commented 3 years ago

Of course the toggle is on, shared in source control, for checking Java files on commit to prevent a CI roundtrip. image

jshiell commented 3 years ago

I'll try and have a look when I've a minute, but by guess is that this is because IDEA just gives us source roots - we have no idea what's in them. So we will have to look at every file within the source root and check the file type before we can discard it - we can't just skip something called src/main/kotlin.

Nonetheless, assuming your commits aren't bonkers in size then it seems odd that this would take very long, so there may well be something awry here.

TWiStErRob commented 3 years ago

The project size is bonkers, my guesstimates are 160 modules. 10k classes, 50/50% Java/Kotlin scattered in arc/main/java and src/main/kotlin. So you're right, you can't skip everything in kotlin folder. But I assume you get the list of files about to be committed, which is not a source root, but more specific. Is there a way to tell if each file needs to be checkstyled and skip if there's nothing to check/check only things need checking? Happy to try to get some profiling info if that helps. There's some performance diagnostic plugin for intellij idea from JetBrains.

jshiell commented 3 years ago

Yikes, that's a bit chunky.

Your assumption is correct though - we just grab the list of the files from the check-in panel when we scan before check-in.

How big are the check-ins in question? My presumption is not that big, which makes this all a bit odd and suggests I'm missing something.

One possibility is that it's something around the virtual file locking ... IDEA is very specific these days (it wasn't 5 years ago) about scoping access to file information & content, and the code seems to generate a read action for each file in the set, which might be less than ideal if we have a large number of files and there's a lot of contention for read-locks.

We also don't validate the files until we've gathered our set of files, which is a convenience as we have multiple frontends to gather the filesets, which pass to a single filter ... but could potentially mean we're doing more work than we need to. In past this wasn't a problem, as the chances of a file being filtered out by type were relatively low (the code is ~15 years old at this point), but with Kotlin's ever-increasing share, that may no longer be as true as it was. So there's potential for optimisation there - but at the cost of complexity in the multiple file-gathering frontends.