Skip to content

Reduce plugin downloads during prefix resolution via two-pass heuristic#11071

Closed
gnodet wants to merge 1 commit intoapache:masterfrom
gnodet:feature/plugin-prefix-resolver-heuristic
Closed

Reduce plugin downloads during prefix resolution via two-pass heuristic#11071
gnodet wants to merge 1 commit intoapache:masterfrom
gnodet:feature/plugin-prefix-resolver-heuristic

Conversation

@gnodet
Copy link
Copy Markdown
Contributor

@gnodet gnodet commented Aug 28, 2025

This PR introduces a conservative optimization in DefaultPluginPrefixResolver to avoid resolving/downloading all declared plugins when resolving a goal prefix.

Summary:

  • Two-pass project scan: first, resolve only likely candidates based on artifactId-derived prefix (e.g., maven-compiler-plugin → compiler); then fall back to original full scan if needed, preserving compatibility with custom goalPrefix values.
  • Adds unit tests (DefaultPluginPrefixResolverTest) validating that:
    • Only the candidate plugin descriptor is loaded for conventional artifactIds
    • Fallback full scan still resolves custom goalPrefix

Motivation:
As reported in #11067, resolving plugin prefixes could trigger downloads of every declared plugin because the implementation loads each plugin descriptor to check goalPrefix. This change significantly reduces that cost in typical projects without changing resolution precedence.

Notes:

  • No behavior change intended other than reduced unnecessary descriptor loads.
  • Build and formatting: spotless:apply executed locally; targeted unit test executed successfully.

Pull Request opened by Augment Code with guidance from the PR author

…wo-pass heuristic on artifactId; add unit tests

This change avoids loading plugin descriptors for non-candidate plugins by first
filtering declared plugins using a conventional artifactId→prefix mapping
(e.g., maven-compiler-plugin → compiler). If a match is not found, we fall back
to the original full scan to preserve correctness for custom goalPrefix values.

Adds DefaultPluginPrefixResolverTest to verify:
- Only candidate plugin descriptor is loaded based on heuristic
- Fallback full scan resolves custom goalPrefix correctly

Fixes apache#11067
@cstamas
Copy link
Copy Markdown
Member

cstamas commented Aug 28, 2025

Ah, so prefix resolver was going over all defined plugins, getting descriptor just to select the right one?

@cstamas
Copy link
Copy Markdown
Member

cstamas commented Aug 28, 2025

What if we change prefix resolver? Right now, this distinction makes not much sense:

if (result == null) {
result = resolveFromRepository(request);
if (result == null) {
throw new NoPluginFoundForPrefixException(
request.getPrefix(),
request.getPluginGroups(),
request.getRepositorySession().getLocalRepository(),
request.getRepositories());
} else {
logger.debug(
"Resolved plugin prefix {} to {}:{} from repository {}",
request.getPrefix(),
result.getGroupId(),
result.getArtifactId(),
(result.getRepository() != null ? result.getRepository().getId() : "null"));
}
} else {
logger.debug(
"Resolved plugin prefix {} to {}:{} from POM {}",
request.getPrefix(),
result.getGroupId(),
result.getArtifactId(),
request.getPom());
}

(resolve from project, or just resolve == the endgame is equal to this below)

Proposed change:

  • take plugin groupIDs (from settings), make it an ordered set (to retain order from settings)
  • if project present, gather plugin groupIDs from it (appending it, hence ordered set)
  • iterate over groupId looking for prefix

This would be MUCH faster and intuitive, then iterating over all project plugins for nothing... also, there is no need for heuristic at all, as we talk about groupId -> prefix mapping, and not about plugin -> descriptor -> prefix mapping (that has much higher cardinality that groupIds involved).

@gnodet
Copy link
Copy Markdown
Contributor Author

gnodet commented Aug 28, 2025

What if we change prefix resolver? Right now, this distinction makes not much sense:

if (result == null) {
result = resolveFromRepository(request);
if (result == null) {
throw new NoPluginFoundForPrefixException(
request.getPrefix(),
request.getPluginGroups(),
request.getRepositorySession().getLocalRepository(),
request.getRepositories());
} else {
logger.debug(
"Resolved plugin prefix {} to {}:{} from repository {}",
request.getPrefix(),
result.getGroupId(),
result.getArtifactId(),
(result.getRepository() != null ? result.getRepository().getId() : "null"));
}
} else {
logger.debug(
"Resolved plugin prefix {} to {}:{} from POM {}",
request.getPrefix(),
result.getGroupId(),
result.getArtifactId(),
request.getPom());
}

(resolve from project, or just resolve == the endgame is equal to this below)

Proposed change:

  • take plugin groupIDs (from settings), make it an ordered set (to retain order from settings)
  • if project present, gather plugin groupIDs from it (appending it, hence ordered set)
  • iterate over groupId looking for prefix

This would be MUCH faster and intuitive, then iterating over all project plugins for nothing... also, there is no need for heuristic at all, as we talk about groupId -> prefix mapping, and not about plugin -> descriptor -> prefix mapping (that has much higher cardinality that groupIds involved).

#11072 looks indeed much better.

@gnodet gnodet closed this Aug 28, 2025
@gnodet gnodet deleted the feature/plugin-prefix-resolver-heuristic branch August 28, 2025 13:24
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.

2 participants