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

LUCENE-10195: Improve Gradle build speed #414

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

jprinet
Copy link
Contributor

@jprinet jprinet commented Oct 26, 2021

Description

Improve Gradle build speed by mainly focussing on up-to-date checks and task caching

Solution

Using Gradle Enterprise to identify room for improvements

Tests

nightly and regression tests

@mikemccand
Copy link
Member

@jprinet -- thank you for this PR, and sorry for the insanely slow response. Is this still relevant/helpful? I don't like how slow our gradle builds are, so if we can make it faster, that'd be awesome. But I am far from a gradle expert so cannot really evaluate the change... hopefully other Lucene devs can chime in?

@dweiss
Copy link
Contributor

dweiss commented Nov 2, 2023

I think some of it has been integrated already. If not, I'll take a look and go through the changes @jprinet made. It's a shame it took so long, apologies, @jprinet !

I don't like how slow our gradle builds are, so if we can make it faster, that'd be awesome.

Are they? What in particular is slow for you, Mike? There's tons of stuff that runs on (full) check across all submodules, for example. I don't think it's avoidable. In majority of cases, you can narrow down the task to a subproject and it runs very quickly for me.

@jprinet
Copy link
Contributor Author

jprinet commented Nov 2, 2023

As far as I remember, all the relevant changes were integrated in:
#421

See the Jira issue for more context:
https://issues.apache.org/jira/browse/LUCENE-10195

This PR should probably just be closed.

@clayburn
Copy link
Contributor

clayburn commented Nov 2, 2023

I agree with @jprinet that the PR should probably be closed just due to it's age. Many of the changes here deal with caching, with the Lucene project explicitly opts out of by default. If the project wanted to enable caching, then we could revisit these changes, or other potential cache misses, but I see a pretty strong stance against the feature in the relevant JIRA tickets.

@mikemccand - If you are interested, ge.apache.org is available to the Lucene project. Currently, it is all CI builds, but you can opt in to this for your local builds as an ASF committer using your ASF LDAP credentials and provisioning an access key. If you did that and produced build scans for some of the slow builds you see, we would be happy to offer advice.

@mikemccand
Copy link
Member

@mikemccand - If you are interested, ge.apache.org is available to the Lucene project. Currently, it is all CI builds, but you can opt in to this for your local builds as an ASF committer using your ASF LDAP credentials and provisioning an access key. If you did that and produced build scans for some of the slow builds you see, we would be happy to offer advice.

Whoa, I did not even know about this service! Thanks for the pointer @clayburn

@mikemccand
Copy link
Member

I don't like how slow our gradle builds are, so if we can make it faster, that'd be awesome.

Are they? What in particular is slow for you, Mike? There's tons of stuff that runs on (full) check across all submodules, for example. I don't think it's avoidable. In majority of cases, you can narrow down the task to a subproject and it runs very quickly for me.

Well, maybe I am too impatient. I don't have hard data to prove they are slow. But if I watch top while running toplevel ./gradlew test I am disappointed at how much concurrency is left on the table.

I do see lots of concurrency (many java processes) when the actual unit testing seems to start, which is great.

With my lucene clone already fully compiled, the gradle build seems to take a few (~3) seconds to confirm everything is up-to-date, printing out lines like > Task :lucene:benchmark:jar UP-TO-DATE . This is on a modern Raptor Lake Intel CPU.

So yeah I retract my statement :) It's not clearly slow. ./gradlew test finishes in 56s for me (on fully built clone). I remember times in the past when this was much longer ... thanks @Dawid and @clayburn.

@Dawid
Copy link

Dawid commented Nov 4, 2023

TLDR;
No problemo, Mickey. You can always count on me, just like last Friday when we all get wasted and I had to Uber you home. Take care!

@dweiss
Copy link
Contributor

dweiss commented Nov 4, 2023

This is the problem with github at mentions, @mikemccand - whoever this is that had to drive you home, it wasn't me...

@dweiss
Copy link
Contributor

dweiss commented Nov 4, 2023

Anyway - it's going to be difficult to saturate your CPU with tests alone, especially on a beefy machine. We limit the number of forked test JVMs - this you could tweak - but you'll soon hit memory bandwidth and I/O bandwidth limits (sooner than CPU usage limits, I think).

This said, sure - there's room for improvement. For example, we used to have test balancing (reordering to try to saturate forked jvm queues) - I don't think this is possible with gradle's public APIs. There's also more interesting outstanding issues that never got solved, like this one that was closed and swept under the rug, but in fact is still an issue:

gradle/gradle#11609 (comment)

@Dawid
Copy link

Dawid commented Nov 4, 2023

This is the problem with github at mentions, @mikemccand - whoever this is that had to drive you home, it wasn't me...

Dawid, please don't treat it as problem, but as a miracle/opportunity. Mike from the other side of the Atlantic Ocean (6300km from us!) just summoned me mistakenly, but... we are both named Dawid, we are both from Poznan, we both bike, we are both in our 40-ties. You are located at Bóżnicza Street (or at least your wife is), I'm located at Mostowa Street which is few hundred meters away. I bet we both curse ZDM for recent Garbary/Małe Garbary road works. I even Google-street-viewed Bóżnicza earlier today to check how the new Stara Rzeźnia will look from this side. We also have few common friends from Allegro.

That'a crazy Mike what you did here. ;)

@dweiss
Copy link
Contributor

dweiss commented Nov 5, 2023

we are both named Dawid, we are both from Poznan, we both bike, we are both in our 40-ties. You are located at Bóżnicza Street (or at least your wife is),

What are the odds, eh? Fairly spooky!

@mikemccand
Copy link
Member

That'a crazy Mike what you did here. ;)

LOL!! This is too crazy.

I admit to being a little too confident in GitHub's autosuggest when I @ someone. I'm not sure why on this particular day it decided to suggest you @Dawid, but, wow what fun as a result!

Copy link

github-actions bot commented Jan 9, 2024

This PR has not had activity in the past 2 weeks, labeling it as stale. If the PR is waiting for review, notify the dev@lucene.apache.org list. Thank you for your contribution!

@github-actions github-actions bot added the Stale label Jan 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants