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

feat: add memory metrics to acceptance tests #1874

Merged
merged 35 commits into from
Oct 16, 2024
Merged
Changes from 1 commit
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
f16f52e
add memory usage records to the JSON report
davidgamez Sep 30, 2024
3efa591
downgrade aspectj dependecies to be compatible with jdk 11
davidgamez Sep 30, 2024
5f6c71b
add memory usage to validator comparator
davidgamez Oct 1, 2024
848d798
run acceptance tests with sample data
davidgamez Oct 1, 2024
e89816c
fix memory usage serialization
davidgamez Oct 2, 2024
b66828f
fix performance collector
davidgamez Oct 2, 2024
0416d1c
fix npe
davidgamez Oct 2, 2024
0cc1833
support negative memory usage for logging
davidgamez Oct 2, 2024
f6789c8
simplifly memory usage report
davidgamez Oct 2, 2024
e59614c
Merge branch 'master' into chore/memory-monitor
davidgamez Oct 2, 2024
3efd975
fix compilation issue
davidgamez Oct 2, 2024
39182dd
add feeds with no reference
davidgamez Oct 3, 2024
06fe749
add no references to the report
davidgamez Oct 3, 2024
a70accf
fix failing tests
davidgamez Oct 3, 2024
b10320d
fix memory table formatting
davidgamez Oct 3, 2024
97678ee
sort feeds on the no reference list and limit them to 25 maximum items
davidgamez Oct 3, 2024
94b9077
add documentation and sort memory usage for feed with no reference
davidgamez Oct 4, 2024
98c0275
orting from the highest to the lowest memory usage
davidgamez Oct 4, 2024
a16fdee
improve acceptance tests documentation
davidgamez Oct 4, 2024
f95843d
Merge branch 'master' into chore/memory-monitor
davidgamez Oct 4, 2024
db36328
revert acceptance tests sample running
davidgamez Oct 4, 2024
2a7a1f6
remove large feeds from exclude list
davidgamez Oct 7, 2024
8fa5651
Merge branch 'master' into test/large-feeds
davidgamez Oct 9, 2024
f9ba38e
Merge branch 'master' into test/large-feeds
davidgamez Oct 9, 2024
5f644fc
fix formatting
davidgamez Oct 9, 2024
c7ef809
fix ordering
davidgamez Oct 9, 2024
b50f9bf
fix unit test
davidgamez Oct 9, 2024
4bd6064
add decreased memory comparator
davidgamez Oct 9, 2024
053cdbe
add memory metrics
davidgamez Oct 10, 2024
2513c23
fix comment formatting
davidgamez Oct 10, 2024
8a304bb
fix invalid references
davidgamez Oct 10, 2024
ce54a4d
remove memory full list
davidgamez Oct 11, 2024
b372c2c
delete unused comparators
davidgamez Oct 11, 2024
f0bf1eb
delete unused code
davidgamez Oct 11, 2024
8bc962f
Merge branch 'master' into test/large-feeds
davidgamez Oct 15, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Merge branch 'master' into test/large-feeds
davidgamez committed Oct 9, 2024
commit f9ba38e87b1725ae0012760cb6f58f91d86ab968
7 changes: 6 additions & 1 deletion docs/ACCEPTANCE_TESTS.md
Original file line number Diff line number Diff line change
@@ -106,7 +106,7 @@ We follow this process:

## Performance metrics within the acceptance reports

There are two man metrics added to the acceptance report comment at the PR level, _Validation Time_ and _Memory Consumption_.
There are two main metrics added to the acceptance report comment at the PR level, _Validation Time_ and _Memory Consumption_.
The performance metrics are **not a blocker** as performance might vary due to external factors including GitHub infrastructure performance.
However, large jumps in performance values should be investigated before approving a PR.

@@ -115,6 +115,11 @@ The validation time consists in general metrics like average, median, standard d
This metrics can be affected by addition of new validators than introduce a penalty in processing time.

### Memory Consumption
There are two main patterns on how to take a memory usage snapshot:

- MemoryMonitor annotation: This annotation persists the memory usage in the target method. As a limitation, for methods that have concurrent thread executions, the annotation persists in multiple snapshots. This cannot be very clear when analyzing memory usage.
- MemoryUsageRegister: using the registry directly give you more flexibility than the annotation and can be used in cases where MemoryMonitor produces multiple entries on concurrent executed methods.

The memory consumption section contains three tables.
- The first, list the first 25 datasets that the difference increased memory comparing with the main branch.
- The second, list the first 25 datasets that the difference decreased memory comparing with the main branch.
Original file line number Diff line number Diff line change
@@ -76,6 +76,8 @@ public ValidationRunner(VersionResolver versionResolver) {
@MemoryMonitor
public Status run(ValidationRunnerConfig config) {
MemoryUsageRegister.getInstance().clearRegistry();
// Registering the memory metrics manually to avoid multiple entries due to concurrent calls
// and exclude from the metric the generation of the reports.
var memoryBefore =
MemoryUsageRegister.getInstance().getMemoryUsageSnapshot("ValidationRunner.run", null);
VersionInfo versionInfo =
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package org.mobilitydata.gtfsvalidator.outputcomparator.io;

import com.google.common.flogger.FluentLogger;

import java.util.Collections;
import java.util.HashMap;
import java.util.List;
@@ -13,6 +15,8 @@
*/
public class DatasetMemoryUsage {

private static final FluentLogger logger = FluentLogger.forEnclosingClass();

private String datasetId;
private List<MemoryUsage> referenceMemoryUsage;
private List<MemoryUsage> latestMemoryUsage;
@@ -29,12 +33,28 @@ public DatasetMemoryUsage(
if (referenceMemoryUsage != null) {
this.referenceUsedMemoryByKey =
referenceMemoryUsage.stream()
.collect(Collectors.toUnmodifiableMap(MemoryUsage::getKey, MemoryUsage::usedMemory));
.collect(
Collectors.toUnmodifiableMap(
MemoryUsage::getKey,
MemoryUsage::usedMemory,
(existing, replacement) -> {
logger.atWarning().log(
"Duplicate key found in referenceMemoryUsage: " + existing);
return existing;
}));
}
if (latestMemoryUsage != null) {
this.latestUsedMemoryByKey =
latestMemoryUsage.stream()
.collect(Collectors.toUnmodifiableMap(MemoryUsage::getKey, MemoryUsage::usedMemory));
.collect(
Collectors.toUnmodifiableMap(
MemoryUsage::getKey,
MemoryUsage::usedMemory,
(existing, replacement) -> {
logger.atWarning().log(
"Duplicate key found in latestMemoryUsage: " + existing);
return existing;
}));
}
}

You are viewing a condensed version of this merge commit. You can view the full changes here.