From 932ad7dd2b82feeeada8eaff815430b47de47795 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Tue, 13 Dec 2016 18:47:06 -0300 Subject: [PATCH] Reduce memory allocations during symbol table --- .../lang/symboltable/ImageFinderFunction.java | 7 +-- .../pmd/lang/java/symboltable/ClassScope.java | 46 +++++++++++++------ .../pmd/lang/java/symboltable/LocalScope.java | 5 +- .../lang/java/symboltable/MethodScope.java | 9 ++-- .../java/symboltable/OccurrenceFinder.java | 12 +++-- .../java/symboltable/SourceFileScope.java | 16 ++++--- .../pmd/lang/java/symboltable/TypeSet.java | 30 ++++++++++-- 7 files changed, 84 insertions(+), 41 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/symboltable/ImageFinderFunction.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/symboltable/ImageFinderFunction.java index c681980f17..b9e14141a5 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/symboltable/ImageFinderFunction.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/symboltable/ImageFinderFunction.java @@ -4,6 +4,7 @@ package net.sourceforge.pmd.lang.symboltable; +import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -12,15 +13,15 @@ import net.sourceforge.pmd.util.SearchFunction; public class ImageFinderFunction implements SearchFunction { - private Set images = new HashSet<>(); + private final Set images; private NameDeclaration decl; public ImageFinderFunction(String img) { - images.add(img); + images = Collections.singleton(img); } public ImageFinderFunction(List imageList) { - images.addAll(imageList); + images = new HashSet<>(imageList); } @Override diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/ClassScope.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/ClassScope.java index 37755df42c..7823895d46 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/ClassScope.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/ClassScope.java @@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.symboltable; import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; -import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Objects; @@ -253,7 +252,7 @@ public class ClassScope extends AbstractJavaScope { Map> classDeclarations = getClassDeclarations(); if (result.isEmpty() && !classDeclarations.isEmpty()) { for (ClassNameDeclaration innerClass : getClassDeclarations().keySet()) { - Applier.apply(finder, innerClass.getScope().getDeclarations().keySet().iterator()); + Applier.apply(finder, innerClass.getScope().getDeclarations(VariableNameDeclaration.class).keySet().iterator()); if (finder.getDecl() != null) { result.add(finder.getDecl()); } @@ -287,19 +286,24 @@ public class ClassScope extends AbstractJavaScope { methodDeclarator.jjtAddChild(formalParameters, 0); formalParameters.jjtSetParent(methodDeclarator); - for (int i = 0; i < parameterTypes.length; i++) { + /* + * jjtAddChild resizes it's child node list according to known indexes. + * Going backwards makes sure the first time it gets the right size avoiding copies. + */ + for (int i = parameterTypes.length - 1; i >= 0; i--) { ASTFormalParameter formalParameter = new ASTFormalParameter(JavaParserTreeConstants.JJTFORMALPARAMETER); formalParameters.jjtAddChild(formalParameter, i); formalParameter.jjtSetParent(formalParameters); - ASTType type = new ASTType(JavaParserTreeConstants.JJTTYPE); - formalParameter.jjtAddChild(type, 0); - type.jjtSetParent(formalParameter); - - ASTVariableDeclaratorId variableDeclaratorId = new ASTVariableDeclaratorId(JavaParserTreeConstants.JJTVARIABLEDECLARATORID); + ASTVariableDeclaratorId variableDeclaratorId = new ASTVariableDeclaratorId( + JavaParserTreeConstants.JJTVARIABLEDECLARATORID); variableDeclaratorId.setImage("arg" + i); formalParameter.jjtAddChild(variableDeclaratorId, 1); variableDeclaratorId.jjtSetParent(formalParameter); + + ASTType type = new ASTType(JavaParserTreeConstants.JJTTYPE); + formalParameter.jjtAddChild(type, 0); + type.jjtSetParent(formalParameter); if (PRIMITIVE_TYPES.contains(parameterTypes[i])) { ASTPrimitiveType primitiveType = new ASTPrimitiveType(JavaParserTreeConstants.JJTPRIMITIVETYPE); @@ -363,20 +367,34 @@ public class ClassScope extends AbstractJavaScope { return null; } - Set qualifiedNames = new LinkedHashSet<>(); - qualifiedNames.addAll(this.getEnclosingScope(SourceFileScope.class).getQualifiedTypeNames().keySet()); - qualifiedNames.addAll(this.getEnclosingScope(SourceFileScope.class).getExplicitImports()); + final SourceFileScope fileScope = getEnclosingScope(SourceFileScope.class); + // Is it an inner class being accessed? + String qualified = findQualifiedName(typeImage, fileScope.getQualifiedTypeNames().keySet()); + if (qualified != null) { + return qualified; + } + + // Is it an explicit import? + qualified = findQualifiedName(typeImage, fileScope.getExplicitImports()); + if (qualified != null) { + return qualified; + } + + return typeImage; + } + + private String findQualifiedName(String typeImage, Set candidates) { int nameLength = typeImage.length(); - - for (String qualified : qualifiedNames) { + for (String qualified : candidates) { int fullLength = qualified.length(); if (qualified.endsWith(typeImage) && (fullLength == nameLength || qualified.charAt(fullLength - nameLength - 1) == '.')) { return qualified; } } - return typeImage; + + return null; } /** diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/LocalScope.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/LocalScope.java index 364a6bfd6b..87ff84c764 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/LocalScope.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/LocalScope.java @@ -4,7 +4,6 @@ package net.sourceforge.pmd.lang.java.symboltable; import java.util.Collections; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -57,9 +56,9 @@ public class LocalScope extends AbstractJavaScope { DeclarationFinderFunction finder = new DeclarationFinderFunction(occurrence); Applier.apply(finder, getVariableDeclarations().keySet().iterator()); if (finder.getDecl() != null) { - result.add(finder.getDecl()); + return Collections.singleton(finder.getDecl()); } - return result; + return Collections.emptySet(); } public String toString() { diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/MethodScope.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/MethodScope.java index c519c4d0f3..7dd10e0aad 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/MethodScope.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/MethodScope.java @@ -3,7 +3,7 @@ */ package net.sourceforge.pmd.lang.java.symboltable; -import java.util.HashSet; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Set; @@ -54,16 +54,15 @@ public class MethodScope extends AbstractJavaScope { } public Set findVariableHere(JavaNameOccurrence occurrence) { - Set result = new HashSet<>(); if (occurrence.isThisOrSuper() || occurrence.isMethodOrConstructorInvocation()) { - return result; + return Collections.emptySet(); } ImageFinderFunction finder = new ImageFinderFunction(occurrence.getImage()); Applier.apply(finder, getVariableDeclarations().keySet().iterator()); if (finder.getDecl() != null) { - result.add(finder.getDecl()); + return Collections.singleton(finder.getDecl()); } - return result; + return Collections.emptySet(); } public String getName() { diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/OccurrenceFinder.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/OccurrenceFinder.java index 25e33d37b6..cde5b5b014 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/OccurrenceFinder.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/OccurrenceFinder.java @@ -14,12 +14,17 @@ import net.sourceforge.pmd.lang.symboltable.Scope; public class OccurrenceFinder extends JavaParserVisitorAdapter { + // Maybe do some sort of State pattern thingy for when NameDeclaration + // is empty/not empty? + private final Set declarations = new HashSet<>(); + + private final Set additionalDeclarations = new HashSet<>(); + public Object visit(ASTPrimaryExpression node, Object data) { NameFinder nameFinder = new NameFinder(node); - // Maybe do some sort of State pattern thingy for when NameDeclaration - // is empty/not empty? - Set declarations = new HashSet<>(); + declarations.clear(); + additionalDeclarations.clear(); List names = nameFinder.getNames(); for (JavaNameOccurrence occ : names) { @@ -36,7 +41,6 @@ public class OccurrenceFinder extends JavaParserVisitorAdapter { break; } } else { - Set additionalDeclarations = new HashSet<>(); for (NameDeclaration decl : declarations) { // now we've got a scope we're starting with, so work from there Scope startingScope = decl.getScope(); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/SourceFileScope.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/SourceFileScope.java index f84ceaebc9..3c35d010f0 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/SourceFileScope.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/SourceFileScope.java @@ -3,10 +3,8 @@ */ package net.sourceforge.pmd.lang.java.symboltable; - import java.util.Collections; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -113,13 +111,12 @@ public class SourceFileScope extends AbstractJavaScope { } protected Set findVariableHere(JavaNameOccurrence occ) { - Set result = new HashSet<>(); ImageFinderFunction finder = new ImageFinderFunction(occ.getImage()); Applier.apply(finder, getDeclarations().keySet().iterator()); if (finder.getDecl() != null) { - result.add(finder.getDecl()); + return Collections.singleton(finder.getDecl()); } - return result; + return Collections.emptySet(); } /** @@ -132,8 +129,13 @@ public class SourceFileScope extends AbstractJavaScope { } private Map getSubTypes(String qualifyingName, Scope subType) { - Map types = new HashMap<>(); - for (ClassNameDeclaration c : subType.getDeclarations(ClassNameDeclaration.class).keySet()) { + Set classDeclarations = subType.getDeclarations(ClassNameDeclaration.class).keySet(); + if (classDeclarations.isEmpty()) { + return Collections.emptyMap(); + } + + Map types = new HashMap<>((int) (classDeclarations.size() / 0.75f) + 1); + for (ClassNameDeclaration c : classDeclarations) { String typeName = c.getName(); if (qualifyingName != null) { typeName = qualifyingName + "." + typeName; diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/TypeSet.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/TypeSet.java index 087b12f977..02ea9ff0e2 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/TypeSet.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/TypeSet.java @@ -206,7 +206,7 @@ public class TypeSet { * Resolver that uses the current package to resolve a simple class name. */ public static class CurrentPackageResolver extends AbstractResolver { - private String pkg; + private final String pkg; /** * Creates a new {@link CurrentPackageResolver} @@ -215,11 +215,19 @@ public class TypeSet { */ public CurrentPackageResolver(PMDASMClassLoader pmdClassLoader, String pkg) { super(pmdClassLoader); - this.pkg = pkg; + if (pkg == null) { + this.pkg = null; + } else { + this.pkg = pkg + "."; + } } @Override public Class resolve(String name) throws ClassNotFoundException { + if (name == null) { + throw new ClassNotFoundException(); + } + final String fqName = qualifyName(name); final Class c = resolveMaybeInner(fqName, fqName); @@ -240,7 +248,11 @@ public class TypeSet { return name; } - return pkg + '.' + name; + /* + * String.concat is bad in general, but for simple 2 string concatenation, it's the fastest + * See http://www.rationaljava.com/2015/02/the-optimum-method-to-concatenate.html + */ + return pkg.concat(name); } } @@ -275,7 +287,11 @@ public class TypeSet { return clazz; } - clazz = pmdClassLoader.loadClass("java.lang." + name); + /* + * String.concat is bad in general, but for simple 2 string concatenation, it's the fastest + * See http://www.rationaljava.com/2015/02/the-optimum-method-to-concatenate.html + */ + clazz = pmdClassLoader.loadClass("java.lang.".concat(name)); CLASS_CACHE.putIfAbsent(name, clazz); return clazz; @@ -283,7 +299,11 @@ public class TypeSet { @Override public boolean couldResolve(String name) { - return super.couldResolve("java.lang." + name); + /* + * String.concat is bad in general, but for simple 2 string concatenation, it's the fastest + * See http://www.rationaljava.com/2015/02/the-optimum-method-to-concatenate.html + */ + return super.couldResolve("java.lang.".concat(name)); } }