From 4cb40418c83ee336514d925217a5215d1ea09fcc Mon Sep 17 00:00:00 2001 From: schaturvedi Date: Tue, 10 Aug 2021 04:39:44 +0530 Subject: [PATCH] 7231: JMC createReport for JMC8 Automated Analysis fails to evaluate several rules --- .../rules/jdk/memory/GcLockerRule.java | 42 ++++++++++--------- .../rules/jdk/memory/GcStallRule.java | 36 +++++++++------- .../rules/jdk/memory/HeapInspectionRule.java | 38 ++++++++++------- .../jdk/memory/StringDeduplicationRule.java | 5 ++- .../rules/jdk/memory/SystemGcRule.java | 32 +++++++------- 5 files changed, 87 insertions(+), 66 deletions(-) diff --git a/core/org.openjdk.jmc.flightrecorder.rules.jdk/src/main/java/org/openjdk/jmc/flightrecorder/rules/jdk/memory/GcLockerRule.java b/core/org.openjdk.jmc.flightrecorder.rules.jdk/src/main/java/org/openjdk/jmc/flightrecorder/rules/jdk/memory/GcLockerRule.java index 8a4f8ed29..c9c42a847 100644 --- a/core/org.openjdk.jmc.flightrecorder.rules.jdk/src/main/java/org/openjdk/jmc/flightrecorder/rules/jdk/memory/GcLockerRule.java +++ b/core/org.openjdk.jmc.flightrecorder.rules.jdk/src/main/java/org/openjdk/jmc/flightrecorder/rules/jdk/memory/GcLockerRule.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. * @@ -83,27 +83,29 @@ public class GcLockerRule implements IRule { private IResult getResult( IItemCollection items, IPreferenceValueProvider valueProvider, IResultValueProvider resultProvider) { - GarbageCollectionsInfo aggregate = resultProvider.getResultValue(GarbageCollectionInfoRule.GC_INFO); - if (aggregate != null) { - int gcLockers = aggregate.getGcLockers(); - if (gcLockers > 0) { - int gcCount = aggregate.getGcCount(); - IQuantity limit = valueProvider.getPreferenceValue(GC_LOCKER_RATIO_LIMIT); - double ratio = gcLockers / gcCount; - double score = RulesToolkit.mapExp74(ratio, limit.doubleValue()); - return ResultBuilder.createFor(this, valueProvider).setSeverity(Severity.get(score)) - .setSummary(Messages.getString(Messages.GcLockerRuleFactory_TEXT_INFO)) - .setExplanation(Messages.getString(Messages.GcLockerRuleFactory_TEXT_INFO_LONG)) - .addResult(TypedResult.SCORE, UnitLookup.NUMBER_UNITY.quantity(score)) - .addResult(GC_LOCKER_RATIO, UnitLookup.PERCENT_UNITY.quantity(ratio)).build(); - } else { - return ResultBuilder.createFor(this, valueProvider).setSeverity(Severity.OK) - .setSummary(Messages.getString(Messages.GcLockerRuleFactory_TEXT_OK)).build(); + if (resultProvider != null) { + GarbageCollectionsInfo aggregate = resultProvider.getResultValue(GarbageCollectionInfoRule.GC_INFO); + if (aggregate != null) { + int gcLockers = aggregate.getGcLockers(); + if (gcLockers > 0) { + int gcCount = aggregate.getGcCount(); + IQuantity limit = valueProvider.getPreferenceValue(GC_LOCKER_RATIO_LIMIT); + double ratio = gcLockers / gcCount; + double score = RulesToolkit.mapExp74(ratio, limit.doubleValue()); + return ResultBuilder.createFor(this, valueProvider).setSeverity(Severity.get(score)) + .setSummary(Messages.getString(Messages.GcLockerRuleFactory_TEXT_INFO)) + .setExplanation(Messages.getString(Messages.GcLockerRuleFactory_TEXT_INFO_LONG)) + .addResult(TypedResult.SCORE, UnitLookup.NUMBER_UNITY.quantity(score)) + .addResult(GC_LOCKER_RATIO, UnitLookup.PERCENT_UNITY.quantity(ratio)).build(); + } else { + return ResultBuilder.createFor(this, valueProvider).setSeverity(Severity.OK) + .setSummary(Messages.getString(Messages.GcLockerRuleFactory_TEXT_OK)).build(); + } } - } else { - return ResultBuilder.createFor(this, valueProvider).setSeverity(Severity.NA) - .setSummary(Messages.getString(Messages.GcLockerRule_TEXT_NA)).build(); } + return ResultBuilder.createFor(this, valueProvider).setSeverity(Severity.NA) + .setSummary(Messages.getString(Messages.GcLockerRule_TEXT_NA)).build(); + } @Override diff --git a/core/org.openjdk.jmc.flightrecorder.rules.jdk/src/main/java/org/openjdk/jmc/flightrecorder/rules/jdk/memory/GcStallRule.java b/core/org.openjdk.jmc.flightrecorder.rules.jdk/src/main/java/org/openjdk/jmc/flightrecorder/rules/jdk/memory/GcStallRule.java index b9b9dccd0..62a95e93c 100644 --- a/core/org.openjdk.jmc.flightrecorder.rules.jdk/src/main/java/org/openjdk/jmc/flightrecorder/rules/jdk/memory/GcStallRule.java +++ b/core/org.openjdk.jmc.flightrecorder.rules.jdk/src/main/java/org/openjdk/jmc/flightrecorder/rules/jdk/memory/GcStallRule.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. * @@ -75,20 +75,28 @@ public RunnableFuture createEvaluation( FutureTask evaluationTask = new FutureTask<>(new Callable() { @Override public IResult call() throws Exception { - GarbageCollectionsInfo aggregate = resultProvider.getResultValue(GarbageCollectionInfoRule.GC_INFO); - if (aggregate.foundNonRequestedSerialOldGc()) { - CollectorType oldCollectorType = CollectorType.getOldCollectorType(items); - if (oldCollectorType == CollectorType.CMS) { - return ResultBuilder.createFor(GcStallRule.this, valueProvider).setSeverity(Severity.WARNING) - .setSummary(Messages.getString(Messages.SerialOldRuleFactory_TEXT_WARN_CMS)) - .setExplanation(Messages.getString(Messages.SerialOldRuleFactory_TEXT_WARN_CMS_LONG)) - .build(); - } else if (oldCollectorType == CollectorType.G1_OLD) { - return ResultBuilder.createFor(GcStallRule.this, valueProvider).setSeverity(Severity.WARNING) - .setSummary(Messages.getString(Messages.SerialOldRuleFactory_TEXT_WARN_G1)) - .setExplanation(Messages.getString(Messages.SerialOldRuleFactory_TEXT_WARN_G1_LONG)) - .build(); + if (resultProvider != null) { + GarbageCollectionsInfo aggregate = resultProvider.getResultValue(GarbageCollectionInfoRule.GC_INFO); + if (aggregate.foundNonRequestedSerialOldGc()) { + CollectorType oldCollectorType = CollectorType.getOldCollectorType(items); + if (oldCollectorType == CollectorType.CMS) { + return ResultBuilder.createFor(GcStallRule.this, valueProvider) + .setSeverity(Severity.WARNING) + .setSummary(Messages.getString(Messages.SerialOldRuleFactory_TEXT_WARN_CMS)) + .setExplanation( + Messages.getString(Messages.SerialOldRuleFactory_TEXT_WARN_CMS_LONG)) + .build(); + } else if (oldCollectorType == CollectorType.G1_OLD) { + return ResultBuilder.createFor(GcStallRule.this, valueProvider) + .setSeverity(Severity.WARNING) + .setSummary(Messages.getString(Messages.SerialOldRuleFactory_TEXT_WARN_G1)) + .setExplanation(Messages.getString(Messages.SerialOldRuleFactory_TEXT_WARN_G1_LONG)) + .build(); + } } + } else { + return ResultBuilder.createFor(GcStallRule.this, valueProvider).setSeverity(Severity.NA) + .setSummary(Messages.getString(Messages.GcLockerRule_TEXT_NA)).build(); } IQuantity c = items.getAggregate(Aggregators.count(null, null, JdkFilters.CONCURRENT_MODE_FAILURE)); if (c != null && c.clampedLongValueIn(NUMBER_UNITY) > 0) { diff --git a/core/org.openjdk.jmc.flightrecorder.rules.jdk/src/main/java/org/openjdk/jmc/flightrecorder/rules/jdk/memory/HeapInspectionRule.java b/core/org.openjdk.jmc.flightrecorder.rules.jdk/src/main/java/org/openjdk/jmc/flightrecorder/rules/jdk/memory/HeapInspectionRule.java index 9aa0d248c..0281667d4 100644 --- a/core/org.openjdk.jmc.flightrecorder.rules.jdk/src/main/java/org/openjdk/jmc/flightrecorder/rules/jdk/memory/HeapInspectionRule.java +++ b/core/org.openjdk.jmc.flightrecorder.rules.jdk/src/main/java/org/openjdk/jmc/flightrecorder/rules/jdk/memory/HeapInspectionRule.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. * @@ -98,23 +98,29 @@ public IResult call() throws Exception { private IResult getHeapInspectionResult( IItemCollection items, IPreferenceValueProvider vp, IResultValueProvider rp) { IQuantity limit = vp.getPreferenceValue(HEAP_INSPECTION_LIMIT); - GarbageCollectionsInfo aggregate = rp.getResultValue(GarbageCollectionInfoRule.GC_INFO); - if (aggregate.getObjectCountGCs() > 0) { - double score = RulesToolkit.mapExp74(aggregate.getObjectCountGCs(), limit.longValue()); - String longMessage = Messages.getString(Messages.HeapInspectionGcRuleFactory_TEXT_INFO_LONG); - if (RulesToolkit.isEventsEnabled(items, JdkTypeIDs.OBJECT_COUNT)) { - longMessage += "\n" + Messages.getString(Messages.HeapInspectionGcRuleFactory_TEXT_INFO_LONG_JFR); //$NON-NLS-1$ + if (rp != null) { + GarbageCollectionsInfo aggregate = rp.getResultValue(GarbageCollectionInfoRule.GC_INFO); + if (aggregate.getObjectCountGCs() > 0) { + double score = RulesToolkit.mapExp74(aggregate.getObjectCountGCs(), limit.longValue()); + String longMessage = Messages.getString(Messages.HeapInspectionGcRuleFactory_TEXT_INFO_LONG); + if (RulesToolkit.isEventsEnabled(items, JdkTypeIDs.OBJECT_COUNT)) { + longMessage += "\n" + Messages.getString(Messages.HeapInspectionGcRuleFactory_TEXT_INFO_LONG_JFR); //$NON-NLS-1$ + } + return ResultBuilder.createFor(this, vp).setSeverity(Severity.get(score)) + .setSummary(Messages.getString(Messages.HeapInspectionGcRuleFactory_TEXT_INFO)) + .setExplanation(longMessage) + .addResult(TypedResult.SCORE, UnitLookup.NUMBER_UNITY.quantity(score)) + .addResult(OBJECT_COUNT_GCS, UnitLookup.NUMBER_UNITY.quantity(aggregate.getObjectCountGCs())) + .build(); + } else { + return ResultBuilder.createFor(this, vp).setSeverity(Severity.OK) + .setSummary(Messages.getString(Messages.HeapInspectionGcRuleFactory_TEXT_OK)) + .setExplanation(Messages.getString(Messages.HeapInspectionGcRuleFactory_TEXT_OK_LONG)).build(); } - return ResultBuilder.createFor(this, vp).setSeverity(Severity.get(score)) - .setSummary(Messages.getString(Messages.HeapInspectionGcRuleFactory_TEXT_INFO)) - .setExplanation(longMessage).addResult(TypedResult.SCORE, UnitLookup.NUMBER_UNITY.quantity(score)) - .addResult(OBJECT_COUNT_GCS, UnitLookup.NUMBER_UNITY.quantity(aggregate.getObjectCountGCs())) - .build(); - } else { - return ResultBuilder.createFor(this, vp).setSeverity(Severity.OK) - .setSummary(Messages.getString(Messages.HeapInspectionGcRuleFactory_TEXT_OK)) - .setExplanation(Messages.getString(Messages.HeapInspectionGcRuleFactory_TEXT_OK_LONG)).build(); } + + return ResultBuilder.createFor(this, vp).setSeverity(Severity.NA) + .setSummary(Messages.getString(Messages.GcLockerRule_TEXT_NA)).build(); } @Override diff --git a/core/org.openjdk.jmc.flightrecorder.rules.jdk/src/main/java/org/openjdk/jmc/flightrecorder/rules/jdk/memory/StringDeduplicationRule.java b/core/org.openjdk.jmc.flightrecorder.rules.jdk/src/main/java/org/openjdk/jmc/flightrecorder/rules/jdk/memory/StringDeduplicationRule.java index a1687b609..bec6d3ec2 100644 --- a/core/org.openjdk.jmc.flightrecorder.rules.jdk/src/main/java/org/openjdk/jmc/flightrecorder/rules/jdk/memory/StringDeduplicationRule.java +++ b/core/org.openjdk.jmc.flightrecorder.rules.jdk/src/main/java/org/openjdk/jmc/flightrecorder/rules/jdk/memory/StringDeduplicationRule.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. * @@ -195,7 +195,8 @@ protected IResult getResult(IItemCollection items, IPreferenceValueProvider vp, IQuantity heapUsedRatio = null; if (maxHeapSize != null) { IQuantity avgHeapUsed = items.getAggregate(JdkAggregators.AVG_HEAP_USED_AFTER_GC); - heapUsedRatio = UnitLookup.PERCENT_UNITY.quantity(avgHeapUsed.ratioTo(maxHeapSize)); + if (avgHeapUsed != null) + heapUsedRatio = UnitLookup.PERCENT_UNITY.quantity(avgHeapUsed.ratioTo(maxHeapSize)); heapInfo = Messages.getString(Messages.StringDeduplicationRule_RESULT_HEAP_USAGE); } diff --git a/core/org.openjdk.jmc.flightrecorder.rules.jdk/src/main/java/org/openjdk/jmc/flightrecorder/rules/jdk/memory/SystemGcRule.java b/core/org.openjdk.jmc.flightrecorder.rules.jdk/src/main/java/org/openjdk/jmc/flightrecorder/rules/jdk/memory/SystemGcRule.java index 4cf9584fb..1506294b5 100644 --- a/core/org.openjdk.jmc.flightrecorder.rules.jdk/src/main/java/org/openjdk/jmc/flightrecorder/rules/jdk/memory/SystemGcRule.java +++ b/core/org.openjdk.jmc.flightrecorder.rules.jdk/src/main/java/org/openjdk/jmc/flightrecorder/rules/jdk/memory/SystemGcRule.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. * @@ -96,20 +96,24 @@ public IResult call() throws Exception { private IResult getSystemGcResult( IItemCollection items, IPreferenceValueProvider valueProvider, IResultValueProvider resultProvider) { - GarbageCollectionsInfo aggregate = resultProvider.getResultValue(GarbageCollectionInfoRule.GC_INFO); - IQuantity limit = valueProvider.getPreferenceValue(SYSTEM_GC_RATIO_LIMIT); - if (aggregate.getSystemGcCount() > 0) { - double systemGcRatio = aggregate.getSystemGcCount() / aggregate.getGcCount(); - double score = RulesToolkit.mapExp100(systemGcRatio, limit.doubleValue()); - return ResultBuilder.createFor(this, valueProvider).setSeverity(Severity.get(score)) - .setSummary(Messages.getString(Messages.SystemGcRuleFactory_TEXT_INFO)) - .setExplanation(Messages.getString(Messages.SystemGcRuleFactory_TEXT_INFO_LONG)) - .addResult(TypedResult.SCORE, UnitLookup.NUMBER_UNITY.quantity(score)) - .addResult(SYSTEM_GC_RATIO, UnitLookup.PERCENT_UNITY.quantity(systemGcRatio)).build(); - } else { - return ResultBuilder.createFor(this, valueProvider).setSeverity(Severity.OK) - .setSummary(Messages.getString(Messages.SystemGcRuleFactory_TEXT_OK)).build(); + if (resultProvider != null) { + GarbageCollectionsInfo aggregate = resultProvider.getResultValue(GarbageCollectionInfoRule.GC_INFO); + IQuantity limit = valueProvider.getPreferenceValue(SYSTEM_GC_RATIO_LIMIT); + if (aggregate.getSystemGcCount() > 0) { + double systemGcRatio = aggregate.getSystemGcCount() / aggregate.getGcCount(); + double score = RulesToolkit.mapExp100(systemGcRatio, limit.doubleValue()); + return ResultBuilder.createFor(this, valueProvider).setSeverity(Severity.get(score)) + .setSummary(Messages.getString(Messages.SystemGcRuleFactory_TEXT_INFO)) + .setExplanation(Messages.getString(Messages.SystemGcRuleFactory_TEXT_INFO_LONG)) + .addResult(TypedResult.SCORE, UnitLookup.NUMBER_UNITY.quantity(score)) + .addResult(SYSTEM_GC_RATIO, UnitLookup.PERCENT_UNITY.quantity(systemGcRatio)).build(); + } else { + return ResultBuilder.createFor(this, valueProvider).setSeverity(Severity.OK) + .setSummary(Messages.getString(Messages.SystemGcRuleFactory_TEXT_OK)).build(); + } } + return ResultBuilder.createFor(this, valueProvider).setSeverity(Severity.NA) + .setSummary(Messages.getString(Messages.GcLockerRule_TEXT_NA)).build(); } @Override