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

Improve GRADLE build Performance #5822

Closed
wants to merge 1 commit into from

Conversation

shisheng-1
Copy link

Compiler daemon. The Gradle Java plugin allows you to run the compiler as a separate process by setting options.fork = true. This feature can lead to much less garbage collection and make Gradle’s infrastructure faster. This project has more than 1000 source files. We can consider enabling this feature.

=====================
If there are any inappropriate modifications in this PR, please give me a reply and I will change them.

@boring-cyborg
Copy link

boring-cyborg bot commented Nov 12, 2021

Thanks for opening this pull request!

Please check out our contributor checklist and check if Travis or Codacy found any issues with your PR. Also make sure your commits are signed, and that you applied Bisq's code style and formatting.

A maintainer will add an is:priority label to your PR if it is up for compensation. Please see our Bisq Q1 2020 Update post for more details.

@ripcurlx ripcurlx requested a review from cbeams November 12, 2021 08:20
@ripcurlx
Copy link
Contributor

@cbeams Any chance that you could look at this small Gradle change, if it makes sense for our setup? Thanks!

Copy link
Member

@cbeams cbeams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NACK. The change does not appear to improve build times; it rather appears to slow them down.

Testing approach

I ran the following command three times each on HEAD of current master (A) and with this PR's patch applied (B).

$ time ./gradlew cleanCompileJava cleanCompileTestJava compileJava compileTestJava

Note that I had run several builds against the same daemon to fully warm it up before I started recording any of the runs.

Results

A

  1. 1m 50s
  2. 2m 05s
  3. 1m 57s

B

  1. 2m 18s
  2. 2m 08s
  3. 2m 05s

So: at best, the build times in B could match A, but generally they were a bit longer. In fairness, the longest time (build B.1) of 2m 18s may be explained by the overhead necessary to spin up the compiler daemon for the first time. In any case, the results are not compelling.

Notes

In future PRs, please do this kind of testing yourself and add notes like the above to demonstrate whether the improvement actually works. Thanks in any case.

@cbeams
Copy link
Member

cbeams commented Nov 12, 2021

A bit more detail on why this change doesn't make a difference:

@shisheng-1 wrote:

This project has more than 1000 source files.

True, but the devil is in the details. Per the feature's documentation:

It’s unlikely to be useful for small projects, but you should definitely consider it if a single task is compiling close to a thousand or more source files together.

There are just over 3000 source files across all Bisq subprojects, but no one subproject has more than 1000:

$ for i in $(echo */src */build); do echo -n "$i: "; find $i -name '*.java' | wc -l; done | grep -v ': 0'; echo -n 'TOTAL: '; find */src */build -name '*.java' | wc -l;
apitest/src: 74
assets/src: 264
cli/src: 88
common/src: 106
core/src: 958
daemon/src: 20
desktop/src: 530
inventory/src: 3
monitor/src: 27
p2p/src: 166
pricenode/src: 61
relay/src: 2
seednode/src: 3
statsnode/src: 2
proto/build: 754
TOTAL: 3058

So no single compileJava task is compiling over 1000 files, and certainly not most of them, therefore the overhead of the compiler daemon doesn't pay off and indeed becomes slightly counterproductive to build times.

@ripcurlx
Copy link
Contributor

@cbeams Thanks for the quick investigation! I'm closing this PR based on #5822 (review).

@ripcurlx ripcurlx closed this Nov 12, 2021
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 this pull request may close these issues.

None yet

3 participants