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 MAVEN build Performance #29

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ChenZhangg
Copy link

Parallel builds in Maven 3 Maven 3.x has the capability to perform parallel builds.

According to Maven parallel test, we can run tests in parallel.

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

@arso
Copy link
Contributor

arso commented Nov 30, 2021

hi @ChenZhangg. Many thanks for looking into this area which, I think, needs some more love :)
Before we run the tests, would it be possible for you to share your findinds about build performance gains? Ie. what is the difference in build time (what type of hardware was it tested on).
The reason I am asking is the fact that the full build (including mutliple user agents for UI (gwt) is very memory demanding and I prefer to be careful not to break other building flows.

@sandrobonazzola
Copy link
Member

@ChenZhangg can you please follow up to @arso comment?

Copy link
Contributor

@arso arso left a comment

Choose a reason for hiding this comment

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

I have tested that change locally and it seem ok - personally I has been running builds with '-T1C' (one thread per core) option for more than a year now. This is enabled here for travis and I see that checks are passing which means that it is safe to merge it.
I have left few comments regarding formatting. Once these are resolved I am happy to give my +1

@@ -856,6 +864,9 @@
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
<testFailureIgnore>true</testFailureIgnore>
<parallel>classes</parallel>
Copy link
Contributor

@arso arso Apr 8, 2022

Choose a reason for hiding this comment

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

Please fix formatting to match the line above - use 2 instead of 1 space. Same for the line below

@@ -844,6 +847,11 @@
<plugins>
<plugin>
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
<parallel>classes</parallel>
Copy link
Contributor

@arso arso Apr 8, 2022

Choose a reason for hiding this comment

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

Please fix formatting to match the line above - use 2 instead of 1 space. Same for the line below

@@ -619,6 +619,9 @@
<configuration>
<reportsDirectory>${ovirt.surefire.reportsDirectory}</reportsDirectory>
<argLine>--illegal-access=permit</argLine>
<parallel>classes</parallel>
Copy link
Contributor

@arso arso Apr 8, 2022

Choose a reason for hiding this comment

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

Please fix formatting to match the line above - use 2 instead 3 spaces. Same for the line below

@sandrobonazzola
Copy link
Member

@ChenZhangg can you please follow up?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants