Skip to content
This repository was archived by the owner on Nov 19, 2024. It is now read-only.

Conversation

@uhafner
Copy link
Member

@uhafner uhafner commented Apr 23, 2022

Improve XStream serialization and memory footprint for coverage instances:

  • Use alias for each type of coverage node.
  • Use custom converter for coverage values.
  • Use custom converter for coverage percentages
  • Use custom converter for coverage per line number mapping
  • Use custom converter for delta coverage
  • Create Coverage instances using a builder
  • Use Flyweight pattern for Coverage instances

The analysis-model XML now is 3.7M rather than 5.7M before.

Bildschirmfoto 2022-05-16 um 23 43 09

After compacting the new mappings of line to coverage the size is even smaller (2.7M):

Bildschirmfoto 2022-05-18 um 09 45 32

In a final step, all 4 new maps and sets have been simplified and we now have 2.3M:

Bildschirmfoto 2022-05-20 um 22 14 22

uhafner added 3 commits April 23, 2022 18:01
- Use alias for each type of coverage node.
- Use custom converter for coverage values.
Otherwise, applying the flyweight pattern will not be possible.
@uhafner uhafner added the enhancement Enhancement of existing functionality label Apr 23, 2022
@codecov
Copy link

codecov bot commented Apr 23, 2022

Codecov Report

Merging #374 (b9ef6ef) into master (71971d7) will increase coverage by 0.49%.
The diff coverage is 90.05%.

@@             Coverage Diff              @@
##             master     #374      +/-   ##
============================================
+ Coverage     72.19%   72.68%   +0.49%     
- Complexity      963      975      +12     
============================================
  Files            81       82       +1     
  Lines          3506     3632     +126     
  Branches        410      432      +22     
============================================
+ Hits           2531     2640     +109     
- Misses          839      848       +9     
- Partials        136      144       +8     
Impacted Files Coverage Δ
...ns/plugins/coverage/model/CoverageBuildAction.java 75.00% <ø> (-2.46%) ⬇️
...ins/plugins/coverage/model/CoveragePercentage.java 86.11% <77.77%> (-13.89%) ⬇️
...kins/plugins/coverage/model/CoverageXmlStream.java 82.19% <82.19%> (ø)
...va/io/jenkins/plugins/coverage/model/Coverage.java 100.00% <100.00%> (ø)
...o/jenkins/plugins/coverage/model/CoverageLeaf.java 100.00% <100.00%> (ø)
...o/jenkins/plugins/coverage/model/CoverageNode.java 93.16% <100.00%> (ø)
.../plugins/coverage/model/CoverageNodeConverter.java 96.36% <100.00%> (+0.44%) ⬆️
...ns/plugins/coverage/model/CoverageTreeCreator.java 94.23% <100.00%> (+0.35%) ⬆️
...nkins/plugins/coverage/model/FileCoverageNode.java 86.66% <100.00%> (ø)
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 71971d7...b9ef6ef. Read the comment docs.

@uhafner uhafner marked this pull request as ready for review April 24, 2022 11:11
@fo-code fo-code self-requested a review May 2, 2022 12:20
Copy link
Collaborator

@fo-code fo-code left a comment

Choose a reason for hiding this comment

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

Looks goot to me! Just one little thing...

xStream.alias("method", MethodCoverageNode.class);
xStream.alias("leaf", CoverageLeaf.class);
xStream.alias("coverage", Coverage.class);
xStream.alias("fraction", Fraction.class);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Fraction is not used for serialization anymore. This alias can be removed.

uhafner added 2 commits May 6, 2022 16:46
…serialization

# Conflicts:
#	plugin/src/main/java/io/jenkins/plugins/coverage/model/Coverage.java
#	plugin/src/main/java/io/jenkins/plugins/coverage/model/CoverageBuildAction.java
#	plugin/src/main/java/io/jenkins/plugins/coverage/model/CoverageNode.java
#	plugin/src/test/java/io/jenkins/plugins/coverage/model/CoverageNodeTest.java
#	plugin/src/test/java/io/jenkins/plugins/coverage/model/CoverageTest.java
#	plugin/src/test/java/io/jenkins/plugins/coverage/model/FileCoverageNodeTest.java
Base automatically changed from change-coverage to master May 14, 2022 13:06
uhafner added 5 commits May 14, 2022 23:01
# Conflicts:
#	plugin/src/main/java/io/jenkins/plugins/coverage/model/CoveragePercentage.java
#	plugin/src/test/java/io/jenkins/plugins/coverage/model/FileCoverageNodeTest.java
@fo-code
Copy link
Collaborator

fo-code commented May 17, 2022

Maybe we should serialize to something like:

<coveragePerLine>33: 1/1, 34: 1/1, 35: 1/1, 48: 1/2, ...</coveragePerLine>

@uhafner Yes, the coverage/line-mapping is the place where a lot of storage can be saved - the other maps used in FileCoverageNode are not relevant for this.
I calculated the storage usage:
This mapping causes 38% of the required storage for the single node representation of a file with 87 lines which have coverage information.

Your idea for the serialization looks like the perfect improvement for this in my opinion.

@uhafner uhafner marked this pull request as draft May 18, 2022 07:49
@uhafner uhafner added the internal Internal changes without user or API impact label May 20, 2022
@uhafner uhafner marked this pull request as ready for review May 20, 2022 20:08
Copy link
Collaborator

@fo-code fo-code left a comment

Choose a reason for hiding this comment

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

Looks good to me! I tested it locally and the required disk space decreased a lot.

@uhafner uhafner merged commit bfaf7e5 into master May 22, 2022
@uhafner uhafner deleted the coverage-serialization branch May 22, 2022 20:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

enhancement Enhancement of existing functionality internal Internal changes without user or API impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants