[MNG-7899] Various memory usage improvements 4

[sebastien-doyon]

https://issues.apache.org/jira/browse/MNG-7899
Merging the getStatus() method to optimize :

• Use the main StringBuilder to append string instead of using a
  separate one
• Use the StringBuilder.append() with index to avoid String.substring(),
  less temporary strings
• Reuse the FileSizeFormat object in the while loop avoiding multiple
  temporary instances creation

To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.

• [ X] I hereby declare this contribution to be licenced under the Apache License Version 2.0, January 2004

• [X ] In any other case, please file an Apache Individual Contributor License Agreement.

[sebastien-doyon]

Memory allocation profiling using ConsoleMavenTransferListener_memalloc_Test.java calling the ConsoleMavenTransferListener class 100 million times.

This test shows a drastic decrease of :

• char[] allocation (before : 190 GiB, after : 85,4 GiB)
• java.lang.String allocation (before : 45,3 GiB, after : 3,8 GiB)
• java.lang.StringBuffer allocation (before : 65,4 GiB, after : 1,14 GiB)
• java.text.DecimalFormat allocation (before : 27,4 GiB, after : none recorded)
• more...

The elapse time during the test execution show a 50% reduction, ~100 seconds for the initial code and ~50 seconds for the optimised code.

Here the JMC capture for the initial code (file available here)

Here the JMC capture for the optimised code (file available here)

To reproduce:

• checkout the MNG-7899-profiling-initial tag
• execute the following commands :

mvn clean verify -Drat.skip=true -DskipTests
mvn test -Drat.skip=true -pl :maven-embedder -Dtest=**/ConsoleMavenTransferListener_memalloc_Test -Dspotless.check.skip

• open the maven-embedder/recording-initial.jfr file with JMC
• checkout the MNG-7899-profiling-optimised tag
• execute the following commands :

mvn clean verify -Drat.skip=true -DskipTests
mvn test -Drat.skip=true -pl :maven-embedder -Dtest=**/ConsoleMavenTransferListener_memalloc_Test -Dspotless.check.skip

• open the maven-embedder/recording-optimized.jfr file with JMC

[gnodet]

@sebastien-doyon unit tests seem broken.
Also and fwiw, this looks like all this work may be broken if we somehow integrate #1279... we may want to investigate on that branch then...

[gnodet]

@sebastien-doyon unit tests seem broken. Also and fwiw, this looks like all this work may be broken if we somehow integrate #1279... we may want to investigate on that branch then...

Also, #1268, #1269 and #1270 really look related to the same goal, I.e. optimize the logging, so I think they should be merged in order to have a better understand of the benefits (even if they could be 3 different commits).

[sebastien-doyon]

@sebastien-doyon unit tests seem broken.

@gnodet yes, I am trying to find the problem, but without access to the surefire report files, it is hard to find the problem. Do you have access or know how to get access to the target directory of a build in github? That would help debug.

Also and fwiw, this looks like all this work may be broken if we somehow integrate #1279... we may want to investigate on that branch then...

I think the PR 1279 is complementary with this PR. The only change in the 1279 PR that would be needed is to add back the MessageBuilder builder(StringBuilder stringBuilder) method to api/maven-api-core/src/main/java/org/apache/maven/api/services/MessageBuilderFactory.java and maven-core/src/main/java/org/apache/maven/internal/impl/DefaultMessageBuilderFactory.java classes.

Also, #1268, #1269 and #1270 really look related to the same goal, I.e. optimize the logging, so I think they should be merged in order to have a better understand of the benefits (even if they could be 3 different commits).

If you think, I was trying to separate unrelated changes so the obvious ones could be merged while those with problems or comments could be worked out separately, like this one. Tell me if you really prefer me to merge in on PR.

[sebastien-doyon]

@gnodet the last commit fixes the unit test that was broken.

[gnodet]

LGTM
There are still a couple of things to clean in this package....

• FileSizeFormat should be moved to top level
• it should be refactored to accept an Appender as argument
• BatchModeMavenTransferListener should be removed
• Slf4jMavenTransferListener should inherit AbstractMavenTransferListener
• AbstractMavenTransferListener uses direct ansi sequences (this will be fixed in [MNG-7995] Switch to JLine to provide line editing #1279)

@sebastien-doyon feel free to work on these issues if you're fancy

[gnodet]

I wonder if the FileSizeFormat should be refactored to be given a StringBuffer to write into, instead of (or in addition to) the current methods.
But this may be for another PR...

[sebastien-doyon]

Done in the last commit

[sebastien-doyon]

@gnodet

There are still a couple of things to clean in this package....

FileSizeFormat should be moved to top level

Done in the last commit

it should be refactored to accept an Appender as argument

Done in the last commit, the FileSizeFormat.formatProcess() was refactored to accept an StringBuilder as argument

BatchModeMavenTransferListener should be removed

Done in the last commit

Slf4jMavenTransferListener should inherit AbstractMavenTransferListener

I leave this one for another PR, I suggest transform the AbstractMavenTransferListener to a message formatter class that would contain the common code building the text, and move to PrintStream code to the ConsoleMavenTransferListener. Maybe in the 1279 PR or a subsequent one.

[sebastien-doyon]

@gnodet a new PR was created for that 1296