Skip to content

Commit

Permalink
ReplaceDuplicateStringLiterals: Improve runtime characteristics
Browse files Browse the repository at this point in the history 
Reduce the peak memory consumption as well as improve runtime performance
  • Loading branch information
@knutwannheden
knutwannheden committed Jul 4, 2024
1 parent 045bd00 commit d5a3753
Showing 1 changed file with 30 additions and 27 deletions.
57 changes: 30 additions & 27 deletions src/main/java/org/openrewrite/staticanalysis/ReplaceDuplicateStringLiterals.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import lombok.EqualsAndHashCode;
import lombok.Value;
import lombok.experimental.NonFinal;
import org.openrewrite.*;
import org.openrewrite.internal.StringUtils;
import org.openrewrite.internal.lang.Nullable;
Expand Down Expand Up @@ -76,7 +77,7 @@ public J visit(@Nullable Tree tree, ExecutionContext ctx) {
if (tree instanceof JavaSourceFile) {
JavaSourceFile cu = (JavaSourceFile) tree;
Optional<JavaSourceSet> sourceSet = cu.getMarkers().findFirst(JavaSourceSet.class);
if(!Boolean.TRUE.equals(includeTestSources) && !(sourceSet.isPresent() && "main".equals(sourceSet.get().getName()))) {
if (!Boolean.TRUE.equals(includeTestSources) && !(sourceSet.isPresent() && "main".equals(sourceSet.get().getName()))) {
return cu;
}
}
Expand All @@ -90,17 +91,20 @@ public J visitClassDeclaration(J.ClassDeclaration classDecl, ExecutionContext ct
}

DuplicateLiteralInfo duplicateLiteralInfo = DuplicateLiteralInfo.find(classDecl);
Map<String, Set<J.Literal>> duplicateLiteralsMap = duplicateLiteralInfo.getDuplicateLiterals();
Map<String, List<J.Literal>> duplicateLiteralsMap = duplicateLiteralInfo.getDuplicateLiterals();
if (duplicateLiteralsMap.isEmpty()) {
return classDecl;
}
Set<String> variableNames = duplicateLiteralInfo.getVariableNames();
Map<String, String> fieldValueToFieldName = duplicateLiteralInfo.getFieldValueToFieldName();
String classFqn = classDecl.getType().getFullyQualifiedName();
for (String valueOfLiteral : duplicateLiteralsMap.keySet()) {
Map<J.Literal, String> replacements = new HashMap<>();
for (Map.Entry<String, List<J.Literal>> entry : duplicateLiteralsMap.entrySet()) {
String valueOfLiteral = entry.getKey();
List<J.Literal> duplicateLiterals = duplicateLiteralsMap.get(valueOfLiteral);
String classFieldName = fieldValueToFieldName.get(valueOfLiteral);
String variableName;
if (fieldValueToFieldName.containsKey(valueOfLiteral)) {
String classFieldName = fieldValueToFieldName.get(valueOfLiteral);
if (classFieldName != null) {
variableName = getNameWithoutShadow(classFieldName, variableNames);
if (StringUtils.isBlank(variableName)) {
continue;
Expand All @@ -113,7 +117,7 @@ public J visitClassDeclaration(J.ClassDeclaration classDecl, ExecutionContext ct
if (StringUtils.isBlank(variableName)) {
continue;
}
J.Literal replaceLiteral = ((J.Literal) duplicateLiteralsMap.get(valueOfLiteral).toArray()[0]).withId(randomId());
J.Literal replaceLiteral = duplicateLiterals.get(0).withId(randomId());
String insertStatement = "private static final String " + variableName + " = #{any(String)};";
if (classDecl.getKind() == J.ClassDeclaration.Kind.Type.Enum) {
J.EnumValueSet enumValueSet = classDecl.getBody().getStatements().stream()
Expand All @@ -125,7 +129,7 @@ public J visitClassDeclaration(J.ClassDeclaration classDecl, ExecutionContext ct
if (enumValueSet != null) {
// "Temporary" work around due to an issue in the JavaTemplate related to BlockStatementTemplateGenerator#enumClassDeclaration.
Space singleSpace = Space.build(" ", emptyList());
Expression literal = duplicateLiteralsMap.get(valueOfLiteral).toArray(new J.Literal[0])[0].withId(randomId());
Expression literal = duplicateLiterals.get(0).withId(randomId());
J.Modifier privateModifier = new J.Modifier(randomId(), Space.build("\n", emptyList()), Markers.EMPTY, null, J.Modifier.Type.Private, emptyList());
J.Modifier staticModifier = new J.Modifier(randomId(), singleSpace, Markers.EMPTY, null, J.Modifier.Type.Static, emptyList());
J.Modifier finalModifier = new J.Modifier(randomId(), singleSpace, Markers.EMPTY, null, J.Modifier.Type.Final, emptyList());
Expand Down Expand Up @@ -181,9 +185,10 @@ public J visitClassDeclaration(J.ClassDeclaration classDecl, ExecutionContext ct
}
}
variableNames.add(variableName);
doAfterVisit(new ReplaceStringLiterals(classDecl, variableName, duplicateLiteralsMap.get(valueOfLiteral)));
entry.getValue().forEach(v -> replacements.put(v, variableName));
}
return classDecl;
return replacements.isEmpty() ? classDecl :
new ReplaceStringLiterals(classDecl, replacements).visitNonNull(classDecl, ctx, getCursor().getParent());
}

/**
Expand Down Expand Up @@ -242,20 +247,11 @@ private static boolean isPrivateStaticFinalVariable(J.VariableDeclarations decla
private static class DuplicateLiteralInfo {
Set<String> variableNames;
Map<String, String> fieldValueToFieldName;
Map<String, Set<J.Literal>> literalsMap = new HashMap<>();

public Map<String, Set<J.Literal>> getDuplicateLiterals() {
Map<String, Set<J.Literal>> filteredMap = new TreeMap<>(Comparator.reverseOrder());
for (String valueOfLiteral : literalsMap.keySet()) {
if (literalsMap.get(valueOfLiteral).size() >= 3) {
filteredMap.put(valueOfLiteral, literalsMap.get(valueOfLiteral));
}
}
return filteredMap;
}
@NonFinal
Map<String, List<J.Literal>> duplicateLiterals;

public static DuplicateLiteralInfo find(J.ClassDeclaration inClass) {
DuplicateLiteralInfo result = new DuplicateLiteralInfo(new LinkedHashSet<>(), new LinkedHashMap<>());
DuplicateLiteralInfo result = new DuplicateLiteralInfo(new LinkedHashSet<>(), new LinkedHashMap<>(), new HashMap<>());
new JavaIsoVisitor<Integer>() {

@Override
Expand Down Expand Up @@ -309,14 +305,21 @@ public J.Literal visitLiteral(J.Literal literal, Integer integer) {
parent.getValue() instanceof J.NewClass ||
parent.getValue() instanceof J.MethodInvocation) {

result.literalsMap.computeIfAbsent(((String) literal.getValue()), k -> new HashSet<>());
result.literalsMap.get((String) literal.getValue()).add(literal);
result.duplicateLiterals.computeIfAbsent(((String) literal.getValue()), k -> new ArrayList<>(1)).add(literal);
}
}
return literal;
}

}.visit(inClass, 0);
Map<String, List<J.Literal>> filteredMap = new TreeMap<>(Comparator.reverseOrder());
for (Map.Entry<String, List<J.Literal>> entry : result.duplicateLiterals.entrySet()) {
if (entry.getValue().size() >= 3) {
filteredMap.put(entry.getKey(), entry.getValue());
}
}
result.duplicateLiterals = filteredMap;

return result;
}
}
Expand All @@ -328,12 +331,12 @@ public J.Literal visitLiteral(J.Literal literal, Integer integer) {
@EqualsAndHashCode(callSuper = false)
private static class ReplaceStringLiterals extends JavaVisitor<ExecutionContext> {
J.ClassDeclaration isClass;
String variableName;
Set<J.Literal> literals;
Map<J.Literal, String> replacements;

@Override
public J visitLiteral(J.Literal literal, ExecutionContext ctx) {
if (literals.contains(literal)) {
String variableName = replacements.get(literal);
if (variableName != null) {
assert isClass.getType() != null;
return new J.Identifier(
randomId(),
Expand All @@ -344,7 +347,7 @@ public J visitLiteral(J.Literal literal, ExecutionContext ctx) {
JavaType.Primitive.String,
new JavaType.Variable(
null,
Flag.flagsToBitMap(new HashSet<>(Arrays.asList(Flag.Private, Flag.Static, Flag.Final))),
Flag.flagsToBitMap(EnumSet.of(Flag.Private, Flag.Static, Flag.Final)),
variableName,
isClass.getType(),
JavaType.Primitive.String,
Expand Down

0 comments on commit d5a3753

Please sign in to comment.