From f1fb1b0a67b9e089bef692040a21234d4cad20cd Mon Sep 17 00:00:00 2001 From: Andrey Turbanov Date: Thu, 19 Jan 2023 23:04:00 +0300 Subject: [PATCH] Avoid unnecessary Map.containsKey calls. Map.containsKey calls are often unnecessary. Depends on the code, we can: 1. call get() and then compare result with null. Applicable if we know that Map don't contain null value 2. use putIfAbsent()/computeIfAbsent() if containsKey() was used before put 3. use getOrDefault() if we have "fallback" value --- .../context/CompilationAndWeavingContext.java | 6 +----- .../compiler/ast/AccessForInlineVisitor.java | 20 +++++++++++-------- .../compiler/lookup/AjLookupEnvironment.java | 5 +++-- .../compiler/lookup/EclipseFactory.java | 10 ++++++---- .../compiler/lookup/PrivilegedHandler.java | 12 +++++++---- .../internal/core/builder/AjBuildManager.java | 12 ++++------- .../core/builder/StatefulNameEnvironment.java | 5 +++-- .../weaver/CrosscuttingMembersSet.java | 4 ++-- .../aspectj/weaver/ResolvedMemberImpl.java | 7 ++----- .../java/org/aspectj/weaver/ResolvedType.java | 5 +++-- .../main/java/org/aspectj/weaver/World.java | 5 +++-- .../weaver/patterns/ExactTypePattern.java | 4 +--- .../weaver/patterns/WildTypePattern.java | 5 +++-- .../loadtime/definition/DocumentParser.java | 7 +++++-- 14 files changed, 56 insertions(+), 51 deletions(-) diff --git a/bridge/src/main/java/org/aspectj/bridge/context/CompilationAndWeavingContext.java b/bridge/src/main/java/org/aspectj/bridge/context/CompilationAndWeavingContext.java index bfb8ec190c..c914bf9f99 100644 --- a/bridge/src/main/java/org/aspectj/bridge/context/CompilationAndWeavingContext.java +++ b/bridge/src/main/java/org/aspectj/bridge/context/CompilationAndWeavingContext.java @@ -196,11 +196,7 @@ private static ContextTokenImpl nextToken() { private static ContextFormatter getFormatter(ContextStackEntry entry) { Integer key = entry.phaseId; - if (formatterMap.containsKey(key)) { - return formatterMap.get(key); - } else { - return defaultFormatter; - } + return formatterMap.getOrDefault(key, defaultFormatter); } private static class ContextTokenImpl implements ContextToken { diff --git a/org.aspectj.ajdt.core/src/main/java/org/aspectj/ajdt/internal/compiler/ast/AccessForInlineVisitor.java b/org.aspectj.ajdt.core/src/main/java/org/aspectj/ajdt/internal/compiler/ast/AccessForInlineVisitor.java index 8e06a586c7..00127ca459 100644 --- a/org.aspectj.ajdt.core/src/main/java/org/aspectj/ajdt/internal/compiler/ast/AccessForInlineVisitor.java +++ b/org.aspectj.ajdt.core/src/main/java/org/aspectj/ajdt/internal/compiler/ast/AccessForInlineVisitor.java @@ -179,10 +179,11 @@ private FieldBinding getAccessibleField(FieldBinding binding, TypeBinding receiv alreadyResolvedMembers.put(binding, m); } - if (inAspect.accessForInline.containsKey(m)) { - return (FieldBinding) inAspect.accessForInline.get(m); + FieldBinding ret = (FieldBinding) inAspect.accessForInline.get(m); + if (ret != null) { + return ret; } - FieldBinding ret = new InlineAccessFieldBinding(inAspect, binding, m); + ret = new InlineAccessFieldBinding(inAspect, binding, m); inAspect.accessForInline.put(m, ret); return ret; } @@ -216,9 +217,11 @@ private MethodBinding getAccessibleMethod(MethodBinding binding, TypeBinding rec // runtime this will be satisfied by the super). m = world.makeResolvedMember(binding, receiverType); } - if (inAspect.accessForInline.containsKey(m)) - return (MethodBinding) inAspect.accessForInline.get(m); - MethodBinding ret = world.makeMethodBinding(AjcMemberMaker.inlineAccessMethodForMethod(inAspect.typeX, m)); + MethodBinding ret = (MethodBinding) inAspect.accessForInline.get(m); + if (ret != null) { + return ret; + } + ret = world.makeMethodBinding(AjcMemberMaker.inlineAccessMethodForMethod(inAspect.typeX, m)); inAspect.accessForInline.put(m, ret); return ret; } @@ -236,8 +239,9 @@ public SuperAccessMethodPair(ResolvedMember originalMethod, MethodBinding access private MethodBinding getSuperAccessMethod(MethodBinding binding) { ResolvedMember m = world.makeResolvedMember(binding); ResolvedMember superAccessMember = AjcMemberMaker.superAccessMethod(inAspect.typeX, m); - if (inAspect.superAccessForInline.containsKey(superAccessMember)) { - return inAspect.superAccessForInline.get(superAccessMember).accessMethod; + SuperAccessMethodPair pair = inAspect.superAccessForInline.get(superAccessMember); + if (pair != null) { + return pair.accessMethod; } MethodBinding ret = world.makeMethodBinding(superAccessMember); inAspect.superAccessForInline.put(superAccessMember, new SuperAccessMethodPair(m, ret)); diff --git a/org.aspectj.ajdt.core/src/main/java/org/aspectj/ajdt/internal/compiler/lookup/AjLookupEnvironment.java b/org.aspectj.ajdt.core/src/main/java/org/aspectj/ajdt/internal/compiler/lookup/AjLookupEnvironment.java index 6542ac8285..ea9dd799a9 100644 --- a/org.aspectj.ajdt.core/src/main/java/org/aspectj/ajdt/internal/compiler/lookup/AjLookupEnvironment.java +++ b/org.aspectj.ajdt.core/src/main/java/org/aspectj/ajdt/internal/compiler/lookup/AjLookupEnvironment.java @@ -885,9 +885,10 @@ private boolean doDeclareParents(DeclareParents declareParents, SourceTypeBindin List newParents = declareParents.findMatchingNewParents(resolvedSourceType, false); if (!newParents.isEmpty()) { for (ResolvedType parent : newParents) { - if (dangerousInterfaces.containsKey(parent)) { + String dangerous = dangerousInterfaces.get(parent); + if (dangerous != null) { ResolvedType onType = factory.fromEclipse(sourceType); - factory.showMessage(IMessage.ERROR, onType + ": " + dangerousInterfaces.get(parent), + factory.showMessage(IMessage.ERROR, onType + ": " + dangerous, onType.getSourceLocation(), null); } if (Modifier.isFinal(parent.getModifiers())) { diff --git a/org.aspectj.ajdt.core/src/main/java/org/aspectj/ajdt/internal/compiler/lookup/EclipseFactory.java b/org.aspectj.ajdt.core/src/main/java/org/aspectj/ajdt/internal/compiler/lookup/EclipseFactory.java index dadb3bb091..17ec277227 100644 --- a/org.aspectj.ajdt.core/src/main/java/org/aspectj/ajdt/internal/compiler/lookup/EclipseFactory.java +++ b/org.aspectj.ajdt.core/src/main/java/org/aspectj/ajdt/internal/compiler/lookup/EclipseFactory.java @@ -368,8 +368,9 @@ private boolean hasAnyArguments(ParameterizedTypeBinding ptb) { */ private UnresolvedType fromTypeVariableBinding(TypeVariableBinding aTypeVariableBinding) { // first, check for recursive call to this method for the same tvBinding - if (typeVariableBindingsInProgress.containsKey(aTypeVariableBinding)) { - return typeVariableBindingsInProgress.get(aTypeVariableBinding); + UnresolvedType alreadyInProgress = typeVariableBindingsInProgress.get(aTypeVariableBinding); + if (alreadyInProgress != null) { + return alreadyInProgress; } // Check if its a type variable binding that we need to recover to an alias... @@ -382,8 +383,9 @@ private UnresolvedType fromTypeVariableBinding(TypeVariableBinding aTypeVariable } } - if (typeVariablesForThisMember.containsKey(new String(aTypeVariableBinding.sourceName))) { - return typeVariablesForThisMember.get(new String(aTypeVariableBinding.sourceName)); + UnresolvedType typeVarForThis = typeVariablesForThisMember.get(new String(aTypeVariableBinding.sourceName)); + if (typeVarForThis != null) { + return typeVarForThis; } // Create the UnresolvedTypeVariableReferenceType for the type variable diff --git a/org.aspectj.ajdt.core/src/main/java/org/aspectj/ajdt/internal/compiler/lookup/PrivilegedHandler.java b/org.aspectj.ajdt.core/src/main/java/org/aspectj/ajdt/internal/compiler/lookup/PrivilegedHandler.java index ad9e8374b7..e9764fef85 100644 --- a/org.aspectj.ajdt.core/src/main/java/org/aspectj/ajdt/internal/compiler/lookup/PrivilegedHandler.java +++ b/org.aspectj.ajdt.core/src/main/java/org/aspectj/ajdt/internal/compiler/lookup/PrivilegedHandler.java @@ -59,8 +59,10 @@ public FieldBinding getPrivilegedAccessField(FieldBinding baseField, ASTNode loc baseField = ((ParameterizedFieldBinding) baseField).originalField; } ResolvedMember key = inAspect.factory.makeResolvedMember(baseField); - if (accessors.containsKey(key)) - return (FieldBinding) accessors.get(key); + FieldBinding fieldBinding = (FieldBinding) accessors.get(key); + if (fieldBinding != null) { + return fieldBinding; + } FieldBinding ret = new PrivilegedFieldBinding(inAspect, baseField); checkWeaveAccess(key.getDeclaringType(), location); if (!baseField.alwaysNeedsAccessMethod(true)) @@ -89,8 +91,10 @@ public MethodBinding getPrivilegedAccessMethod(MethodBinding baseMethod, ASTNode } else { key = inAspect.factory.makeResolvedMember(baseMethod); } - if (accessors.containsKey(key)) - return (MethodBinding) accessors.get(key); + MethodBinding methodBinding = (MethodBinding) accessors.get(key); + if (methodBinding != null) { + return methodBinding; + } MethodBinding ret; if (baseMethod.isConstructor()) { diff --git a/org.aspectj.ajdt.core/src/main/java/org/aspectj/ajdt/internal/core/builder/AjBuildManager.java b/org.aspectj.ajdt.core/src/main/java/org/aspectj/ajdt/internal/core/builder/AjBuildManager.java index 4f45959df7..8a9795026c 100644 --- a/org.aspectj.ajdt.core/src/main/java/org/aspectj/ajdt/internal/core/builder/AjBuildManager.java +++ b/org.aspectj.ajdt.core/src/main/java/org/aspectj/ajdt/internal/core/builder/AjBuildManager.java @@ -697,7 +697,7 @@ private ByteArrayOutputStream getOutxmlContents(List aspectNames) { /** * Returns a map where the keys are File objects corresponding to all the output directories and the values are a list of - * aspects which are sent to that ouptut directory + * aspects which are sent to that output directory */ private Map> findOutputDirsForAspects() { Map> outputDirsToAspects = new HashMap<>(); @@ -729,10 +729,8 @@ private Map> findOutputDirsForAspects() { char[] fileName = entry.getValue(); File outputDir = buildConfig.getCompilationResultDestinationManager().getOutputLocationForClass( new File(new String(fileName))); - if (!outputDirsToAspects.containsKey(outputDir)) { - outputDirsToAspects.put(outputDir, new ArrayList<>()); - } - outputDirsToAspects.get(outputDir).add(aspectName); + outputDirsToAspects.computeIfAbsent(outputDir, f -> new ArrayList<>()) + .add(aspectName); } } } @@ -1243,9 +1241,7 @@ private void addAspectName(String name, char[] fileContainingAspect) { if (state.getAspectNamesToFileNameMap() == null) { state.initializeAspectNamesToFileNameMap(); } - if (!state.getAspectNamesToFileNameMap().containsKey(name)) { - state.getAspectNamesToFileNameMap().put(name, fileContainingAspect); - } + state.getAspectNamesToFileNameMap().putIfAbsent(name, fileContainingAspect); } } }; diff --git a/org.aspectj.ajdt.core/src/main/java/org/aspectj/ajdt/internal/core/builder/StatefulNameEnvironment.java b/org.aspectj.ajdt.core/src/main/java/org/aspectj/ajdt/internal/core/builder/StatefulNameEnvironment.java index dad61e7b23..7094502f86 100644 --- a/org.aspectj.ajdt.core/src/main/java/org/aspectj/ajdt/internal/core/builder/StatefulNameEnvironment.java +++ b/org.aspectj.ajdt.core/src/main/java/org/aspectj/ajdt/internal/core/builder/StatefulNameEnvironment.java @@ -63,8 +63,9 @@ private NameEnvironmentAnswer findType(String name) { if (seenOnPreviousBuild != null) { return new NameEnvironmentAnswer(seenOnPreviousBuild, null); } - if (this.inflatedClassFilesCache.containsKey(name)) { - return this.inflatedClassFilesCache.get(name); + NameEnvironmentAnswer cached = this.inflatedClassFilesCache.get(name); + if (cached != null) { + return cached; } else { File fileOnDisk = classesFromName.get(name); // System.err.println("find: " + name + " found: " + cf); diff --git a/org.aspectj.matcher/src/main/java/org/aspectj/weaver/CrosscuttingMembersSet.java b/org.aspectj.matcher/src/main/java/org/aspectj/weaver/CrosscuttingMembersSet.java index 2a06b2113a..856025f134 100644 --- a/org.aspectj.matcher/src/main/java/org/aspectj/weaver/CrosscuttingMembersSet.java +++ b/org.aspectj.matcher/src/main/java/org/aspectj/weaver/CrosscuttingMembersSet.java @@ -154,10 +154,10 @@ private boolean addOrReplaceDescendantsOf(ResolvedType aspectType, boolean inWea } public void addAdviceLikeDeclares(ResolvedType aspectType) { - if (!members.containsKey(aspectType)) { + CrosscuttingMembers xcut = members.get(aspectType); + if (xcut == null) { return; } - CrosscuttingMembers xcut = members.get(aspectType); xcut.addDeclares(aspectType.collectDeclares(true)); } diff --git a/org.aspectj.matcher/src/main/java/org/aspectj/weaver/ResolvedMemberImpl.java b/org.aspectj.matcher/src/main/java/org/aspectj/weaver/ResolvedMemberImpl.java index ffc9da8174..9fa391ccc2 100644 --- a/org.aspectj.matcher/src/main/java/org/aspectj/weaver/ResolvedMemberImpl.java +++ b/org.aspectj.matcher/src/main/java/org/aspectj/weaver/ResolvedMemberImpl.java @@ -881,11 +881,8 @@ protected UnresolvedType parameterize(UnresolvedType aType, Map getDeclaredAdvice() { for (int j = 0; j < ptypes.length; j++) { if (ptypes[j] instanceof TypeVariableReferenceType) { TypeVariableReferenceType tvrt = (TypeVariableReferenceType) ptypes[j]; - if (typeVariableMap.containsKey(tvrt.getTypeVariable().getName())) { - newPTypes[j] = typeVariableMap.get(tvrt.getTypeVariable().getName()); + UnresolvedType newPType = typeVariableMap.get(tvrt.getTypeVariable().getName()); + if (newPType != null) { + newPTypes[j] = newPType; } else { newPTypes[j] = ptypes[j]; } diff --git a/org.aspectj.matcher/src/main/java/org/aspectj/weaver/World.java b/org.aspectj.matcher/src/main/java/org/aspectj/weaver/World.java index 6eab96f150..43641605c7 100644 --- a/org.aspectj.matcher/src/main/java/org/aspectj/weaver/World.java +++ b/org.aspectj.matcher/src/main/java/org/aspectj/weaver/World.java @@ -1427,8 +1427,9 @@ public AspectPrecedenceCalculator(World forSomeWorld) { */ public int compareByPrecedence(ResolvedType firstAspect, ResolvedType secondAspect) { PrecedenceCacheKey key = new PrecedenceCacheKey(firstAspect, secondAspect); - if (cachedResults.containsKey(key)) { - return cachedResults.get(key); + Integer cachedOrder = cachedResults.get(key); + if (cachedOrder != null) { + return cachedOrder; } else { int order = 0; DeclarePrecedence orderer = null; // Records the declare diff --git a/org.aspectj.matcher/src/main/java/org/aspectj/weaver/patterns/ExactTypePattern.java b/org.aspectj.matcher/src/main/java/org/aspectj/weaver/patterns/ExactTypePattern.java index 6a747a3eca..44edd4acc5 100644 --- a/org.aspectj.matcher/src/main/java/org/aspectj/weaver/patterns/ExactTypePattern.java +++ b/org.aspectj.matcher/src/main/java/org/aspectj/weaver/patterns/ExactTypePattern.java @@ -336,9 +336,7 @@ public TypePattern parameterizeWith(Map typeVariableMap, if (type.isTypeVariableReference()) { TypeVariableReference t = (TypeVariableReference) type; String key = t.getTypeVariable().getName(); - if (typeVariableMap.containsKey(key)) { - newType = typeVariableMap.get(key); - } + newType = typeVariableMap.getOrDefault(key, type); } else if (type.isParameterizedType()) { newType = w.resolve(type).parameterize(typeVariableMap); } diff --git a/org.aspectj.matcher/src/main/java/org/aspectj/weaver/patterns/WildTypePattern.java b/org.aspectj.matcher/src/main/java/org/aspectj/weaver/patterns/WildTypePattern.java index 212378bf71..ee64b1cd19 100644 --- a/org.aspectj.matcher/src/main/java/org/aspectj/weaver/patterns/WildTypePattern.java +++ b/org.aspectj.matcher/src/main/java/org/aspectj/weaver/patterns/WildTypePattern.java @@ -582,8 +582,9 @@ public TypePattern parameterizeWith(Map typeVariableMap, if (newNamePatterns.length == 1) { String simpleName = newNamePatterns[0].maybeGetSimpleName(); if (simpleName != null) { - if (typeVariableMap.containsKey(simpleName)) { - String newName = ((ReferenceType) typeVariableMap.get(simpleName)).getName().replace('$', '.'); + UnresolvedType unresolvedType = typeVariableMap.get(simpleName); + if (unresolvedType != null) { + String newName = ((ReferenceType) unresolvedType).getName().replace('$', '.'); StringTokenizer strTok = new StringTokenizer(newName, "."); newNamePatterns = new NamePattern[strTok.countTokens()]; int index = 0; diff --git a/weaver/src/main/java/org/aspectj/weaver/loadtime/definition/DocumentParser.java b/weaver/src/main/java/org/aspectj/weaver/loadtime/definition/DocumentParser.java index eddc5bd329..edc0749fb3 100644 --- a/weaver/src/main/java/org/aspectj/weaver/loadtime/definition/DocumentParser.java +++ b/weaver/src/main/java/org/aspectj/weaver/loadtime/definition/DocumentParser.java @@ -111,8 +111,11 @@ private DocumentParser() { } public static Definition parse(final URL url) throws Exception { - if (CACHE && parsedFiles.containsKey(url.toString())) { - return parsedFiles.get(url.toString()); + if (CACHE) { + Definition cached = parsedFiles.get(url.toString()); + if (cached != null) { + return cached; + } } Definition def = null;