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

reportScoverage without extra JVM process #75

Open
dsilvasc opened this issue May 8, 2018 · 8 comments · May be fixed by #170
Open

reportScoverage without extra JVM process #75

dsilvasc opened this issue May 8, 2018 · 8 comments · May be fixed by #170

Comments

@dsilvasc
Copy link

dsilvasc commented May 8, 2018

The reportScoverage task spawns a separate JVM process. Would it be possible to run it within the gradle process?

For a gradle project with many sub-projects, spawning a JVM per subproject for test coverage reports ends up slowing down the build with the JVM startup time and with a cold JIT for each subproject report. Keeping the reporting task within the gradle process could shave about 1.5sec per subproject.

> Task :testing:reportScoverage
...
Starting process 'command '/Library/Java/JavaVirtualMachines/jdk1.8.0_161.jdk/Contents/Home/bin/java''. ... Command: /Library/Java/JavaVirtualMachines/jdk1.8.0_161.jdk/Contents/Home/bin/java ... org.scoverage.SingleReportApp ...
[scoverage] Generating scoverage reports...
...
:testing:reportScoverage (Thread[Task worker for ':' Thread 3,5,main]) completed. Took 1.531 secs.
@maiflai
Copy link
Contributor

maiflai commented May 8, 2018

Sorry, I think this isn't easy.

The report task requires Scala. I think running in the Gradle process would load the Scala 2.12 runtime libraries and then lock all build plugins to the same Scala version.

Running as a child JVM is a trivial way to isolate this.

Suggestions welcome though,
Stu.

@dsilvasc
Copy link
Author

I think that's how gradle-scalastyle-plugin works, so Gradle projects with scalastyle checks already have the Scala runtime loaded in the gradle process. Maybe it could be an option for those projects to also embed the scoverage reporter?

@eyalroth
Copy link
Contributor

I believe this was implemented in 3.0.0.

@netvl
Copy link

netvl commented Sep 3, 2021

While loading Scala into the main process is exactly how this plugin behaves, it is unfortunately a very bad idea. Because this way there is no static identifier of the Scala version being used in the Gradle classpath, it means that it is very easy to get class conflicts if you use another Gradle plugin which depends on Scala, and its Scala version is different from SCoverage's.

Gradle uses JVM forking very extensively. Its model of test execution, for example, is entirely reliant on forking. Ideally, IMO, SCoverage should take advantage of it. Specifically, tests and other stuff use workers to run code in separate threads; it would probably make sense for SCoverage to use the same idea too, I think.

@eyalroth
Copy link
Contributor

eyalroth commented Sep 4, 2021

@netvl The plugin has no direct runtime dependency on Scala (only for compilation), and it doesn't add any runtime dependency other than the scalac plugin, which in turn also doesn't add any dependency.

The plugin also has no test tasks of its own (only report/aggregation tasks which are fairly light), and allows for forking on tests like in any other build.

We have cross scala version tests for the project, which also test a scenario of a multi-module project with one module in scala 2.12 and the other in 2.13, and the report/aggregation on the entire project works with no additional/special configuration.

@netvl
Copy link

netvl commented Sep 7, 2021

@eyalroth Unless I'm missing something, the logic here adds the SCoverage classes to the "current thread" class loader. This runner is invoked from various tasks directly. This means that it will modify the classloader configuration of whatever thread executes the task, which by default is one of the Gradle's primary process threads. So even if SCoverage is not present as a direct dependency via the plugin artifact dependencies, it nevertheless appears in the Gradle process classpath eventually. I would argue that doing such reflection-based classloader manipulation is very confusing. At least I was very confused when I saw classic Scala compatibility errors without any competing Scala versions in the ./gradlew buildEnvironment report, and disabling SCoverage or aligning the Scala versions solved the problem.

eyalroth added a commit to eyalroth/gradle-scoverage that referenced this issue Sep 11, 2021
…'s classpath to the main gradle classloader
@eyalroth eyalroth linked a pull request Sep 11, 2021 that will close this issue
eyalroth added a commit to eyalroth/gradle-scoverage that referenced this issue Sep 11, 2021
…'s classpath to the main gradle classloader
@eyalroth
Copy link
Contributor

@netvl I totally forgot about this code/functionality.

I'm seeing that Gradle added the worker API in Gradle 6 which provides a means to run a task in an isolated classpath/classloader; however, it seems to be problematic. See #170 for more information.

@dsilvasc
Copy link
Author

another potential approach: #80

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants