Skip to content
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

Reduce delegate command calls when classpath changes #3439

Merged
merged 1 commit into from
Dec 22, 2023

Conversation

jdneo
Copy link
Collaborator

@jdneo jdneo commented Dec 21, 2023

This PR use the project uri from the classpath update event to reduce the delegate command invocation when checking the lombok dependency.

Previously, when a classpath update event comes, the extension will check the lombok dependency for all java projects. It was not using the project uri of the classpath update event. Assume a workspace contains 10 projects, it will generate 10*10=100 getClasspath command invocation.

I did a benchmark roughly for the project: https://github.com/Azure/azure-sdk-for-java/tree/main/sdk/spring with following steps:

  1. Disable auto build (too many modules causes too much time to build the workspace)
  2. Set java.trace.server to message
  3. Open the folder.
  4. Wait until no more traces appear in the output channel.

Without the change, it takes 7 mins and 30 secs, with 3295 requests sent to server.
With this change, it takes 5 mins and 40 secs, with 347 requests sent to server.

Signed-off-by: Sheng Chen <sheche@microsoft.com>
@rgrunber rgrunber self-requested a review December 21, 2023 21:16
@rgrunber
Copy link
Member

I agree with the change itself, but I don't see how this would result in such a large improvement. When I modify a pom file of some multi-maven module project, I only see checkLombokDependency called once as part of the classpath update, which on subsequent calls will just save re-evaluating java.project.getAll, but that's just a single call.

@jdneo
Copy link
Collaborator Author

jdneo commented Dec 22, 2023

After java.project.getAll, client will call await apiManager.getApiInstance().getClasspaths(projectUri, {scope: 'test'}); for each project. But actually, there is only one project's classpath has changed. This is a waste. More project it has in workspace, more unnecessary getClasspath() are called for those unchanged projects.

@rgrunber rgrunber added this to the End December 2023 milestone Dec 22, 2023
@rgrunber rgrunber merged commit b8fd158 into redhat-developer:master Dec 22, 2023
2 checks passed
@rgrunber
Copy link
Member

You're right. For some reason I thought you were only re-using a cached projectUris instead of re-calculating, but missed that you were actually passing in just a single element. so getClassPaths becomes a single call and getAllJavaProjects is avoided.

@jdneo jdneo deleted the cs/lombok-perf branch December 22, 2023 05:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants