Reduce memory footprint #2460

marcphilipp · 2020-10-24T15:59:20Z

With these changes I was able to go from ~57160 to ~125205 dynamic tests with a -Xmx64m and the sample project provided by @ben-manes in #2450 without compromising any functionality.

I hereby agree to the terms of the JUnit Contributor License Agreement.

Definition of Done

There are no TODOs left in the code
Method preconditions are checked and documented in the method's Javadoc
Coding conventions (e.g. for logging) have been followed
Change is covered by automated tests including corner cases, errors, and exception handling
Public API has Javadoc and @API annotations
Change is documented in the User Guide and Release Notes
All continuous integration builds pass

codecov · 2020-10-24T16:12:53Z

Codecov Report

Merging #2460 (092751f) into main (74e8de7) will increase coverage by 0.06%.
The diff coverage is 98.56%.

@@             Coverage Diff              @@
##               main    #2460      +/-   ##
============================================
+ Coverage     90.46%   90.52%   +0.06%     
- Complexity     4604     4624      +20     
============================================
  Files           402      403       +1     
  Lines         11386    11462      +76     
  Branches        912      919       +7     
============================================
+ Hits          10300    10376      +76     
+ Misses          809      808       -1     
- Partials        277      278       +1

Impacted Files	Coverage Δ	Complexity Δ
...tform/launcher/listeners/LegacyReportingUtils.java	`100.00% <ø> (ø)`	`6.00 <0.00> (ø)`
...it/vintage/engine/descriptor/DescriptionUtils.java	`88.88% <88.88%> (ø)`	`3.00 <3.00> (?)`
...form/engine/support/hierarchical/NodeTestTask.java	`90.09% <94.73%> (+0.29%)`	`29.00 <3.00> (+2.00)`
...r/engine/descriptor/TestFactoryTestDescriptor.java	`95.08% <100.00%> (ø)`	`21.00 <0.00> (ø)`
.../engine/descriptor/TestTemplateTestDescriptor.java	`100.00% <100.00%> (ø)`	`19.00 <1.00> (ø)`
.../main/java/org/junit/platform/engine/UniqueId.java	`81.81% <100.00%> (ø)`	`23.00 <5.00> (+1.00)`
...va/org/junit/platform/launcher/TestIdentifier.java	`97.82% <100.00%> (+2.47%)`	`22.00 <10.00> (+7.00)`
...ain/java/org/junit/platform/launcher/TestPlan.java	`97.77% <100.00%> (+0.10%)`	`17.00 <2.00> (ø)`
...va/org/junit/vintage/engine/VintageTestEngine.java	`100.00% <100.00%> (ø)`	`8.00 <2.00> (+1.00)`
.../vintage/engine/descriptor/TestSourceProvider.java	`96.55% <100.00%> (+0.55%)`	`11.00 <3.00> (+3.00)`
... and 10 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 74e8de7...092751f. Read the comment docs.

sormuras · 2020-10-24T16:39:54Z

About 50% memory savings? Nice!

mmerdes

Looks good to me. Good to have solid test coverage and experimentally measured improvements. Not sure if I oversee all implications. As this touches some core classes another review might be called for.

junit-platform-launcher/src/main/java/org/junit/platform/launcher/TestPlan.java

sbrannen

I think this PR looks good.

Thanks for putting in all the effort!

Also, thanks for pushing individual commits that tell a story of the evolution of this PR.

I only spotted a few places for minor improvements, and I left a question or two.

...atform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/NodeTestTask.java

junit-platform-launcher/src/main/java/org/junit/platform/launcher/TestIdentifier.java

junit-vintage-engine/src/main/java/org/junit/vintage/engine/descriptor/DescriptionUtils.java

...tage-engine/src/main/java/org/junit/vintage/engine/discovery/IsPotentialJUnit4TestClass.java

...gine/src/main/java/org/junit/vintage/engine/discovery/RunnerTestDescriptorPostProcessor.java

In order to allow them to be garbage collected, dynamic test descriptors are no longer added to their parent’s list of children. This is fine because they were never used in that list.

This reduces the memory overhead of each `TestIdentifier` and each `UniqueId`.

Instead of using a `LinkedHashSet` for empty or single-element sets of tags, we now use the immutable empty set for the most common case of no tags and `Collections.singleton` which is more memory efficient for Single tag sets.

The list of segments of a `UniqueId` is immutable. Prior to this commit, we were using an unsized `ArrayList` to store them which was wasteful since the backing array was often too large.

Instead of storing all `Future` instances we now only store those that Are not yet finished.

`Description.getMethod()` evaluates a regex every time it is called which is expensive for large test suites. `DescriptionUtils` now tries computing it without evaluating a regex first and only falls back on `Description.getMethod()` if the display name does not look as expected.

