From 49d314fc5b4ee08cec937f4a01c94f6becfa089d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Sun, 10 Sep 2017 05:35:09 -0300 Subject: [PATCH 1/3] [core] Incremental Analysis can detect changes to jars - We now look inside the jars of the classpath in search for any changes. If we find any, we invalidate the cache. - Resolves #604 --- .../pmd/cache/AbstractAnalysisCache.java | 27 ++++++++++++-- .../pmd/cache/FileAnalysisCacheTest.java | 36 +++++++++++++++++-- 2 files changed, 58 insertions(+), 5 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/cache/AbstractAnalysisCache.java b/pmd-core/src/main/java/net/sourceforge/pmd/cache/AbstractAnalysisCache.java index 6b128275cc..41d06f0fc4 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/cache/AbstractAnalysisCache.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/cache/AbstractAnalysisCache.java @@ -5,13 +5,19 @@ package net.sourceforge.pmd.cache; import java.io.File; +import java.io.IOException; +import java.net.URL; import java.net.URLClassLoader; -import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; +import java.util.logging.Level; import java.util.logging.Logger; +import java.util.zip.Adler32; +import java.util.zip.CheckedInputStream; + +import org.apache.commons.io.IOUtils; import net.sourceforge.pmd.PMD; import net.sourceforge.pmd.Rule; @@ -87,7 +93,7 @@ public abstract class AbstractAnalysisCache implements AnalysisCache { final long classLoaderChecksum; if (classLoader instanceof URLClassLoader) { final URLClassLoader urlClassLoader = (URLClassLoader) classLoader; - classLoaderChecksum = Arrays.hashCode(urlClassLoader.getURLs()); + classLoaderChecksum = computeClassLoaderHash(urlClassLoader); if (cacheIsValid && classLoaderChecksum != classpathChecksum) { // Do we even care? @@ -114,6 +120,23 @@ public abstract class AbstractAnalysisCache implements AnalysisCache { ruleMapper.initialize(ruleSets); } + private long computeClassLoaderHash(final URLClassLoader classLoader) { + final Adler32 adler32 = new Adler32(); + for (final URL url : classLoader.getURLs()) { + try (CheckedInputStream inputStream = new CheckedInputStream(url.openStream(), adler32)) { + // Just read it, the CheckedInputStream will update the checksum on it's own + while (IOUtils.skip(inputStream, Long.MAX_VALUE) == Long.MAX_VALUE) { + // just loop + } + } catch (final IOException e) { + // Can this even happen? + LOG.log(Level.SEVERE, "Incremental analysis can't check auxclasspath contents", e); + throw new RuntimeException(e); + } + } + return adler32.getValue(); + } + @Override public void ruleViolationAdded(final RuleViolation ruleViolation) { final AnalysisResult analysisResult = updatedResultsCache.get(ruleViolation.getFilename()); diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/cache/FileAnalysisCacheTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/cache/FileAnalysisCacheTest.java index 7dc2d03785..c0c732ba5a 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/cache/FileAnalysisCacheTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/cache/FileAnalysisCacheTest.java @@ -110,7 +110,7 @@ public class FileAnalysisCacheTest { @Test public void testCacheValidityWithNoChanges() { final RuleSets rs = mock(RuleSets.class); - final URLClassLoader cl = mock(URLClassLoader.class); + final ClassLoader cl = mock(ClassLoader.class); setupCacheWithFiles(newCacheFile, rs, cl, sourceFile); @@ -123,7 +123,7 @@ public class FileAnalysisCacheTest { @Test public void testRulesetChangeInvalidatesCache() { final RuleSets rs = mock(RuleSets.class); - final URLClassLoader cl = mock(URLClassLoader.class); + final ClassLoader cl = mock(ClassLoader.class); setupCacheWithFiles(newCacheFile, rs, cl, sourceFile); @@ -138,6 +138,7 @@ public class FileAnalysisCacheTest { public void testClasspathChangeWithoutDFAorTypeResolutionDoesNotInvalidatesCache() throws MalformedURLException, IOException { final RuleSets rs = mock(RuleSets.class); final URLClassLoader cl = mock(URLClassLoader.class); + when(cl.getURLs()).thenReturn(new URL[] { }); setupCacheWithFiles(newCacheFile, rs, cl, sourceFile); @@ -152,11 +153,17 @@ public class FileAnalysisCacheTest { public void testClasspathChangeInvalidatesCache() throws MalformedURLException, IOException { final RuleSets rs = mock(RuleSets.class); final URLClassLoader cl = mock(URLClassLoader.class); + when(cl.getURLs()).thenReturn(new URL[] { }); setupCacheWithFiles(newCacheFile, rs, cl, sourceFile); final FileAnalysisCache reloadedCache = new FileAnalysisCache(newCacheFile); - when(cl.getURLs()).thenReturn(new URL[] { tempFolder.newFile().toURI().toURL(), }); + final File classpathFile = tempFolder.newFile(); + when(cl.getURLs()).thenReturn(new URL[] { classpathFile.toURI().toURL(), }); + + // Make sure the classpath file is not empty + Files.write(Paths.get(classpathFile.getAbsolutePath()), "some text".getBytes()); + final net.sourceforge.pmd.Rule r = mock(net.sourceforge.pmd.Rule.class); when(r.usesDFA()).thenReturn(true); when(rs.getAllRules()).thenReturn(Collections.singleton(r)); @@ -164,6 +171,29 @@ public class FileAnalysisCacheTest { assertFalse("Cache believes unmodified file is up to date after classpath changed", reloadedCache.isUpToDate(sourceFile)); } + + @Test + public void testClasspathJarContentsChangeInvalidatesCache() throws MalformedURLException, IOException { + final RuleSets rs = mock(RuleSets.class); + final URLClassLoader cl = mock(URLClassLoader.class); + + final File classpathFile = tempFolder.newFile(); + when(cl.getURLs()).thenReturn(new URL[] { classpathFile.toURI().toURL(), }); + + final net.sourceforge.pmd.Rule r = mock(net.sourceforge.pmd.Rule.class); + when(r.usesDFA()).thenReturn(true); + when(rs.getAllRules()).thenReturn(Collections.singleton(r)); + + setupCacheWithFiles(newCacheFile, rs, cl, sourceFile); + + // Edit the classpath referenced file + Files.write(Paths.get(classpathFile.getAbsolutePath()), "some text".getBytes()); + + final FileAnalysisCache reloadedCache = new FileAnalysisCache(newCacheFile); + reloadedCache.checkValidity(rs, cl); + assertFalse("Cache believes cache is up to date when a classpath file changed", + reloadedCache.isUpToDate(sourceFile)); + } @Test public void testUnknownFileIsNotUpToDate() throws IOException { From 4ef8495dcbf91334743d7d1129dde98e1d774172 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Sun, 10 Sep 2017 05:51:16 -0300 Subject: [PATCH 2/3] Update changelog, refs #615 --- docs/pages/release_notes.md | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 3d9505e6a4..ec1f313a9f 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -17,12 +17,13 @@ This is a major release. * [Java Type Resolution](#java-type-resolution) * [Metrics Framework](#metrics-framework) * [Error Reporting](#error-reporting) - * [Java Symbol Table](#java-symbol-table) - * [Apex Parser Update](#apex-parser-update) * [New Rules](#new-rules) * [Modified Rules](#modified-rules) * [Deprecated Rules](#deprecated-rules) * [Removed Rules](#removed-rules) + * [Java Symbol Table](#java-symbol-table) + * [Apex Parser Update](#apex-parser-update) + * [Incremental Analysis](#incremental-analysis) * [Fixed Issues](#fixed-issues) * [API Changes](#api-changes) * [External Contributions](#external-contributions) @@ -153,6 +154,21 @@ of the latest improvements from Salesforce, but introduces some breaking changes All existing rules have been updated to reflect these changes. If you have custom rules, be sure to update them. +#### Incremental Analysis + +The incremental analysis feature first introduced in PMD 5.6.0 has been enhanced. A few minor issues have been fixed, +and several improvements have been performed to make it more accurate. + +The cache will now detect changes to the JARs referenced in the `auxclasspath` instead of simply looking at their paths +and order. This means that if you are referencing a JAR you are overwriting in some way, the incremental analysis can +now detect it and invalidat it's cache to avoid false reports. + +We have also improved logging on the analysis code, allowing better insight into how the cache is performing, +under debug / verbose builds you can even see individual hits / misses to the cache (and the reason for any miss!) + +Finally, as this feature keeps maturing, we are gently pushing this forward. If not using incremental analysis, +a warning will now be produced suggesting users to adopt it for better performance. + ### Fixed Issues * apex From 599f588e16bd233507af979549ee97a8ae5fbae7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Sun, 10 Sep 2017 06:09:57 -0300 Subject: [PATCH 3/3] Typo --- docs/pages/release_notes.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index ec1f313a9f..c303ce0da3 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -161,7 +161,7 @@ and several improvements have been performed to make it more accurate. The cache will now detect changes to the JARs referenced in the `auxclasspath` instead of simply looking at their paths and order. This means that if you are referencing a JAR you are overwriting in some way, the incremental analysis can -now detect it and invalidat it's cache to avoid false reports. +now detect it and invalidate it's cache to avoid false reports. We have also improved logging on the analysis code, allowing better insight into how the cache is performing, under debug / verbose builds you can even see individual hits / misses to the cache (and the reason for any miss!)