7231: JMC createReport for JMC8 Automated Analysis fails to evaluate several rules

Reviewed-by: hirt, hdafgard

Author: aptmac

core/org.openjdk.jmc.flightrecorder.rules/src/main/java/org/openjdk/jmc/flightrecorder/rules/messages/internal/Messages.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved.
  *
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
@@ -43,10 +43,12 @@ public class Messages {
 	public static final String ItemTreeToolkit_BREAKDOWN_HEADER_LAYERS = "ItemTreeToolkit_BREAKDOWN_HEADER_LAYERS"; //$NON-NLS-1$
 	public static final String ItemTreeToolkit_BREAKDOWN_HEADER_MAX_DURATION_EVENT_CHAIN = "ItemTreeToolkit_BREAKDOWN_HEADER_MAX_DURATION_EVENT_CHAIN"; //$NON-NLS-1$
 	public static final String ItemTreeToolkit_BREAKDOWN_LAYER_CAPTION = "ItemTreeToolkit_BREAKDOWN_LAYER_CAPTION"; //$NON-NLS-1$
-	public static final String Result_SHORT_RECORDING = "Result_SHORT_RECORDING";; //$NON-NLS-1$
+	public static final String Result_SHORT_RECORDING = "Result_SHORT_RECORDING"; //$NON-NLS-1$
 	public static final String RulesToolkit_ATTRIBUTE_NOT_FOUND = "RulesToolkit_ATTRIBUTE_NOT_FOUND"; //$NON-NLS-1$
 	public static final String RulesToolkit_ATTRIBUTE_NOT_FOUND_LONG = "RulesToolkit_ATTRIBUTE_NOT_FOUND_LONG"; //$NON-NLS-1$
+	public static final String RulesToolkit_EVALUATION_ERROR_DESCRIPTION = "RulesToolkit_EVALUATION_ERROR_DESCRIPTION"; //$NON-NLS-1$
 	public static final String RulesToolkit_EVERY_CHUNK = "RulesToolkit_EVERY_CHUNK"; //$NON-NLS-1$
+	public static final String RulesToolkit_RULE_IGNORED = "RulesToolkit_RULE_IGNORED"; //$NON-NLS-1$
 	public static final String RulesToolkit_RULE_RECOMMENDS_EVENTS = "RulesToolkit_RULE_RECOMMENDS_EVENTS"; //$NON-NLS-1$
 	public static final String RulesToolkit_RULE_REQUIRES_EVENTS = "RulesToolkit_RULE_REQUIRES_EVENTS"; //$NON-NLS-1$
 	public static final String RulesToolkit_RULE_REQUIRES_EVENTS_LONG = "RulesToolkit_RULE_REQUIRES_EVENTS_LONG"; //$NON-NLS-1$
@@ -57,6 +59,7 @@ public class Messages {
 	public static final String RulesToolkit_RULE_REQUIRES_SOME_EVENTS = "RulesToolkit_RULE_REQUIRES_SOME_EVENTS"; //$NON-NLS-1$
 	public static final String RulesToolkit_RULE_REQUIRES_UNAVAILABLE_EVENT_TYPE = "RulesToolkit_RULE_REQUIRES_UNAVAILABLE_EVENT_TYPE"; //$NON-NLS-1$
 	public static final String RulesToolkit_RULE_REQUIRES_UNAVAILABLE_EVENT_TYPE_LONG = "RulesToolkit_RULE_REQUIRES_UNAVAILABLE_EVENT_TYPE_LONG"; //$NON-NLS-1$
+	public static final String RulesToolkit_RULE_RESULT_RETRIEVAL_ERROR = "RulesToolkit_RULE_RESULT_RETRIEVAL_ERROR"; //$NON-NLS-1$
 	public static final String RulesToolkit_TOO_FEW_EVENTS = "RulesToolkit_TOO_FEW_EVENTS"; //$NON-NLS-1$
 	public static final String Severity_INFORMATION = "Severity_INFORMATION"; //$NON-NLS-1$
 	public static final String Severity_NOT_APPLICABLE = "Severity_NOT_APPLICABLE"; //$NON-NLS-1$

core/org.openjdk.jmc.flightrecorder.rules/src/main/java/org/openjdk/jmc/flightrecorder/rules/util/RulesToolkit.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved.
  *
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
@@ -47,11 +47,14 @@
 import java.util.Map.Entry;
 import java.util.Queue;
 import java.util.Set;
+import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ConcurrentLinkedQueue;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Future;
 import java.util.concurrent.RunnableFuture;
 import java.util.function.Predicate;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -80,6 +83,7 @@
 import org.openjdk.jmc.common.unit.UnitLookup;
 import org.openjdk.jmc.common.util.IPreferenceValueProvider;
 import org.openjdk.jmc.common.util.LabeledIdentifier;
+import org.openjdk.jmc.common.util.Pair;
 import org.openjdk.jmc.common.util.PredicateToolkit;
 import org.openjdk.jmc.common.util.StringToolkit;
 import org.openjdk.jmc.common.version.JavaVersion;
@@ -88,10 +92,12 @@
 import org.openjdk.jmc.flightrecorder.jdk.JdkAttributes;
 import org.openjdk.jmc.flightrecorder.jdk.JdkFilters;
 import org.openjdk.jmc.flightrecorder.jdk.JdkTypeIDs;
+import org.openjdk.jmc.flightrecorder.rules.DependsOn;
 import org.openjdk.jmc.flightrecorder.rules.IResult;
 import org.openjdk.jmc.flightrecorder.rules.IResultValueProvider;
 import org.openjdk.jmc.flightrecorder.rules.IRule;
 import org.openjdk.jmc.flightrecorder.rules.ResultBuilder;
+import org.openjdk.jmc.flightrecorder.rules.ResultProvider;
 import org.openjdk.jmc.flightrecorder.rules.RuleRegistry;
 import org.openjdk.jmc.flightrecorder.rules.Severity;
 import org.openjdk.jmc.flightrecorder.rules.messages.internal.Messages;
@@ -1208,12 +1214,59 @@ public static Map<IRule, Future<IResult>> evaluateParallel(
 		if (nThreads < 1) {
 			nThreads = Runtime.getRuntime().availableProcessors();
 		}
+		ResultProvider resultProvider = new ResultProvider();
 		Map<IRule, Future<IResult>> resultFutures = new HashMap<>();
 		Queue<RunnableFuture<IResult>> futureQueue = new ConcurrentLinkedQueue<>();
+		// Map using the rule name as a key, and a Pair containing the rule (left) and it's dependency (right)
+		Map<String, Pair<IRule, IRule>> rulesWithDependencies = new HashMap<>();
+		Map<IRule, IResult> computedResults = new HashMap<>();
 		for (IRule rule : rules) {
-			RunnableFuture<IResult> resultFuture = rule.createEvaluation(items, preferences, null);
-			resultFutures.put(rule, resultFuture);
-			futureQueue.add(resultFuture);
+			if (matchesEventAvailabilityMap(items, rule.getRequiredEvents())) {
+				if (hasDependency(rule)) {
+					IRule depRule = rules.stream().filter(r -> r.getId().equals(getRuleDependencyName(rule)))
+							.findFirst().orElse(null);
+					rulesWithDependencies.put(rule.getId(), new Pair<>(rule, depRule));
+				} else {
+					RunnableFuture<IResult> resultFuture = rule.createEvaluation(items, preferences, resultProvider);
+					resultFutures.put(rule, resultFuture);
+					futureQueue.add(resultFuture);
+				}
+			} else {
+				resultFutures.put(rule, CompletableFuture.completedFuture(getNotApplicableResult(rule, preferences,
+						Messages.getString(Messages.RulesToolkit_RULE_IGNORED))));
+			}
+		}
+		for (String ruleName : rulesWithDependencies.keySet()) {
+			IRule rule = rulesWithDependencies.get(ruleName).left;
+			IRule depRule = rulesWithDependencies.get(ruleName).right;
+			Future<IResult> depResultFuture = resultFutures.get(depRule);
+			if (depResultFuture == null) {
+				resultFutures.put(rule, CompletableFuture.completedFuture(getNotApplicableResult(rule, preferences,
+						Messages.getString(Messages.RulesToolkit_EVALUATION_ERROR_DESCRIPTION))));
+			} else {
+				IResult depResult = null;
+				if (!depResultFuture.isDone()) {
+					((Runnable) depResultFuture).run();
+					try {
+						depResult = depResultFuture.get();
+						resultProvider.addResults(depResult);
+						computedResults.put(depRule, depResult);
+					} catch (InterruptedException | ExecutionException e) {
+						Logger.getLogger(RulesToolkit.class.getName()).log(Level.WARNING, MessageFormat
+								.format(Messages.getString(Messages.RulesToolkit_RULE_RESULT_RETRIEVAL_ERROR), e));
+					}
+				} else {
+					depResult = computedResults.get(depRule);
+				}
+				if (depResult != null && shouldEvaluate(rule, depResult)) {
+					RunnableFuture<IResult> resultFuture = rule.createEvaluation(items, preferences, resultProvider);
+					resultFutures.put(rule, resultFuture);
+					futureQueue.add(resultFuture);
+				} else {
+					resultFutures.put(rule, CompletableFuture.completedFuture(getNotApplicableResult(rule, preferences,
+							Messages.getString(Messages.RulesToolkit_RULE_IGNORED))));
+				}
+			}
 		}
 		for (int i = 0; i < nThreads; i++) {
 			RuleEvaluator re = new RuleEvaluator(futureQueue);
@@ -1223,6 +1276,38 @@ public static Map<IRule, Future<IResult>> evaluateParallel(
 		return resultFutures;
 	}
 
+	private static boolean hasDependency(IRule rule) {
+		DependsOn dependency = rule.getClass().getAnnotation(DependsOn.class);
+		return dependency != null;
+	}
+
+	private static String getRuleDependencyName(IRule rule) {
+		DependsOn dependency = rule.getClass().getAnnotation(DependsOn.class);
+		Class<? extends IRule> dependencyType = dependency.value();
+		return dependencyType.getSimpleName();
+	}
+
+	/**
+	 * Checks to see if a rule should be evaluated based on the severity value of its dependency's
+	 * result severity value.
+	 * 
+	 * @param rule
+	 *            rule to check severity value against its dependency's severity
+	 * @param depResult
+	 *            result from the rule's dependency
+	 * @return true if the dependency rule result satisfies the severity requirement for the passed
+	 *         rule
+	 */
+	private static boolean shouldEvaluate(IRule rule, IResult depResult) {
+		DependsOn dependency = rule.getClass().getAnnotation(DependsOn.class);
+		if (dependency != null) {
+			if (depResult.getSeverity().compareTo(dependency.severity()) < 0) {
+				return false;
+			}
+		}
+		return true;
+	}
+
 	private static class RuleEvaluator implements Runnable {
 		private Queue<RunnableFuture<IResult>> futureQueue;
 

core/org.openjdk.jmc.flightrecorder.rules/src/main/resources/org/openjdk/jmc/flightrecorder/rules/messages/internal/messages.properties

@@ -1,5 +1,5 @@
 #
-#  Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+#  Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved.
 #
 #  DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
 #
@@ -34,7 +34,9 @@
 RulesToolkit_ATTRIBUTE_NOT_FOUND=Could not get the attribute with id ''{0}'' from the event type with id ''{1}''. This recording is likely from an unsupported version.
 # {0} is an event type id, {1} is an attribute id
 RulesToolkit_ATTRIBUTE_NOT_FOUND_LONG=Could not get the attribute with id ''{0}'' from the event type with id ''{1}''. This recording is likely from an unsupported version.<p>If you are using an older Java version, then you can consider upgrading.
+RulesToolkit_EVALUATION_ERROR_DESCRIPTION=Could not evaluate this rule
 RulesToolkit_EVERY_CHUNK=Every Chunk
+RulesToolkit_RULE_IGNORED=Ignored
 # {0} is a rule name, {1} is one or more event type ids
 RulesToolkit_RULE_REQUIRES_UNAVAILABLE_EVENT_TYPE=The {0} rule requires the following event types: {1}.
 # {0} is a rule name, {1} is one or more event type ids
@@ -55,6 +57,7 @@ RulesToolkit_RULE_REQUIRES_EVENT_TYPE_LONG=The {0} rule requires that the follow
 RulesToolkit_RULE_REQUIRES_SOME_EVENTS=The {0} rule requires events to be available from at least one of the following event types: {1}.
 # {0} is one or more event type names
 RulesToolkit_RULE_RECOMMENDS_EVENTS=To improve rule accuracy and/or get more details for further investigation, it is recommended to enable the following event types: {0}.
+RulesToolkit_RULE_RESULT_RETRIEVAL_ERROR=Unexpected problem retrieving rule result.
 RulesToolkit_TOO_FEW_EVENTS=Too few events to calculate the result.
 
 Result_SHORT_RECORDING=This recording is only {0} long, consider creating a recording longer than {1} for improved rule accuracy.

core/tests/org.openjdk.jmc.flightrecorder.rules.test/META-INF/MANIFEST.MF

@@ -9,5 +9,6 @@ Bundle-ActivationPolicy: lazy
 Require-Bundle: org.junit,
  org.openjdk.jmc.common.test,
  org.openjdk.jmc.flightrecorder,
- org.openjdk.jmc.flightrecorder.rules
+ org.openjdk.jmc.flightrecorder.test,
+ org.openjdk.jmc.flightrecorder.rules,
 Automatic-Module-Name: org.openjdk.jmc.flightrecorder.rules.test

core/tests/org.openjdk.jmc.flightrecorder.rules.test/pom.xml

@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <!--   
-   Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+   Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved.
    
    DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
    
@@ -67,6 +67,11 @@
                    <artifactId>flightrecorder</artifactId>
                    <version>${project.version}</version>
            </dependency>
+           <dependency>
+                   <groupId>org.openjdk.jmc</groupId>
+                   <artifactId>flightrecorder.test</artifactId>
+                   <version>${project.version}</version>
+           </dependency>
            <dependency>
                    <groupId>org.openjdk.jmc</groupId>
                    <artifactId>flightrecorder.rules</artifactId>

core/tests/org.openjdk.jmc.flightrecorder.rules.test/src/test/java/org/openjdk/jmc/flightrecorder/rules/RulesToolkitTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved.
  * 
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
@@ -35,11 +35,17 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import java.io.IOException;
 
 import org.junit.Test;
 
 import org.openjdk.jmc.common.version.JavaVersion;
+import org.openjdk.jmc.flightrecorder.CouldNotLoadRecordingException;
+import org.openjdk.jmc.flightrecorder.rules.report.html.JfrHtmlRulesReport;
 import org.openjdk.jmc.flightrecorder.rules.util.RulesToolkit;
+import org.openjdk.jmc.flightrecorder.test.util.RecordingToolkit;
 
 @SuppressWarnings("nls")
 public class RulesToolkitTest {
@@ -80,4 +86,15 @@ public void testGetJavaVersion() {
 		JavaVersion javaVersion = RulesToolkit.getJavaVersion(JAVA_8_40);
 		assertFalse(javaVersion.isEarlyAccess());
 	}
+
+	@Test
+	public void testJfrHtmlRulesReportGeneration() {
+		String report = "";
+		try {
+			report = JfrHtmlRulesReport.createReport(RecordingToolkit.getNamedRecording("8u60.jfr"));
+		} catch (IOException | CouldNotLoadRecordingException e) {
+			fail();
+		}
+		assert (!report.isEmpty());
+	}
 }