Test sources are now cached by description which avoids the overhead of creating multiple instances for the same method, e.g. for parameterized tests or generative test suites that include the same class multiple times with different configurations.

This eliminates allocation overhead for the most common case where a `Description` represents a test, i.e. does not have children.

RunnerTestDescriptors can now be garbage collected after they have been executed because they are now removed from the list of children of the engine descriptor.

Prior to this commit each `TestIdentifier` instance required memory for storing a `null` reference.

marcphilipp linked an issue Oct 24, 2020 that may be closed by this pull request

Executing large number of dynamic tests results in OutOfMemoryError: GC overhead limit exceeded #1445

Closed

mmerdes self-requested a review November 30, 2020 08:58

mmerdes approved these changes Nov 30, 2020

View reviewed changes

junit-platform-launcher/src/main/java/org/junit/platform/launcher/TestPlan.java Show resolved Hide resolved

marcphilipp added this to the 5.7.1 milestone Dec 22, 2020

marcphilipp force-pushed the issues/1445-reduce-memory-footprint branch 2 times, most recently from c716328 to 90dd6fd Compare December 30, 2020 22:02

marcphilipp modified the milestones: 5.7.1, 5.8 M1 Dec 31, 2020

marcphilipp force-pushed the issues/1445-reduce-memory-footprint branch from 90dd6fd to c5efbbd Compare December 31, 2020 12:00

marcphilipp requested a review from sbrannen December 31, 2020 12:00

marcphilipp marked this pull request as ready for review December 31, 2020 12:01

marcphilipp force-pushed the issues/1445-reduce-memory-footprint branch 2 times, most recently from 5ff7537 to 266e8f7 Compare December 31, 2020 12:37

sbrannen requested changes Jan 5, 2021

View reviewed changes

marcphilipp requested a review from sbrannen January 7, 2021 15:02

sbrannen approved these changes Jan 8, 2021

View reviewed changes

marcphilipp added 12 commits January 8, 2021 20:11

Jupiter: Don't retain dynamic TestDescriptors

6abaa4f

In order to allow them to be garbage collected, dynamic test descriptors are no longer added to their parent’s list of children. This is fine because they were never used in that list.

Platform: Don't store String representation of unique IDs

3ad676d

This reduces the memory overhead of each `TestIdentifier` and each `UniqueId`.

Platform: Link to undeprecated LegacyReportingUtils

afe01a4

Platform: Avoid large number of empty LinkedHashSets

f06491f

Instead of using a `LinkedHashSet` for empty or single-element sets of tags, we now use the immutable empty set for the most common case of no tags and `Collections.singleton` which is more memory efficient for Single tag sets.

Platform: Avoid wasting memory from unnecessarily large ArrayLists

08d2f89

The list of segments of a `UniqueId` is immutable. Prior to this commit, we were using an unsized `ArrayList` to store them which was wasteful since the backing array was often too large.

Platform: Avoid storing all returned futures

916732d

Instead of storing all `Future` instances we now only store those that Are not yet finished.

Vintage: Avoid allocation for descriptions without children

be2921f

This eliminates allocation overhead for the most common case where a `Description` represents a test, i.e. does not have children.

Vintage: Allow garbage collecting executed runners

cfc73e5

RunnerTestDescriptors can now be garbage collected after they have been executed because they are now removed from the list of children of the engine descriptor.

Platform: Avoid SerializedForm reference

b1558b9

Prior to this commit each `TestIdentifier` instance required memory for storing a `null` reference.

Document memory improvements in release notes

092751f

marcphilipp force-pushed the issues/1445-reduce-memory-footprint branch from fad47fd to 092751f Compare January 8, 2021 19:27

marcphilipp merged commit 0be0e78 into main Jan 8, 2021

marcphilipp deleted the issues/1445-reduce-memory-footprint branch January 8, 2021 19:35

marcphilipp removed this from the 5.8 M1 milestone Jan 8, 2021

ben-manes mentioned this pull request Oct 2, 2022

Extremely slow for large vintage test suites #3050

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reduce memory footprint #2460

Reduce memory footprint #2460

marcphilipp commented Oct 24, 2020

codecov bot commented Oct 24, 2020 •

edited

Loading

sormuras commented Oct 24, 2020

mmerdes left a comment

sbrannen left a comment

Reduce memory footprint #2460

Reduce memory footprint #2460

Conversation

marcphilipp commented Oct 24, 2020

Definition of Done

codecov bot commented Oct 24, 2020 • edited Loading

Codecov Report

sormuras commented Oct 24, 2020

mmerdes left a comment

Choose a reason for hiding this comment

sbrannen left a comment

Choose a reason for hiding this comment

codecov bot commented Oct 24, 2020 •

edited

Loading