search

obsidiandynamics / kafdrop

Kafka Web UI
Apache License 2.0
5.56k stars 841 forks source link

Ignoring unnecessarily generated docker-ready #510

Open optimizing-ci-builds opened 1 year ago

optimizing-ci-builds commented 1 year ago

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.

Bert-R commented 1 year ago

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?

optimizing-ci-builds commented 1 year ago

In our investigation, we found that with our changes it can make 14% time savings than the original run.

Bert-R commented 1 year ago

@davideicardi Your thoughts?

davideicardi commented 1 year ago

No strong opinion on my side, but 14% time savings is quite good. If there aren't drawbacks why not! We will save some CO2 :grin:

Bert-R commented 1 year ago

@davideicardi Would you mind testing it for yourself? Just checkout this PR and build with -Dskip.docker.resources=true and -Dskip.docker.resources=false and compare the times. I didn't notice any difference, so then I felt extra complexity is not justified.

davideicardi commented 1 year ago

@Bert-R You are right, I don't see any relevant difference. @optimizing-ci-builds How do you have misured this?

optimizing-ci-builds commented 1 year ago

Here are two workflow where we executed this project with and without the PR changes:

With proposed change: GitHub Action Run Link Without 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.