Skip to content

Commit 67698a2

Browse files
fix quadratic performance of tag/step/exception scenario filtering (#411)
Rendering the "scenarios by tag/step/exception" pages cloned all reports and linearly re-scanned every element for each unique tag/step/exception, making generation effectively O(n^2) in the number of scenarios. Replace the per-page full re-scan with a report/element grouping computed once, and build each filtered page directly from only the matching reports instead of cloning and filtering the full report list every time. On a 7619-scenario report this reduces generation time from ~242s to ~36s (6.7x), with no change in generated output (verified via full test suite and byte-for-byte diff of generated reports).
1 parent 5728193 commit 67698a2

2 files changed

Lines changed: 172 additions & 28 deletions

File tree

engine/src/main/java/com/trivago/cluecumber/engine/rendering/pages/pojos/pagecollections/AllScenariosPageCollection.java

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,11 @@
4646
import java.util.ArrayList;
4747
import java.util.Arrays;
4848
import java.util.Comparator;
49+
import java.util.HashMap;
50+
import java.util.HashSet;
4951
import java.util.List;
52+
import java.util.Map;
53+
import java.util.Set;
5054
import java.util.stream.Collectors;
5155

5256
/**
@@ -61,6 +65,12 @@ public class AllScenariosPageCollection extends PageCollection implements Visita
6165
private Step stepFilter;
6266
private String exceptionFilter;
6367

68+
// Lazily computed and cached lookup indices, so that rendering the "scenarios by X" pages for every
69+
// unique tag/step/exception does not require a full re-scan of all reports and elements for each of them.
70+
private Map<Tag, Map<Integer, List<Element>>> elementsByTagAndReportIndex;
71+
private Map<Step, Map<Integer, List<Element>>> elementsByStepAndReportIndex;
72+
private Map<String, Map<Integer, List<Element>>> elementsByExceptionAndReportIndex;
73+
6474
/**
6575
* Constructor.
6676
*
@@ -428,6 +438,84 @@ public void setExpandPreviousScenarioRuns(final boolean expandPreviousScenarioRu
428438
this.expandPreviousScenarioRuns = expandPreviousScenarioRuns;
429439
}
430440

441+
/**
442+
* Group all elements by tag and the index of their originating report.
443+
* <p>
444+
* This is computed once and cached instead of being recomputed for every unique tag, since the
445+
* "scenarios by tag" page is rendered once per unique tag and a full re-scan of all reports and
446+
* elements for each of them can be very expensive for reports with many scenarios and tags.
447+
*
448+
* @return A map of tag to a map of report index to the matching elements of that report.
449+
*/
450+
public Map<Tag, Map<Integer, List<Element>>> getElementsByTagAndReportIndex() {
451+
if (elementsByTagAndReportIndex == null) {
452+
elementsByTagAndReportIndex = new HashMap<>();
453+
for (int reportIndex = 0; reportIndex < reports.size(); reportIndex++) {
454+
for (Element element : reports.get(reportIndex).getElements()) {
455+
for (Tag tag : new HashSet<>(element.getTags())) {
456+
elementsByTagAndReportIndex
457+
.computeIfAbsent(tag, key -> new HashMap<>())
458+
.computeIfAbsent(reportIndex, key -> new ArrayList<>())
459+
.add(element);
460+
}
461+
}
462+
}
463+
}
464+
return elementsByTagAndReportIndex;
465+
}
466+
467+
/**
468+
* Group all elements (including background steps) by step and the index of their originating report.
469+
* <p>
470+
* This is computed once and cached instead of being recomputed for every unique step, since the
471+
* "scenarios by step" page is rendered once per unique step and a full re-scan of all reports and
472+
* elements for each of them can be very expensive for reports with many scenarios and steps.
473+
*
474+
* @return A map of step to a map of report index to the matching elements of that report.
475+
*/
476+
public Map<Step, Map<Integer, List<Element>>> getElementsByStepAndReportIndex() {
477+
if (elementsByStepAndReportIndex == null) {
478+
elementsByStepAndReportIndex = new HashMap<>();
479+
for (int reportIndex = 0; reportIndex < reports.size(); reportIndex++) {
480+
for (Element element : reports.get(reportIndex).getElements()) {
481+
final Set<Step> distinctSteps = new HashSet<>(element.getSteps());
482+
distinctSteps.addAll(element.getBackgroundSteps());
483+
for (Step step : distinctSteps) {
484+
elementsByStepAndReportIndex
485+
.computeIfAbsent(step, key -> new HashMap<>())
486+
.computeIfAbsent(reportIndex, key -> new ArrayList<>())
487+
.add(element);
488+
}
489+
}
490+
}
491+
}
492+
return elementsByStepAndReportIndex;
493+
}
494+
495+
/**
496+
* Group all elements by their first exception class and the index of their originating report.
497+
* <p>
498+
* This is computed once and cached instead of being recomputed for every unique exception, since the
499+
* "scenarios by exception" page is rendered once per unique exception and a full re-scan of all reports
500+
* and elements for each of them can be very expensive for reports with many scenarios and exceptions.
501+
*
502+
* @return A map of exception class to a map of report index to the matching elements of that report.
503+
*/
504+
public Map<String, Map<Integer, List<Element>>> getElementsByExceptionAndReportIndex() {
505+
if (elementsByExceptionAndReportIndex == null) {
506+
elementsByExceptionAndReportIndex = new HashMap<>();
507+
for (int reportIndex = 0; reportIndex < reports.size(); reportIndex++) {
508+
for (Element element : reports.get(reportIndex).getElements()) {
509+
elementsByExceptionAndReportIndex
510+
.computeIfAbsent(element.getFirstExceptionClass(), key -> new HashMap<>())
511+
.computeIfAbsent(reportIndex, key -> new ArrayList<>())
512+
.add(element);
513+
}
514+
}
515+
}
516+
return elementsByExceptionAndReportIndex;
517+
}
518+
431519
/**
432520
* Function to clone the {@link AllScenariosPageCollection} including all included data.
433521
*
@@ -440,6 +528,11 @@ public AllScenariosPageCollection clone() throws CloneNotSupportedException {
440528
clone.setStepFilter(null);
441529
clone.setTagFilter(null);
442530
clone.setExceptionFilter(null);
531+
// A clone only ever holds a (filtered) subset of reports, so any lookup index cached on the
532+
// source collection would be stale and must be recomputed if it were ever accessed on the clone.
533+
clone.elementsByTagAndReportIndex = null;
534+
clone.elementsByStepAndReportIndex = null;
535+
clone.elementsByExceptionAndReportIndex = null;
443536
clone.clearReports();
444537
List<Report> clonedReports = new ArrayList<>();
445538
for (Report r : getReports()) {
@@ -449,6 +542,44 @@ public AllScenariosPageCollection clone() throws CloneNotSupportedException {
449542
return clone;
450543
}
451544

545+
/**
546+
* Clone this collection, but include only the reports that have at least one matching element
547+
* (narrowed down to just those matching elements), as given by a report-index-to-elements map
548+
* such as the ones returned by {@link #getElementsByTagAndReportIndex()}.
549+
* <p>
550+
* This is used instead of {@link #clone()} for the "scenarios by tag/step/exception" pages: since
551+
* most reports typically do not match a given tag/step/exception, cloning and carrying around all
552+
* of them (and repeatedly re-computing statistics like chart counts or start/end times over all of
553+
* them) for every one of the potentially many unique tags/steps/exceptions is very wasteful.
554+
*
555+
* @param matchingElementsByReportIndex A map of original report index to its matching elements.
556+
* @return The filtered clone, containing only reports that have at least one matching element.
557+
*/
558+
public AllScenariosPageCollection cloneWithOnlyMatchingReports(
559+
final Map<Integer, List<Element>> matchingElementsByReportIndex) throws CloneNotSupportedException {
560+
final AllScenariosPageCollection clone = (AllScenariosPageCollection) super.clone();
561+
clone.setFeatureFilter(null);
562+
clone.setStepFilter(null);
563+
clone.setTagFilter(null);
564+
clone.setExceptionFilter(null);
565+
clone.elementsByTagAndReportIndex = null;
566+
clone.elementsByStepAndReportIndex = null;
567+
clone.elementsByExceptionAndReportIndex = null;
568+
clone.clearReports();
569+
List<Report> filteredReports = new ArrayList<>();
570+
for (int reportIndex = 0; reportIndex < reports.size(); reportIndex++) {
571+
List<Element> matchingElements = matchingElementsByReportIndex.get(reportIndex);
572+
if (matchingElements == null || matchingElements.isEmpty()) {
573+
continue;
574+
}
575+
Report reportClone = (Report) reports.get(reportIndex).clone();
576+
reportClone.setElements(new ArrayList<>(matchingElements));
577+
filteredReports.add(reportClone);
578+
}
579+
clone.addReports(filteredReports);
580+
return clone;
581+
}
582+
452583
/**
453584
* Method to accept a {@link PageVisitor}.
454585
*

engine/src/main/java/com/trivago/cluecumber/engine/rendering/pages/renderering/AllScenariosPageRenderer.java

Lines changed: 41 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,9 @@
3131

3232
import javax.inject.Inject;
3333
import javax.inject.Singleton;
34+
import java.util.Collections;
3435
import java.util.List;
36+
import java.util.Map;
3537
import java.util.stream.Collectors;
3638

3739
/**
@@ -116,17 +118,12 @@ public String getRenderedContentByTagFilter(
116118
final PebbleTemplate template,
117119
final Tag tag) throws CluecumberException {
118120

119-
AllScenariosPageCollection allScenariosPageCollectionClone = getAllScenariosPageCollectionClone(allScenariosPageCollection);
121+
Map<Integer, List<Element>> matchingElementsByReportIndex =
122+
allScenariosPageCollection.getElementsByTagAndReportIndex().getOrDefault(tag, Collections.emptyMap());
123+
AllScenariosPageCollection allScenariosPageCollectionClone =
124+
getFilteredAllScenariosPageCollectionClone(allScenariosPageCollection, matchingElementsByReportIndex);
120125
allScenariosPageCollectionClone.setTagFilter(tag);
121126

122-
allScenariosPageCollectionClone.getReports().forEach(report -> {
123-
List<Element> elements = report.getElements()
124-
.stream()
125-
.filter(element -> element.getTags().contains(tag))
126-
.collect(Collectors.toList());
127-
report.setElements(elements);
128-
});
129-
130127
addChartJsonToReportDetails(allScenariosPageCollectionClone);
131128
return processedContent(template, allScenariosPageCollectionClone, propertyManager.getNavigationLinks());
132129
}
@@ -145,17 +142,13 @@ public String getRenderedContentByExceptionFilter(
145142
final PebbleTemplate template,
146143
final String exceptionClass) throws CluecumberException {
147144

148-
AllScenariosPageCollection allScenariosPageCollectionClone = getAllScenariosPageCollectionClone(allScenariosPageCollection);
145+
Map<Integer, List<Element>> matchingElementsByReportIndex =
146+
allScenariosPageCollection.getElementsByExceptionAndReportIndex()
147+
.getOrDefault(exceptionClass, Collections.emptyMap());
148+
AllScenariosPageCollection allScenariosPageCollectionClone =
149+
getFilteredAllScenariosPageCollectionClone(allScenariosPageCollection, matchingElementsByReportIndex);
149150
allScenariosPageCollectionClone.setExceptionFilter(exceptionClass);
150151

151-
allScenariosPageCollectionClone.getReports().forEach(report -> {
152-
List<Element> elements = report.getElements()
153-
.stream()
154-
.filter(element -> element.getFirstExceptionClass().equals(exceptionClass))
155-
.collect(Collectors.toList());
156-
report.setElements(elements);
157-
});
158-
159152
addChartJsonToReportDetails(allScenariosPageCollectionClone);
160153
return processedContent(template, allScenariosPageCollectionClone, propertyManager.getNavigationLinks());
161154
}
@@ -175,15 +168,11 @@ public String getRenderedContentByStepFilter(
175168
final PebbleTemplate template,
176169
final Step step) throws CluecumberException {
177170

178-
AllScenariosPageCollection allScenariosPageCollectionClone = getAllScenariosPageCollectionClone(allScenariosPageCollection);
171+
Map<Integer, List<Element>> matchingElementsByReportIndex =
172+
allScenariosPageCollection.getElementsByStepAndReportIndex().getOrDefault(step, Collections.emptyMap());
173+
AllScenariosPageCollection allScenariosPageCollectionClone =
174+
getFilteredAllScenariosPageCollectionClone(allScenariosPageCollection, matchingElementsByReportIndex);
179175
allScenariosPageCollectionClone.setStepFilter(step);
180-
for (Report report : allScenariosPageCollectionClone.getReports()) {
181-
List<Element> elements = report.getElements()
182-
.stream()
183-
.filter(element -> element.getSteps().contains(step) || element.getBackgroundSteps().contains(step))
184-
.collect(Collectors.toList());
185-
report.setElements(elements);
186-
}
187176

188177
addChartJsonToReportDetails(allScenariosPageCollectionClone);
189178
return processedContent(template, allScenariosPageCollectionClone, propertyManager.getNavigationLinks());
@@ -227,12 +216,36 @@ private void addChartJsonToReportDetails(final AllScenariosPageCollection allSce
227216

228217
private AllScenariosPageCollection getAllScenariosPageCollectionClone(
229218
final AllScenariosPageCollection allScenariosPageCollection) throws CluecumberException {
230-
AllScenariosPageCollection clone;
231219
try {
232-
clone = allScenariosPageCollection.clone();
220+
return finalizeClone(allScenariosPageCollection.clone());
221+
} catch (CloneNotSupportedException e) {
222+
throw new CluecumberException("Clone of AllScenariosPageCollection not supported: " + e.getMessage());
223+
}
224+
}
225+
226+
/**
227+
* Get a clone of the given {@link AllScenariosPageCollection} that contains only the reports that have
228+
* at least one element matching the given report-index-to-elements map, instead of a full clone of all
229+
* reports. Rendering the "scenarios by tag/step/exception" pages for every one of the potentially many
230+
* unique tags/steps/exceptions otherwise requires carrying around (and repeatedly computing statistics
231+
* for) reports that will not be part of the rendered page anyway.
232+
*
233+
* @param allScenariosPageCollection The source {@link AllScenariosPageCollection} instance.
234+
* @param matchingElementsByReportIndex A map of original report index to its matching elements.
235+
* @return The filtered clone.
236+
* @throws CluecumberException Thrown on any error.
237+
*/
238+
private AllScenariosPageCollection getFilteredAllScenariosPageCollectionClone(
239+
final AllScenariosPageCollection allScenariosPageCollection,
240+
final Map<Integer, List<Element>> matchingElementsByReportIndex) throws CluecumberException {
241+
try {
242+
return finalizeClone(allScenariosPageCollection.cloneWithOnlyMatchingReports(matchingElementsByReportIndex));
233243
} catch (CloneNotSupportedException e) {
234244
throw new CluecumberException("Clone of AllScenariosPageCollection not supported: " + e.getMessage());
235245
}
246+
}
247+
248+
private AllScenariosPageCollection finalizeClone(final AllScenariosPageCollection clone) {
236249
addCustomParametersToReportDetails(clone, propertyManager.getCustomParameters());
237250
clone.setGroupPreviousScenarioRuns(propertyManager.isGroupPreviousScenarioRuns());
238251
clone.setExpandPreviousScenarioRuns(propertyManager.isExpandPreviousScenarioRuns());

0 commit comments

Comments
 (0)