The performance problem should now be fixed. I can currently see no more heavy computations on my machine when using the profiler, but I encourage you to test this on your machines as well (@CedNaru, I'd be particularly grateful if you could test it on your machine, as you could reproduce the issue previously).
To test this, check out the project and run gradlew buildPlugin. Then from the IDE, navigate to the plugins window, click the gear icon and select Install plugin from disk. Then choose the zip file located under kt/plugins/godot-intellij-plugin/build/distributions.
Other small fixes
The core type check threw some exceptions as i tried to apply the error to a "most relevant" parent. But in some cases this parent was "out of scope" for the error annotation and thus throwing an exception. I now just add the error where it actually happens. Which in the case of core type checks can mean that the error is reported more than once in one line (which is technically correct anyways).
I also corrected the corresponding error message.
Explanation of the Performance Issue
The root cause of the performance issue was the detection process used to determine if we were operating within a Godot project. Ironically, this check was in place to essentially disable our checks for non-Godot projects - it mainly caused problems for non-Godot projects (almost instantly causing issues) and, depending on the machine and project complexity, for Godot projects as well.
In our main plugin service file, we have a GodotRoot cache which stores for each IntelliJ module (not to be confused with a Gradle module!) if it resides in a Godot root. To determine this, we query Gradle for each IntelliJ module to see if it's within a Gradle project. If it is, we acquire the root directory of that Gradle project and search for a project.godot file in it. If we find one, we assume the project is a Godot project. If not, we essentially disable all checks in our plugin by returning instantly in each check we conduct.
Three problems in this check essentially lead to these performance issues
First, we cached the result of this expensive computation only if we found a Godot root. If we did not (which is the case for all non-Godot projects, for which this check was put in place), we did not store the result. Hence, whenever a check was called, the entire computation was conducted again, impacting all non-Godot projects.
Second, we used kotlin.io's FileWalk to search for a Godot project. This is a depth-first search and was conducted for each check, meaning we searched every file in the project depth-first. What worsened the matter is that in most Godot Kotlin JVM projects, the project.godot file is located more or less at the "bottom" of the root Gradle directory. So, in this depth-first search, we searched all directories and all files in them recursively with no beneficial outcome in most cases, as the file in question is at the root level below those directories we just searched in most godot kotlin jvm projects.
To solve this issue, I’ve implemented a breadth-first search. In addition, we now start by searching in the Gradle root, as most of the time, a recursive search is not necessary.
The last problem was that this function was being called from hundreds of threads and coroutines (for each check more or less). As the function took some time to complete and the cache was only filled after the computation completed, the file search was triggered tens of thousands of times, hence we ended up having tens of thousands of recursive file searches which obviously slowed them down exponentially .
To tackle this, I have adjusted the operation to populate the cache early with the value "no Godot root found", and after the computation is completed, set the definitive result in the cache. I did not want to utilize a lock in this case as this function also gets called frequently by the main thread. As cache accesses happen very frequently, the initially possible incorrect value of "not a Godot root" found doesn’t matter much. Furthermore, based on my profiling, the function now rarely has any race conditions where it's triggered more than once. And even if it does, there are never more than a single digit count of processes calculating this.
Fixes #520
TL;DR
The performance problem should now be fixed. I can currently see no more heavy computations on my machine when using the profiler, but I encourage you to test this on your machines as well (@CedNaru, I'd be particularly grateful if you could test it on your machine, as you could reproduce the issue previously).
To test this, check out the project and run
gradlew buildPlugin
. Then from the IDE, navigate to the plugins window, click the gear icon and selectInstall plugin from disk
. Then choose the zip file located underkt/plugins/godot-intellij-plugin/build/distributions
.Other small fixes
The core type check threw some exceptions as i tried to apply the error to a "most relevant" parent. But in some cases this parent was "out of scope" for the error annotation and thus throwing an exception. I now just add the error where it actually happens. Which in the case of core type checks can mean that the error is reported more than once in one line (which is technically correct anyways). I also corrected the corresponding error message.
Explanation of the Performance Issue
The root cause of the performance issue was the detection process used to determine if we were operating within a Godot project. Ironically, this check was in place to essentially disable our checks for non-Godot projects - it mainly caused problems for non-Godot projects (almost instantly causing issues) and, depending on the machine and project complexity, for Godot projects as well.
In our main plugin service file, we have a GodotRoot cache which stores for each IntelliJ module (not to be confused with a Gradle module!) if it resides in a Godot root. To determine this, we query Gradle for each IntelliJ module to see if it's within a Gradle project. If it is, we acquire the root directory of that Gradle project and search for a
project.godot
file in it. If we find one, we assume the project is a Godot project. If not, we essentially disable all checks in our plugin by returning instantly in each check we conduct.Three problems in this check essentially lead to these performance issues
First, we cached the result of this expensive computation only if we found a Godot root. If we did not (which is the case for all non-Godot projects, for which this check was put in place), we did not store the result. Hence, whenever a check was called, the entire computation was conducted again, impacting all non-Godot projects.
Second, we used
kotlin.io
'sFileWalk
to search for a Godot project. This is a depth-first search and was conducted for each check, meaning we searched every file in the project depth-first. What worsened the matter is that in most Godot Kotlin JVM projects, theproject.godot
file is located more or less at the "bottom" of the root Gradle directory. So, in this depth-first search, we searched all directories and all files in them recursively with no beneficial outcome in most cases, as the file in question is at the root level below those directories we just searched in most godot kotlin jvm projects.To solve this issue, I’ve implemented a breadth-first search. In addition, we now start by searching in the Gradle root, as most of the time, a recursive search is not necessary.
The last problem was that this function was being called from hundreds of threads and coroutines (for each check more or less). As the function took some time to complete and the cache was only filled after the computation completed, the file search was triggered tens of thousands of times, hence we ended up having tens of thousands of recursive file searches which obviously slowed them down exponentially .
To tackle this, I have adjusted the operation to populate the cache early with the value "no Godot root found", and after the computation is completed, set the definitive result in the cache. I did not want to utilize a lock in this case as this function also gets called frequently by the main thread. As cache accesses happen very frequently, the initially possible incorrect value of "not a Godot root" found doesn’t matter much. Furthermore, based on my profiling, the function now rarely has any race conditions where it's triggered more than once. And even if it does, there are never more than a single digit count of processes calculating this.