-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Parallel Tool Execution #4255
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Parallel Tool Execution #4255
Conversation
Closes spring-projectsgh-4254 Signed-off-by: Rafael Cunha <[email protected]>
Closes spring-projectsgh-4254 Signed-off-by: Rafael Cunha <[email protected]>
private static TaskExecutor buildDefaultTaskExecutor() { | ||
ThreadPoolTaskExecutor taskExecutor = new ThreadPoolTaskExecutor(); | ||
taskExecutor.setThreadNamePrefix("ai-toll-calling-"); | ||
taskExecutor.setCorePoolSize(4); | ||
taskExecutor.setMaxPoolSize(16); | ||
taskExecutor.setTaskDecorator(new ContextPropagatingTaskDecorator()); | ||
taskExecutor.initialize(); | ||
return taskExecutor; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think, no matter what, we shouldn't hardcode thread pool configurations as "magic values" here, even if it's just a default thread pool. Perhaps we should introduce corresponding configuration options and a dedicated configuration class to manage these parameters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By the way, the spelling of the thread name also appears to be incorrect: "toll" should be "tool".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can add it to the ToolCallingProperties class, which will allow us to customize the default values using properties. What do you think?
Exemple
@ConfigurationProperties(ToolCallingProperties.CONFIG_PREFIX)
public class ToolCallingProperties {
public static final String CONFIG_PREFIX = "spring.ai.tools";
private final Observations observations = new Observations();
private final TaskExecutorProperties taskExecutor = new TaskExecutorProperties();
public static class TaskExecutorProperties {
/**
* Whether to enable custom task executor configuration for tool calls.
*/
private boolean enabled = false;
/**
* Core number of threads in the pool.
*/
private int corePoolSize = Runtime.getRuntime().availableProcessors();
/**
* Maximum number of threads in the pool.
*/
private int maxPoolSize = Runtime.getRuntime().availableProcessors() * 2;
/**
* Capacity of the queue for holding tasks before they are executed.
*/
private int queueCapacity = 100;
/**
* Prefix for thread names in the pool.
*/
private String threadNamePrefix = "tool-call-exec-";
}
}
private static final ToolExecutionExceptionProcessor DEFAULT_TOOL_EXECUTION_EXCEPTION_PROCESSOR | ||
= DefaultToolExecutionExceptionProcessor.builder().build(); | ||
|
||
private static final TaskExecutor DEFAULT_TASK_EXECUTOR = buildDefaultTaskExecutor(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initializing the static thread pool by calling buildDefaultTaskExecutor()
here causes the method to be executed regardless of whether the user provides a thread pool in the constructor, which seems inappropriate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just deployed a change tweaking this behavior. The system will now create a default instance only when no custom configuration is supplied.
return new ToolResponseMessage.ToolResponse(toolCall.id(), toolName, | ||
toolCallResult != null ? toolCallResult : ""); | ||
}, this.taskExecutor)) | ||
.map(CompletableFuture::join) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this intended to wait for all tools to finish execution in startup order? I'm not sure if this approach makes sense—would using CompletableFuture.allOf()
be more appropriate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed it to use allOf, but we still need to use join to get the responses. However, it will return immediately.
Closes spring-projectsgh-4254 Signed-off-by: Rafael Cunha <[email protected]>
Description
This MR adds parallel execution capability to DefaultToolCallingManager to process multiple AI tool calls concurrently instead of sequentially, significantly improving performance for applications requiring multiple independent tool interactions.
Resolves #4254