Skip to content

#4034 Cache Type instances in TypeFactory.getType(...) to avoid redundant work - #4129

Open
AnkitaAdvitot wants to merge 2 commits into
mapstruct:mainfrom
AnkitaAdvitot:cache-type-instances-typefactory
Open

AnkitaAdvitot wants to merge 2 commits into
mapstruct:mainfrom
AnkitaAdvitot:cache-type-instances-typefactory

Conversation

@AnkitaAdvitot

Copy link
Copy Markdown

Fixes #4034.

Background & Motivation

In MapStruct's annotation processor, TypeFactory.getType(...) is invoked frequently across rounds and model element processors (mapping methods, source/target property resolution, accessor discovery, enclosing element inspection). Previously, TypeFactory instantiated a fresh Type object on nearly every invocation:

  • This caused repetitive enclosing-element traversals, member lookups, and typeUtils.isSubtypeErased(...) checks.
  • Per-Type memoized caches (such as isNullMarked(), accessors, and method lists) were repeatedly re-allocated or re-queried instead of shared.

Solution

  1. Round-Scoped Type Element Cache:
    • Added typeElementCache (IdentityHashMap<TypeElement, Type>) in TypeFactory to intern prototypical types derived directly from non-generic TypeElement instances.
  2. Structural Type Mirror Cache:
    • Added typeCache (HashMap<TypeCacheKey, Type>) using an immutable structural fingerprint (TypeFingerprint) that safely distinguishes:
      • Declared types (interned by qualified name / canonical TypeElement, type argument fingerprints, and enclosing type fingerprint).
      • Array types (by component type fingerprint).
      • Wildcard types (by extends / super bound fingerprints).
      • Type variables (by name and upper bound fingerprints).
      • Primitive and void types (by TypeKind).
    • The cache key strictly distinguishes contextual configuration flags: isLiteral and alwaysImport.
  3. Safety & Erroneous Types:
    • Pre-validation check canBeProcessed(mirror) ensures erroneous/unresolvable types are never cached or suppressed, preserving TypeHierarchyErroneousException semantics.
  4. Testing:
    • Added Issue4034Test and Issue4034Mapper verifying that identical Type instances are returned for repeated lookups (getType(TypeElement), getType(TypeMirror), generic List<String>), differing type arguments (List<String> vs List<Integer>) produce distinct instances, and flag variants (isLiteral, alwaysImport) remain distinct.
    • All tests pass and checkstyle audit passes with 0 violations.

@hduelme

hduelme commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

@AnkitaAdvitot thanks for your contribution.
Can you explain your test a bit more. Currently I don't understand why you combined @ProcessorTest and @Test in one test class.

@AnkitaAdvitot

Copy link
Copy Markdown
Author

Hi @hduelme,

Thanks for pointing that out!

Originally, I combined them in Issue4034Test to cover verification for #4034 from two angles:

  1. @ProcessorTest (shouldCompileMapperWithCachedTypes) verified that end-to-end mapper compilation and runtime execution worked without regressions across both compilers (JDK & Eclipse).
  2. @Test (shouldInternTypeInstancesAcrossLookups) directly asserted TypeFactory's caching/interning semantics (isSameAs, parameterized types, literals, etc.), which couldn't be asserted inside @ProcessorTest since it runs post-compilation without access to internal compiler components like TypeFactory.

I agree that combining them in one test class with @WithClasses is confusing and doesn't follow MapStruct conventions.

I have separated them:

  • Issue4034Test is now purely a @ProcessorTest bug test for Issue4034Mapper.
  • The TypeFactory caching unit test has been moved into its own dedicated unit test class: org.mapstruct.ap.internal.model.common.TypeFactoryTest.

I've pushed the updated commit to this branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cache Type instances in TypeFactory.getType(...) to avoid redundant work

3 participants