Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Gradle Worker API #2903

Open
vmishenev opened this issue Mar 7, 2023 · 8 comments
Open

Use Gradle Worker API #2903

vmishenev opened this issue Mar 7, 2023 · 8 comments
Labels
enhancement An issue for a feature or an overall improvement feedback: Kotlin libs Feedback from Kotlin's internal libraries runner: Gradle plugin An issue/PR related to Dokka's Gradle plugin

Comments

@vmishenev
Copy link
Member

WIP

Motivation

  1. Currently, Dokka reloads a big classpath for every task. Also, it has reflection magic to support different versions of plugins.
  2. Dokka works parallel badly. For example, the run time on on a project with 100 tasks is 8 minutes.

Proposal

Gradle Worker API can help Dokka to avoid it. But we have 2 options for using it:

  1. Use cached classpath and noIsolation mode (to keep a classpath in a static variable that is shared between tasks). This approach is used in Kapt.
  2. Use processIsolation mode. Dokka task will be executed in worker daemon processes. The running processes with the same classpath can be reused for other tasks. In this case, the classpath is loaded once per process.

We need to choose only one option.
We already have prototypes. @aSemy created one for the second option and there is a prototype for the cached classpath here.

Pros&cons

  • From a performance point of view, processIsolation requires time to run worker processes (default is number of CPU processors) and load classpath in each process. However, if daemons are already running, the time of Dokka executing is the same as for cached classpath.
    My observations:
    Coroutines (precompilied): ~35s (cached classpath) vs ~66s (processIsolation with 8 workers)
    Small project with 100 tasks : ~20s (cached classpath) vs ~60s (processIsolation with 8 workers)
    My hypothesis: the difference in execution time between the two approaches is a constant (for my computer with 8 workers the difference is ~40 sec) to run processes and load classpathes. But the constant depends on the number of workers (e.g. coroutines --max-workers=2 takes 44 sec).
    Also, we can adjust the number of workers (depending on a number of tasks) to get more performance.

  • Stability. noIsolation needs synchronization for a static state. For example, we have a data race with static properties on the IDE part. Also, Kapt experienced OOM issues. Since we use external libraries (and the compiler) and Dokka is not designed for a multithreading environment, some such kinds of problems can appear.

  • Library compatibility. The Gradle documentation says: "External libraries may rely on certain system properties to be set which may conflict between work items. Or a library might not be compatible with the version of JDK that Gradle is running with and may need to be run with a different version. ". But I am not sure that is relevant for our external libraries.

There are other points for choosing an approach. Everybody can share their opinion about it here.

To sum up, I personally vote for the first option. In the case of Dokka, we will increase little bit of time building but stability is more important than performance.

@vmishenev vmishenev added enhancement An issue for a feature or an overall improvement feedback: Kotlin libs Feedback from Kotlin's internal libraries labels Mar 7, 2023
@IgnatBeresnev IgnatBeresnev added the runner: Gradle plugin An issue/PR related to Dokka's Gradle plugin label Mar 8, 2023
@aSemy
Copy link
Contributor

aSemy commented Mar 8, 2023

There's another isolation mode too, classLoaderIsolation(). It's a halfway point between the other two, and I think it's worth considering. I strongly suspect that it would be the most performant of the 3 options.

One benefit to processIsolation is that it prevents problems with coroutines, and so hopefully it would make finalizeCoroutines option obsolete.

/**
* Whether coroutines dispatchers should be shutdown after
* generating documentation via [DokkaGenerator.generate].
*
* It effectively stops all background threads associated with
* coroutines in order to make classes unloadable by the JVM,
* and rejects all new tasks with [RejectedExecutionException]
*
* This is primarily useful for multi-module builds where coroutines
* can be shut down after each module's partial task to avoid
* possible memory leaks.
*
* However, this can lead to problems in specific lifecycles where
* coroutines are shared and will be reused after documentation generation,
* and closing it down will leave the build in an inoperable state.
* One such example is unit tests, for which finalization should be disabled.
*/
val finalizeCoroutines: Boolean

I think that whatever option is chosen, I think #2740 will be broadly the same.

@TWiStErRob
Copy link
Contributor

@aSemy What is the reason for Dokka tasks executing one by one, one after the other in a Gradle parallel build? When I do a publish I can see compilation and all other tasks complete very quickly, then all workers (28+) are busy with different Dokka tasks from submodules. The console logs are suggesting that one needs to complete before the next one can start.

@aSemy
Copy link
Contributor

aSemy commented Jul 25, 2023

@aSemy What is the reason for Dokka tasks executing one by one, one after the other in a Gradle parallel build? When I do a publish I can see compilation and all other tasks complete very quickly, then all workers (28+) are busy with different Dokka tasks from submodules. The console logs are suggesting that one needs to complete before the next one can start.

DGP (Dokka Gradle Plugin) is not compatible with many Gradle features like project isolation, build cache, configuration cache - see #2700 - so tasks run sequentially, even across different subprojects. And DGP does not use the Worker API at the moment (hence this issue and my PR #2740).

If you want to generate Dokka docs faster then look at Dokkatoo. Dokkatoo is a re-implemented DGP that supports all the speedy Gradle features. Dokkatoo is not a drop-in replacement for DGP, but it's pretty similar, and you run add both DGP and Dokkatoo in the same project to verify that the output of both is identical.

(Funny that you've pinged me (just a contributor) rather than the actual maintainers @IgnatBeresnev and @vmishenev 😄)

@TWiStErRob
Copy link
Contributor

I pinged you exactly because of all those issues, PRs and repo you linked. I know DGP is not compatible with fancy new features, but org.gradle.parallel is a pretty pretty old feature. I'm wondering what is locking inside Dokka that doesn't allow basic parallelism.

@aSemy
Copy link
Contributor

aSemy commented Jul 25, 2023

I pinged you exactly because of all those issues, PRs and repo you linked. I know DGP is not compatible with fancy new features, but org.gradle.parallel is a pretty pretty old feature. I'm wondering what is locking inside Dokka that doesn't allow basic parallelism.

Ah okay. Hmm, I'm not sure I can give a definitive answer because I'm not exactly sure how --parallel works and what the requirements are, and precisely what DGP is doing that would prevent it, but these items in particular are related:

@TWiStErRob
Copy link
Contributor

Thanks for the pointers!

@IgnatBeresnev IgnatBeresnev added this to the Gradle runner 2.0 milestone Aug 17, 2023
@martinbonnin
Copy link
Contributor

+1 for classloaderIsolation or at least making the isolation configurable. In addition to performance, classloaderIsolation makes it way easier to debug the builds.

@adam-enko
Copy link
Contributor

One potential benefit of process isolation is that it would allow for class data sharing. With a multimodule project, Dokka Generator has to run multiple times with the same classpath. The Dokka classpath can be quite large (the analyzer component is ~80MB). Using CDS would mean that the classes could be 'cached' between generations, improving startup time and reducing memory usage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An issue for a feature or an overall improvement feedback: Kotlin libs Feedback from Kotlin's internal libraries runner: Gradle plugin An issue/PR related to Dokka's Gradle plugin
Projects
None yet
Development

No branches or pull requests

6 participants