diff --git a/dataflow/src/main/java/org/checkerframework/dataflow/cfg/node/LocalVariableNode.java b/dataflow/src/main/java/org/checkerframework/dataflow/cfg/node/LocalVariableNode.java index 734522a797b1..c563fabd2200 100644 --- a/dataflow/src/main/java/org/checkerframework/dataflow/cfg/node/LocalVariableNode.java +++ b/dataflow/src/main/java/org/checkerframework/dataflow/cfg/node/LocalVariableNode.java @@ -6,12 +6,13 @@ import org.checkerframework.checker.nullness.qual.Nullable; import org.checkerframework.dataflow.qual.SideEffectFree; +import org.checkerframework.javacutil.InternalUtils; import org.checkerframework.javacutil.TreeUtils; import java.util.Collection; import java.util.Collections; -import java.util.Objects; +import javax.lang.model.element.Name; import javax.lang.model.element.VariableElement; /** @@ -112,12 +113,24 @@ public boolean equals(@Nullable Object obj) { return false; } LocalVariableNode other = (LocalVariableNode) obj; - return getName().equals(other.getName()); + Name thisName = + (tree instanceof IdentifierTree) + ? ((IdentifierTree) tree).getName() + : ((VariableTree) tree).getName(); + Name otherName = + (other.tree instanceof IdentifierTree) + ? ((IdentifierTree) other.tree).getName() + : ((VariableTree) other.tree).getName(); + return InternalUtils.sameName(thisName, otherName); } @Override public int hashCode() { - return Objects.hash(getName()); + Name name = + (tree instanceof IdentifierTree) + ? ((IdentifierTree) tree).getName() + : ((VariableTree) tree).getName(); + return name.hashCode(); } @Override diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index cebe9441f372..71a8dff33fcd 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -112,6 +112,46 @@ Fixed a bug that caused an IndexOutOfBoundsException for lambdas in varargs, for type systems that had the Aliasing Checker as a subchecker, like the Optional Checker. +Performance: `BaseTypeVisitor.FoundRequired.found` and `.required` are now +typed as `Object` instead of `String`; their `toString()` returns the +lazily-formatted annotated-type string. This defers `AnnotatedTypeMirror.toString()` +(which traverses the type structure and decodes UTF-8 names) until a diagnostic is +actually emitted, so suppressed errors pay no formatting cost. **API change:** +callers that read these fields into a `String` variable must call `.toString()` +explicitly. + +Performance: `AnnotationFileParser.annosInPackage`, `annosInType`, and +`createNameToAnnotationMap` now return +`IdentityHashMap` instead of +`Map`, keyed by the live `Name` objects from the +compilation's name table. The same change applies to +`InsertAjavaAnnotations.FileState.allAnnotations` and the `TypeAnnotationMover` +constructor parameter. Within one compilation's `Elements` instance, same-content +names are interned to the same object, so identity comparison is correct and avoids +UTF-8 decodes on every annotation lookup. **API changes:** callers of the three +`AnnotationFileParser` methods and the `TypeAnnotationMover` constructor must +update their declared types from `Map` to +`IdentityHashMap`. + +Performance: `SourceChecker.shouldSkipUses(Element)` now caches results per +enclosing-class qualified `Name`, avoiding a repeated `Symbol.toString()` UTF-8 +decode and regex match for every element within the same class. No behavior change. + +Performance: `LocalVariableNode.hashCode()` and `equals()` now read the variable +name as a `Name` directly from the tree instead of going through `getName()` (which +decodes to `String`). `equals` uses `InternalUtils.sameName`; `hashCode` uses +`Name.hashCode()` directly (which returns the name-table byte offset — no decode). +No behavior change. + +Performance: `Variable.computeHashCode` and `ProperType.computeHashCode` in the +type-inference-8 subsystem no longer call `toString()` to compute a hash; they use +`Name.hashCode()` and `TypeKind` instead. No behavior change. + +Together the above changes (PR #1797) reduce `Convert.utf2chars` + +`Convert.utf2string` self-time from ~2.3% to ~0.89% of a full `checknullness` +build; wall-clock A/B shows ~5% improvement on `checknullness` (~135 s → ~128 s, +median of four warm-daemon reps per side). + **Closed issues:** diff --git a/docs/developer/performance-notes.md b/docs/developer/performance-notes.md index 06d32c23bc94..ba0844dd2b96 100644 --- a/docs/developer/performance-notes.md +++ b/docs/developer/performance-notes.md @@ -378,6 +378,21 @@ so small per-call wins paid back substantially. garbage. Correctness re-verified with `allNullnessTests`, `IndexTest`, `SignatureTest`, `NullnessTest`, `InterningTest`, `ValueTest`, and the `:checker:test`, `:framework:test`, `:javacutil:test`, `:dataflow:test` suites. +- **PR #1797 — `IdentityHashMap` for annotation name maps (June 2026).** + The annotation-name lookup maps in `AnnotationFileParser`, `InsertAjavaAnnotations`, and + `TypeAnnotationMover` previously used `HashMap`, requiring `Name.toString()` + (a UTF-8 decode on byte-backed tables) at every map-build site. Changed to + `IdentityHashMap`: keys are the `Name` objects returned by + `getSimpleName()` / `getQualifiedName()` directly, and lookups use `elements.getName(s)` to + intern a JavaParser `String` into the same table, guaranteeing identity equality within one + compilation. Also removed a redundant `elements.getName(annoElt.getSimpleName())` call in + `AnnotationFileParser.getImportedAnnotations` — `getSimpleName()` already returns an interned + `Name` from the same table, so the round-trip was a decode-and-re-intern no-op. Safety: all maps + are built and consumed within a single compilation's `Elements` instance, so same-table identity + holds; the `getAnnotation` fallback (`elements.getTypeElement(fqn)` + `createNameToAnnotationMap`) + handles first-encounter FQN annotations and populates both simple-name and FQN entries for future + hits. + - **PR #1776** — *Avoid the defensive deep copy in read-only `fromElement` consumers.* `AnnotatedTypeFactory.fromElement` returns `cached.deepCopy()` on every cache hit so callers may mutate the result; this is @@ -429,6 +444,15 @@ so small per-call wins paid back substantially. gap; `FieldAccess` and similar pay +8 bytes. Peak overhead measured at ~128 bytes for a large method, well worth the savings on store- comparison hot paths. +- **PR #1797 — `LocalVariableNode.hashCode`/`equals` avoid `getName()` and `Objects.hash()` (June 2026).** + Both methods previously called `getName()`, which calls `Name.toString()` (a UTF-8 decode on + byte-backed tables). Changed to read the `Name` directly from the tree + (`IdentifierTree.getName()` / `VariableTree.getName()`) for both operations: `equals` uses + `InternalUtils.sameName`; `hashCode` calls `name.hashCode()` directly — on `SharedNameTable` + this returns the byte-table `index`, which is content-stable via interning, no decode needed. + Also removes the `Objects.hash(name)` varargs call, which allocated an `Object[]` per invocation + (the varargs antipattern flagged in Applied optimizations → Generic map/lookup patterns). + - **PR #1765** — *`BinaryOperation.hashCode` symmetry fix.* For commutative operations, `equals()` ignores operand order; the hash code must match. Replaced the order-dependent `Objects.hash(kind, left, right)` with @@ -736,6 +760,33 @@ mix of perf, clarification, and small correctness fixes: `ArrayList` per cached element. `QualifierDefaults.shouldBeAnnotated`: hoisted repeated `getKind()` calls into a local. +- **PR #1797 — `FoundRequired` lazy type formatting (June 2026).** `BaseTypeVisitor.FoundRequired` + previously computed `ATM.toString()`/`toString(true)` eagerly in the constructor, paying full + ATM-formatting cost even when the reported error would be suppressed by `@SuppressWarnings` or + `-AsuppressWarnings`. Changed `found`/`required` fields from `String` to `Object` with anonymous + inner classes whose `toString()` evaluates the ATM format lazily; `shouldPrintVerbose` result is + memoized in a `verboseComputed`/`verbose` pair shared across both objects, so it is called at most + once regardless of which field is stringified first. Also lazified the concatenated + parameter-name prefix string in `checkMethodInvocabilityError`. CF's own sources suppress + thousands of warnings, so the deferred cost is often zero. Wall-clock impact is proportional to + suppression rate on the profiled workload; on the `checkNullness` build of CF itself the delta + is within the noise floor (see A/B note in the name-decoding narrative below). +- **PR #1797 — `SourceChecker.shouldSkipUses` cache (June 2026).** Previously called + `typeElement.toString()` (a `Symbol.toString()` → `Name.toString()` UTF-8 decode) and matched + against a compiled regex on every invocation. Added an `IdentityHashMap` cache + keyed on `typeElement.getQualifiedName()` (identity-stable within one compilation's name table): + first-visit still decodes and matches, but repeat visits for the same enclosing class are O(1). + Reduces the `utf2string` attribution to cache-miss-only (4 samples in the post-fix profile). +- **PR #1797 — `Variable.computeHashCode` and `ProperType.computeHashCode` avoid `toString()` (June 2026).** + `Variable.computeHashCode` hashed `elt.getSimpleName().toString()`, decoding the byte-backed `Name` + on every hash computation. Changed to `elt.getSimpleName().hashCode()`, which returns the + byte-table `index` (content-stable via interning, no decode). `ProperType.computeHashCode` hashed + `properType.toString()` — an `ATM.toString()` call on every type-inference cache lookup. Replaced + with `TypeKind.hashCode() + 31 * elt.getSimpleName().hashCode()` (element extracted for + `DeclaredType` and `TypeVariable`; other kinds hash by kind alone). The new `ProperType` hash is + weaker (no package component), but hash collisions only affect map distribution, not correctness + (`equals` is unchanged). + ### Correctness fixes adjacent to the perf work These belong with the campaign because they were uncovered while @@ -1431,6 +1482,14 @@ not single-leaf. Re-prioritized venues: unguarded `toString` was found (see the eager-error-formatting bullet below), plus `ProperType.computeHashCode` (hashes `toString()`) and `SourceChecker.shouldSkipUses` (`Symbol.toString()` per call for a regex match). + *Update (PR #1797, June 2026):* the stringification share is now addressed — `FoundRequired` + formatting is lazy, `shouldSkipUses` is cached, `ProperType`/`Variable` hash without + `toString()`, `LocalVariableNode.hashCode`/`equals` read `Name` directly, and the annotation + name maps use `IdentityHashMap`. Measured full-build (`./gradlew checknullness`) + warm-daemon A/B: branch ~2m10s, master ~2m16–18s (~7s, consistently one-sided but near the + 5–10s noise floor — as expected for a ~0.9% utf2* share). JFR: `utf2chars` 0.57% + + `utf2string` 0.32% = **0.89% combined** (down from the ~2.3% pre-#1796 level); remaining + callers are cold stub-parsing paths, diagnostic-only formatting, and first-visit cache misses. Stale pre-cache attribution kept below for history: @@ -1580,19 +1639,17 @@ not single-leaf. Re-prioritized venues: `getDeclAnnotations`/`isSupportedQualifier` calls per node), not the per-lookup cost. Low value as a direct target; better addressed indirectly if the defaulting/CF-into-javac venues reduce node visits. -- **Annotation *formatting* in the hot path — CONFIRMED real (June 2026), mechanism found, fix - open: eager error formatting.** Stack samples on the full `checknullness` build settled the - "confirm before chasing" question: 148 samples (~1.1%) contain `AnnotatedTypeMirror.toString`, - and the callers are **not** diagnostics-only. The two paths: (1) - `BaseTypeVisitor.checkContainsSameToString` — a static `SimpleAnnotatedTypeScanner` whose lambda - calls `type.toString()` *and* `type.toString(true)` on **every component of every type** — invoked - via `containsSameToString` from `FoundRequired.of` and `shouldPrintVerbose`; (2) - `reportCommonAssignmentError`/`reportMethodInvocabilityError`, which build `FoundRequired` (i.e. - format both full types) **before** `checker.reportError`, so the formatting cost is paid even when - the warning is subsequently suppressed (`@SuppressWarnings`, `-AsuppressWarnings`) — and CF's own - sources suppress thousands. The fix shape is lazy formatting: defer the `FoundRequired` string - computation until the report is known to be emitted. This is the largest remaining CF-controlled - `utf2chars` consumer after PR #1796 (which removed the name-*comparison* share). +- **Annotation *formatting* in the hot path — APPLIED in PR #1797 (June 2026).** Stack samples on + the full `checknullness` build settled the "confirm before chasing" question: 148 samples (~1.1%) + contained `AnnotatedTypeMirror.toString`, and the callers were **not** diagnostics-only. The two + paths: (1) `BaseTypeVisitor.checkContainsSameToString` — a static `SimpleAnnotatedTypeScanner` + whose lambda calls `type.toString()` *and* `type.toString(true)` on **every component of every + type** — invoked via `containsSameToString` from `FoundRequired.of` and `shouldPrintVerbose`; + (2) `reportCommonAssignmentError`/`reportMethodInvocabilityError`, which built `FoundRequired` + (i.e. formatted both full types) **before** `checker.reportError`, so the formatting cost was + paid even when the warning was subsequently suppressed. Fix: `FoundRequired.found`/`required` + changed from `String` to lazy `Object` wrappers; `shouldPrintVerbose` result memoized. See the + Applied optimizations entry above for the measured A/B. - **CF driving javac internals — the biggest realistic CPU lever (~25% of total).** The wall-clock breakdown above attributes ~25% of all time to CF reaching into javac: @@ -1601,11 +1658,11 @@ not single-leaf. Re-prioritized venues: `Name`/UTF-8 decoding (`Convert.utf2chars`/`utf2string`, `Utf8NameTable.equals` — every time CF compares or stringifies a `Name` that isn't yet decoded/interned), and repeated `TreePath` construction/tree walks. PR #1763 (`getKind()` overrides), PR #1673 - (interned-name caching), and PR #1796 (interned-`Name` identity comparison — removed the - name-*comparison* share of the decode facet; what is left of it is stringification: - the eager-error-formatting bullet above, `ProperType.computeHashCode`, - `SourceChecker.shouldSkipUses`, `LocalVariableNode.getName`, stub-parser - `annosInPackage`) each chipped at one facet. This is bigger than dataflow + stubs + (interned-name caching), PR #1796 (interned-`Name` identity comparison — removed the + name-*comparison* share), and PR #1797 (lazy `FoundRequired` formatting, `shouldSkipUses` + cache, `ProperType`/`Variable`/`LocalVariableNode` hash fixes, `IdentityHashMap` + annotation maps — removed the stringification share; combined utf2* now **0.89%** on the full + `checknullness` build) each chipped at one facet. This is bigger than dataflow + stubs + visitor combined and is the highest-leverage remaining CPU target for realistic compiles; it is incremental, not architectural — audit the remaining forcers/decoders that already have (or could cache) the needed info. Confirmed real, not the @@ -1659,8 +1716,9 @@ CF-controllable clusters and their state, highest-leverage first: the smaller-scope scan; residual is method-subtree scanning. The cheap levers are exhausted (scoping tighter than a method has no element; `trees.getTree` and the single-pass map were rejected). 5. **Small / blocked:** `ElementUtils.qualifiedNameCache` (`synchronizedMap`+`WeakHashMap` lock/expunge, - ~0.58%, blocked on a thread-reachability + daemon-memory audit — see Short list above); annotation - formatting in the hot path (~0.5%, confirm it is type-checking not the stub parser before chasing). + ~0.58%, blocked on a thread-reachability + daemon-memory audit — see Short list above). Annotation + formatting in the hot path is now **resolved** (PR #1797 lazy `FoundRequired`); remaining utf2* + at 0.89% is cold-path / first-visit-miss only. --- diff --git a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java index 61d666ce1a26..b147a73cbb6d 100644 --- a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java +++ b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java @@ -3638,8 +3638,8 @@ protected void reportCommonAssignmentError( @CompilerMessageKey String errorKey, Object... extraArgs) { FoundRequired pair = FoundRequired.of(valueType, varType); - String valueTypeString = pair.found; - String varTypeString = pair.required; + Object valueTypeString = pair.found; + Object varTypeString = pair.required; checker.reportError( valueTree, errorKey, @@ -3755,36 +3755,116 @@ private String fileAndLineNumber(Tree tree) { } /** - * Class that creates string representations of {@link AnnotatedTypeMirror}s which are only - * verbose if required to differentiate the two types. + * Class that lazily creates string representations of {@link AnnotatedTypeMirror}s which are + * only verbose if required to differentiate the two types. + * + *

The strings are evaluated lazily when {@code toString()} is called on the {@link #found} + * or {@link #required} fields. This prevents expensive formatting operations when an error is + * suppressed and never emitted. */ protected static class FoundRequired { - /** The found type's string representation. */ - public final String found; + /** Whether the {@link #verbose} flag has been computed. */ + private boolean verboseComputed = false; - /** The required type's string representation. */ - public final String required; + /** Whether the string representation should be verbose. */ + private boolean verbose = false; - private FoundRequired(AnnotatedTypeMirror found, AnnotatedTypeMirror required) { - if (shouldPrintVerbose(found, required)) { - this.found = found.toString(true); - this.required = required.toString(true); - } else { - this.found = found.toString(); - this.required = required.toString(); + /** + * Lazily computes and memoizes whether the string representation should be verbose. + * + * @param foundType the found type + * @param requiredType the required type + * @return true if verbose toString should be used + */ + private boolean isVerbose(AnnotatedTypeMirror foundType, AnnotatedTypeMirror requiredType) { + if (!verboseComputed) { + verbose = shouldPrintVerbose(foundType, requiredType); + verboseComputed = true; } + return verbose; } - /** Create a FoundRequired for a type and bounds. */ - private FoundRequired(AnnotatedTypeMirror found, AnnotatedTypeParameterBounds required) { - if (shouldPrintVerbose(found, required)) { - this.found = found.toString(true); - this.required = required.toString(true); - } else { - this.found = found.toString(); - this.required = required.toString(); + /** + * Lazily computes and memoizes whether the string representation should be verbose. + * + * @param foundType the found type + * @param requiredBounds the required bounds + * @return true if verbose toString should be used + */ + private boolean isVerbose( + AnnotatedTypeMirror foundType, AnnotatedTypeParameterBounds requiredBounds) { + if (!verboseComputed) { + verbose = shouldPrintVerbose(foundType, requiredBounds); + verboseComputed = true; } + return verbose; + } + + /** + * An object whose {@code toString()} method returns the found type's string representation. + * Evaluated lazily to improve performance. + */ + public final Object found; + + /** + * An object whose {@code toString()} method returns the required type's string + * representation. Evaluated lazily to improve performance. + */ + public final Object required; + + /** + * Create a FoundRequired for two types. + * + * @param found the found type + * @param required the required type + */ + private FoundRequired(AnnotatedTypeMirror found, AnnotatedTypeMirror required) { + this.found = + new Object() { + @Override + public String toString() { + return isVerbose(found, required) + ? found.toString(true) + : found.toString(); + } + }; + this.required = + new Object() { + @Override + public String toString() { + return isVerbose(found, required) + ? required.toString(true) + : required.toString(); + } + }; + } + + /** + * Create a FoundRequired for a type and bounds. + * + * @param found the found type + * @param required the required bounds + */ + private FoundRequired(AnnotatedTypeMirror found, AnnotatedTypeParameterBounds required) { + this.found = + new Object() { + @Override + public String toString() { + return isVerbose(found, required) + ? found.toString(true) + : found.toString(); + } + }; + this.required = + new Object() { + @Override + public String toString() { + return isVerbose(found, required) + ? required.toString(true) + : required.toString(); + } + }; } /** @@ -3980,7 +4060,12 @@ protected void checkTypeArguments( paramName, typeOrMethodName, fr.found, - paramName + " " + fr.required); + new Object() { + @Override + public String toString() { + return paramName + " " + fr.required; + } + }); } } } diff --git a/framework/src/main/java/org/checkerframework/framework/ajava/InsertAjavaAnnotations.java b/framework/src/main/java/org/checkerframework/framework/ajava/InsertAjavaAnnotations.java index fe6e35e0c9ab..80dea5b1f8b4 100644 --- a/framework/src/main/java/org/checkerframework/framework/ajava/InsertAjavaAnnotations.java +++ b/framework/src/main/java/org/checkerframework/framework/ajava/InsertAjavaAnnotations.java @@ -43,15 +43,15 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; -import java.util.HashMap; import java.util.HashSet; +import java.util.IdentityHashMap; import java.util.LinkedHashSet; import java.util.List; -import java.util.Map; import java.util.Set; import java.util.StringJoiner; import javax.lang.model.element.ElementKind; +import javax.lang.model.element.Name; import javax.lang.model.element.PackageElement; import javax.lang.model.element.TypeElement; import javax.lang.model.util.Elements; @@ -173,7 +173,7 @@ private class BuildInsertionsVisitor extends DoubleJavaParserVisitor { *

The map is populated from import statements and also when parsing a file that uses the * fully qualified name of an annotation it doesn't import. */ - private @MonotonicNonNull Map allAnnotations = null; + private @MonotonicNonNull IdentityHashMap allAnnotations = null; /** The annotation insertions seen so far. */ final List insertions = new ArrayList<>(); @@ -437,12 +437,12 @@ private int getFilePosition(Position position) { * Two entries for each annotation: one for the simple name and another for the * fully-qualified name, with the same value. */ - private Map getImportedAnnotations(CompilationUnit cu) { + private IdentityHashMap getImportedAnnotations(CompilationUnit cu) { if (cu.getImports() == null) { - return Collections.emptyMap(); + return new IdentityHashMap<>(); } - Map result = new HashMap<>(); + IdentityHashMap result = new IdentityHashMap<>(); for (ImportDeclaration importDecl : cu.getImports()) { if (importDecl.isAsterisk()) { @SuppressWarnings("signature" // https://tinyurl.com/cfissue/3094: @@ -474,7 +474,7 @@ private Map getImportedAnnotations(CompilationUnit cu) { if (importType != null && importType.getKind() == ElementKind.ANNOTATION_TYPE) { TypeElement annoElt = elements.getTypeElement(imported); if (annoElt != null) { - result.put(annoElt.getSimpleName().toString(), annoElt); + result.put(annoElt.getSimpleName(), annoElt); } } } diff --git a/framework/src/main/java/org/checkerframework/framework/ajava/TypeAnnotationMover.java b/framework/src/main/java/org/checkerframework/framework/ajava/TypeAnnotationMover.java index a3a9c82fdb97..407138a18a9e 100644 --- a/framework/src/main/java/org/checkerframework/framework/ajava/TypeAnnotationMover.java +++ b/framework/src/main/java/org/checkerframework/framework/ajava/TypeAnnotationMover.java @@ -15,10 +15,10 @@ import java.lang.annotation.Target; import java.util.ArrayList; import java.util.Collections; -import java.util.HashMap; +import java.util.IdentityHashMap; import java.util.List; -import java.util.Map; +import javax.lang.model.element.Name; import javax.lang.model.element.TypeElement; import javax.lang.model.util.Elements; @@ -38,7 +38,7 @@ public class TypeAnnotationMover extends VoidVisitorAdapter { * Annotations imported by the file, stored as a mapping from names to the TypeElements for the * annotations. Contains entries for the simple and fully qualified names of each annotation. */ - private final Map allAnnotations; + private final IdentityHashMap allAnnotations; /** Element utilities. */ private final Elements elements; @@ -51,8 +51,9 @@ public class TypeAnnotationMover extends VoidVisitorAdapter { * name and its fully-qualified name both mapped to its TypeElement. * @param elements the Element utilities */ - public TypeAnnotationMover(Map allAnnotations, Elements elements) { - this.allAnnotations = new HashMap<>(allAnnotations); + public TypeAnnotationMover( + IdentityHashMap allAnnotations, Elements elements) { + this.allAnnotations = new IdentityHashMap<>(allAnnotations); this.elements = elements; } @@ -129,7 +130,8 @@ private List getAnnotationsToMove( private @Nullable TypeElement getAnnotationDeclaration(AnnotationExpr annotation) { @SuppressWarnings("signature") // https://tinyurl.com/cfissue/3094 @FullyQualifiedName String annoNameFq = annotation.getNameAsString(); - TypeElement annoTypeElt = allAnnotations.get(annoNameFq); + Name annoNameObj = elements.getName(annoNameFq); + TypeElement annoTypeElt = allAnnotations.get(annoNameObj); if (annoTypeElt == null) { annoTypeElt = elements.getTypeElement(annoNameFq); if (annoTypeElt == null) { diff --git a/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java b/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java index 076ccaaed36b..a5bc8e088660 100644 --- a/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java +++ b/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java @@ -72,6 +72,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.HashSet; +import java.util.IdentityHashMap; import java.util.LinkedHashSet; import java.util.List; import java.util.Locale; @@ -3143,10 +3144,17 @@ protected String getWarningMessagePrefix() { // /** - * Tests whether the class owner of the passed element is an unannotated class and matches the - * pattern specified in the {@code checker.skipUses} property. + * Cache for {@link #shouldSkipUses(Element)}. Maps the qualified name of an enclosing class to + * whether its uses should be skipped. + */ + private final IdentityHashMap shouldSkipUsesCache = + new IdentityHashMap<>(); + + /** + * Tests whether the class owner of the passed element matches the pattern specified in the + * {@code checker.skipUses} property. * - * @param element an element + * @param element the element * @return true iff the enclosing class of element should be skipped */ public final boolean shouldSkipUses(@Nullable Element element) { @@ -3158,11 +3166,19 @@ public final boolean shouldSkipUses(@Nullable Element element) { throw new BugInCF( "enclosingTypeElement(%s [%s]) => null%n", element, element.getClass()); } + javax.lang.model.element.Name qualifiedName = typeElement.getQualifiedName(); + Boolean cached = shouldSkipUsesCache.get(qualifiedName); + if (cached != null) { + return cached; + } + @SuppressWarnings("signature:assignment.type.incompatible" // TypeElement.toString(): // @FullyQualifiedName ) @FullyQualifiedName String name = typeElement.toString(); - return shouldSkipUses(name); + boolean result = shouldSkipUses(name); + shouldSkipUsesCache.put(qualifiedName, result); + return result; } /** diff --git a/framework/src/main/java/org/checkerframework/framework/stub/AnnotationFileParser.java b/framework/src/main/java/org/checkerframework/framework/stub/AnnotationFileParser.java index 0b4e19e85577..df52782f5b0b 100644 --- a/framework/src/main/java/org/checkerframework/framework/stub/AnnotationFileParser.java +++ b/framework/src/main/java/org/checkerframework/framework/stub/AnnotationFileParser.java @@ -112,6 +112,7 @@ import javax.lang.model.element.Element; import javax.lang.model.element.ElementKind; import javax.lang.model.element.ExecutableElement; +import javax.lang.model.element.Name; import javax.lang.model.element.PackageElement; import javax.lang.model.element.TypeElement; import javax.lang.model.element.VariableElement; @@ -248,7 +249,7 @@ public class AnnotationFileParser { * * @see #getImportedAnnotations */ - private Map allAnnotations; + private IdentityHashMap allAnnotations; /** * A list of the fully-qualified names of enum constants and static fields with constant values @@ -503,7 +504,7 @@ private void setRoot(CompilationUnitTree root) { * @param packageElement a package * @return a map from annotation name to TypeElement */ - public static Map annosInPackage(PackageElement packageElement) { + public static IdentityHashMap annosInPackage(PackageElement packageElement) { return createNameToAnnotationMap( ElementFilter.typesIn(packageElement.getEnclosedElements())); } @@ -515,7 +516,7 @@ public static Map annosInPackage(PackageElement packageElem * @param typeElement a type * @return a map from annotation name to TypeElement */ - public static Map annosInType(TypeElement typeElement) { + public static IdentityHashMap annosInType(TypeElement typeElement) { return createNameToAnnotationMap(ElementFilter.typesIn(typeElement.getEnclosedElements())); } @@ -525,13 +526,13 @@ public static Map annosInType(TypeElement typeElement) { * @param typeElements the elements whose annotations to retrieve * @return a map from annotation names (both fully-qualified and simple names) to TypeElement */ - public static Map createNameToAnnotationMap( + public static IdentityHashMap createNameToAnnotationMap( List typeElements) { - Map result = new HashMap<>(); + IdentityHashMap result = new IdentityHashMap<>(); for (TypeElement typeElm : typeElements) { if (typeElm.getKind() == ElementKind.ANNOTATION_TYPE) { - putIfAbsent(result, typeElm.getSimpleName().toString(), typeElm); - putIfAbsent(result, ElementUtils.getQualifiedName(typeElm), typeElm); + putIfAbsent(result, typeElm.getSimpleName(), typeElm); + putIfAbsent(result, typeElm.getQualifiedName(), typeElm); } } return result; @@ -575,8 +576,8 @@ public static Map createNameToAnnotationMap( * fully-qualified name, with the same value. * @see #allAnnotations */ - private Map getImportedAnnotations() { - Map result = new HashMap<>(); + private IdentityHashMap getImportedAnnotations() { + IdentityHashMap result = new IdentityHashMap<>(); // TODO: The size can be greater than 1, but this ignores all but the first element. assert !stubUnit.getCompilationUnits().isEmpty(); @@ -653,7 +654,7 @@ private Map getImportedAnnotations() { // Single annotation or nested annotation. TypeElement annoElt = elements.getTypeElement(imported); if (annoElt != null) { - putIfAbsent(result, annoElt.getSimpleName().toString(), annoElt); + putIfAbsent(result, annoElt.getSimpleName(), annoElt); importedTypes.put(annoElt.getSimpleName().toString(), annoElt); } else { stubWarnNotFound(importDecl, "could not load import: " + imported); @@ -2686,10 +2687,11 @@ private boolean isAnnotatedForThisChecker(List annotations) { * @return the AnnotationMirror for the annotation, or null if it cannot be built */ private @Nullable AnnotationMirror getAnnotation( - AnnotationExpr annotation, Map allAnnotations) { + AnnotationExpr annotation, Map allAnnotations) { @SuppressWarnings("signature") // https://tinyurl.com/cfissue/3094 @FullyQualifiedName String annoNameFq = annotation.getNameAsString(); - TypeElement annoTypeElt = allAnnotations.get(annoNameFq); + Name annoNameObj = elements.getName(annoNameFq); + TypeElement annoTypeElt = allAnnotations.get(annoNameObj); if (annoTypeElt == null) { // If the annotation was not imported, then #getImportedAnnotations did not add it to // the allAnnotations field. This code adds the annotation when it is encountered (i.e. diff --git a/framework/src/main/java/org/checkerframework/framework/util/typeinference8/types/ProperType.java b/framework/src/main/java/org/checkerframework/framework/util/typeinference8/types/ProperType.java index d04f500fe217..08dbd78378ca 100644 --- a/framework/src/main/java/org/checkerframework/framework/util/typeinference8/types/ProperType.java +++ b/framework/src/main/java/org/checkerframework/framework/util/typeinference8/types/ProperType.java @@ -282,7 +282,16 @@ public boolean equals(Object o) { * @return the hash code */ private int computeHashCode() { - int hc = properType.toString().hashCode(); + int hc = properType.getKind().hashCode(); + javax.lang.model.element.Element elt = null; + if (properType instanceof javax.lang.model.type.DeclaredType) { + elt = ((javax.lang.model.type.DeclaredType) properType).asElement(); + } else if (properType instanceof javax.lang.model.type.TypeVariable) { + elt = ((javax.lang.model.type.TypeVariable) properType).asElement(); + } + if (elt != null) { + hc = 31 * hc + elt.getSimpleName().hashCode(); + } hc = 31 * hc + Kind.PROPER.hashCode(); return hc; } diff --git a/framework/src/main/java/org/checkerframework/framework/util/typeinference8/types/Variable.java b/framework/src/main/java/org/checkerframework/framework/util/typeinference8/types/Variable.java index 98202614439d..21506c695962 100644 --- a/framework/src/main/java/org/checkerframework/framework/util/typeinference8/types/Variable.java +++ b/framework/src/main/java/org/checkerframework/framework/util/typeinference8/types/Variable.java @@ -191,7 +191,7 @@ public boolean equals(Object o) { */ private int computeHashCode() { Element elt = typeVariableJava.asElement(); - int hc = elt.getSimpleName().toString().hashCode(); + int hc = elt.getSimpleName().hashCode(); hc = 31 * hc + elt.getEnclosingElement().hashCode(); hc = 31 * hc + invocation.hashCode(); return hc;