Improved incremental build #3231
Conversation
| // Just a heads-up: gradle support warned me about systemProperties System.getProperties(). It's really | ||
| // dangerous to just copy over all system properties to a task invocation. You should really be specific about | ||
| // the properties you'd like to expose inside the task, or you might get very strange issues. | ||
| systemProperties = System.getProperties().toMap() as Map<String, Any> |
There was a problem hiding this comment.
What's the right way to pass system properties in command line when run gradle?
gradle run -Dai.djl.offline=true
There are many DJL specific system properties, it's hard for use the list them
There was a problem hiding this comment.
If they are all namespaced like that example, you can copy over the properties that start with ai.djl instead of blanket copying over all System properties. That should be much safer overall, and also avoid copying properties which change frequently and 'break' the incremental builds.
| } | ||
|
|
||
| // write properties | ||
| val propFile = project.projectDir / "build/classes/java/main/native/lib/llama.properties" | ||
| val propFile = file("$path/native/lib/llama.properties") |
There was a problem hiding this comment.
Which one is preferred in gradle: file() or File()?
There was a problem hiding this comment.
Nothing as far as I know, just my personal preference 😄 File would be the constructor, and file() would be the option provided by the Gradle DSL
…s for all tests due to constant changes in the properties
frankfliu
force-pushed
the
improved-incremental-build
branch
from
fac1576 to
caedea6
Compare
|
Thanks for the merge! |
Description
Some Gradle tasks were configured to prevent Gradle from using up-to-date checks (and build caching, if enabled). This meant that some tasks were being executed even if the output was not different, wasting build time.
This PR proposes changes to the build scripts to make the builds more efficient for repeated builds on the same machine. This should save a few seconds, with more improvements if build caching were to be enabled.
Some key highlights:
tasks that share output locations (eg. tasks that downloaded files directly into build/classes) would prevent Gradle from identifying the owning tasks, which disables caching/up-to-date checks. These were split into separate locations and left to the
processResources/jartasks to collect. The property files are now part of the source tree, and the properties are now injected by theprocessResourcestask.the Spotbugs plugin was reconfigured to have separate paths for the main and test code, for the same reasons as above.
tasks that do not declare inputs/outputs or have no outputs also disable the checks. These tasks were updated to include the right inputs/outputs. In the case of the verifyJava task, a new output file was created to let Gradle know when the task was completed successfully previously. Gradle will automatically skip these tasks if inputs do not change, as the output would have remained the same.
properties were kept constant where possible to allow the incremental checks to work. I removed the system properties pass-through that was present in the tests. I could not find why this was added and the tests passed after removal in any case. Feedback is welcome on this.
The improvements were discovered through analyzing the build with Develocity.
This Build Scan is of a second build run after the improvements were implemented. The timeline view shows a breakdown of where time was spent in the build. In this case, as it was performed on a machine that had already run the build task, you can see here that no tasks were executed as they were all up to date.