-
Notifications
You must be signed in to change notification settings - Fork 843
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
Ignoring unnecessarily generated docker-ready #510
base: master
Are you sure you want to change the base?
Ignoring unnecessarily generated docker-ready #510
Conversation
Though it's true that this folder is not being used during the verification, I don't see any practical benefit in adding this condition, as the performance overhead of copying these two tiny files is negligible. What gain are you expecting? |
In our investigation, we found that with our changes it can make 14% time savings than the original run. |
@davideicardi Your thoughts? |
No strong opinion on my side, but 14% time savings is quite good. |
@davideicardi Would you mind testing it for yourself? Just checkout this PR and build with |
@Bert-R You are right, I don't see any relevant difference. @optimizing-ci-builds How do you have misured this? |
Here are two workflow where we executed this project with and without the PR changes: With proposed change: GitHub Action Run Link The with proposed change build took 3m 25s while the without proposed change build took 4m 7s -- representing an approximately 40-second reduction in overall build time. The with proposed change log also shows the "Build with Maven" step took 1m 4s. While the without proposed change log shows the "Build with Maven" step took 1m 17s. This difference represents a 17% (13 seconds) runtime improvement with the proposed changes. Although there may be some noise in the runtime measurement of each build, we believe the proposed change can help lower the cost of each build most of the time. |
In our analysis, we observe that target/docker-ready is generated but not used in CI. Because the contents of docker-ready/ are not accessed after they are generation, we propose to disable the generation of this directory to save build runtime. The generation of this directory can be disabled by simply adding a property element to the pom.xml or adding an argument to the mvn command. The argument to add to the mvn command is -Dskip.docker.resources=true.