Skip to content

Resolver Thread management#1541

Merged
cstamas merged 7 commits intoapache:masterfrom
cstamas:threads
Aug 13, 2025
Merged

Resolver Thread management#1541
cstamas merged 7 commits intoapache:masterfrom
cstamas:threads

Conversation

@cstamas
Copy link
Member

@cstamas cstamas commented Aug 7, 2025

Rework thread management.

High level changes:

  • introduce simple SmartExecutor to hide underlying things; offer minimal surface just enough for consumers
  • make 3 consumers use them with try-with-resources consistently (basic connector, metadata resolver, BF collector)
  • on Java 21 use virtual threads, as all 3 consumer executes very IO bound tasks (ideal contenders)

Details:

  • consumer basic connector logic simplified; instead of going directly for "direct" executor, use one single method, and perform direct executions as needed (as it had already all in place already); basic connector retains the ability to "direct execute" tasks when appropriate (as it knows ahead the task count)
  • all 3 consumers use session cached SmartExecutor instances (basic connector exceptions for direct execution explained above)

Consequences: the parameters

  • for basic connector: aether.connector.basic.downstreamThreads, aether.connector.basic.upstreamThreads and aether.connector.basic.threads (they may be suffixed with .repoId)
  • for BF collector aether.dependencyCollector.bf.threads
  • for metadata resolver aether.metadataResolver.threads

are from now on global and reflect the reality for maven session, in a way they are now truly aplied per-Maven session (limits now represent true per-session limits).

Before, this was not the case, and Maven was able to fully "step over" these limits: consider BF collector, it created always "local" thread pool with size 5. But alas, Maven 4 builds project models already in parallel (!), hence, BF collector (to resolve models for model builder) was already called from several threads, and each thread created "local" pool for it's own use.

This may imply that pool sizes may need a revisit, as they may be too conservative now (or they were just not truly applied, as intended).

Fixes #1537

@cstamas cstamas requested a review from gnodet August 7, 2025 17:09
@cstamas cstamas self-assigned this Aug 7, 2025
@cstamas cstamas added the enhancement New feature or request label Aug 7, 2025
@cstamas cstamas marked this pull request as ready for review August 13, 2025 11:30
@cstamas cstamas added this to the 2.0.11 milestone Aug 13, 2025
return DIRECT;
}
}
return new SmartExecutor.Limited(Executors.newThreadPerTaskExecutor(Thread.ofVirtual().name(namePrefix, 1).factory()),maxConcurrentTasks);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@slawekjaranowski seems spotless ignores, or is unaware of this class, as this class clearly have formatting issues and build did not fail.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

default value for java sources is:

<include>src/main/java/**/*.java</include>
<include>src/test/java/**/*.java</include>

https://github.com/diffplug/spotless/tree/main/plugin-maven#java
😄

@cstamas cstamas merged commit 7b4bd23 into apache:master Aug 13, 2025
8 checks passed
@cstamas cstamas deleted the threads branch August 13, 2025 17:19
@XenoAmess
Copy link

XenoAmess commented Aug 14, 2025

This may imply that pool sizes may need a revisit, as they may be too conservative now (or they were just not truly applied, as intended).

@cstamas
Yeah you are right...
In virtual thread senario + normal usecase I even doubt if it be better to have no number limit...

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

would it be considerable to add Multi-Release JAR mechanism? virtual thread seems helpful in this repo.(at least helpful for bf)

3 participants