diff --git a/src/main/java/io/cryostat/core/reports/InterruptibleReportGenerator.java b/src/main/java/io/cryostat/core/reports/InterruptibleReportGenerator.java index 42f56351..94048483 100644 --- a/src/main/java/io/cryostat/core/reports/InterruptibleReportGenerator.java +++ b/src/main/java/io/cryostat/core/reports/InterruptibleReportGenerator.java @@ -55,29 +55,16 @@ import org.openjdk.jmc.flightrecorder.rules.TypedResult; import org.openjdk.jmc.flightrecorder.rules.util.RulesToolkit; -import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import org.apache.commons.io.input.CountingInputStream; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -/** - * Re-implementation of {@link ReportGenerator} where the report generation task is represented by a - * {@link Future}, allowing callers to cancel ongoing rules report analyses or to easily time out on - * analysis requests. This should eventually replace {@link ReportGenerator} entirely - there should - * only be benefits to using this implementation. - */ -@SuppressFBWarnings( - value = "THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION", - justification = "There are no basic exceptions being thrown") public class InterruptibleReportGenerator { private final ExecutorService qThread = Executors.newCachedThreadPool(); private final ExecutorService executor; private final Logger logger = LoggerFactory.getLogger(getClass()); - @SuppressFBWarnings( - value = "EI_EXPOSE_REP2", - justification = "fields are not exposed since there are no getters") public InterruptibleReportGenerator(ExecutorService executor) { this.executor = executor; } @@ -126,6 +113,9 @@ private Pair, Long> generateResultHelper( Map> rulesWithDependencies = new HashMap<>(); Map computedResults = new HashMap<>(); try (CountingInputStream countingRecordingStream = new CountingInputStream(recording)) { + // TODO parsing the JFR file should also happen on the executor rather than the qThread, + // so that this method can return to the qThread more quickly and free up the ability to + // queue more work on the executor. IItemCollection items = JfrLoaderToolkit.loadEvents(countingRecordingStream); for (IRule rule : rules) { if (RulesToolkit.matchesEventAvailabilityMap(items, rule.getRequiredEvents())) { @@ -155,6 +145,10 @@ private Pair, Long> generateResultHelper( .build())); } } + // TODO this implementation forces rule dependencies to be evaluated on the qThread. The + // executor is only used for rules that have no dependencies or which have all their + // dependencies satisfied (by the qThread). Ideally we should perform all of the rule + // evaluations on executor threads. for (Entry> entry : rulesWithDependencies.entrySet()) { IRule rule = entry.getValue().left; IRule depRule = entry.getValue().right; @@ -201,8 +195,10 @@ private Pair, Long> generateResultHelper( } } } - RuleEvaluator re = new RuleEvaluator(futureQueue); - executor.submit(re); + RunnableFuture resultFuture; + while ((resultFuture = futureQueue.poll()) != null) { + executor.submit(resultFuture); + } Collection results = new HashSet(); for (Future future : resultFutures.values()) { results.add(future.get()); @@ -346,20 +342,4 @@ private static boolean shouldEvaluate(IRule rule, IResult depResult) { } return true; } - - private static class RuleEvaluator implements Runnable { - private Queue> futureQueue; - - public RuleEvaluator(Queue> futureQueue) { - this.futureQueue = futureQueue; - } - - @Override - public void run() { - RunnableFuture resultFuture; - while ((resultFuture = futureQueue.poll()) != null) { - resultFuture.run(); - } - } - } }