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.

Sign up for GitHub

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

Jump to bottom

Store the uri of document into CompletionResponse #2614

Merged
merged 1 commit into from
Apr 24, 2023
Merged

Store the uri of document into CompletionResponse #2614

merged 1 commit into from
Apr 24, 2023

Conversation

jdneo
Copy link
Contributor

@jdneo jdneo commented Apr 23, 2023

  • instead of storing the uri into the data field of each completion item, now we store the uri into the CompletionResponse, and get it during resolving.
  • Triggering completion via 'S' in Spring Petclinic project, the response (textDocument/completion) data size can be reduced from 3.05MB to 2.63MB. (Directly copy the trace to a text file)

@jdneo
Store the uri of document into CompletionResponse
6d46da6 
- instead of storing the uri into the data field of each completion item,
  now we store the uri into the CompletionResponse, and get it during
  resolving.
- Triggering completion via 'S' in Spring Petclinic project, the data
  size can be reduced from 3.05MB to 2.63MB. (Directly copy the trace
  to a text file)

Signed-off-by: Sheng Chen <sheche@microsoft.com>
testforstephen
testforstephen approved these changes Apr 23, 2023
View reviewed changes
Copy link
Contributor

@testforstephen testforstephen left a comment

Choose a reason for hiding this comment

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

LGTM. Duplicating uri in each completion item is redundant, it makes sense not to return it to the client completion items.

@testforstephen testforstephen added this to the End April 2023 milestone Apr 23, 2023
@testforstephen testforstephen merged commit 1259f6c into eclipse-jdtls:master Apr 24, 2023
@jdneo jdneo deleted the cs/completionUri branch April 24, 2023 01:56
@jdneo
Copy link
Contributor Author

jdneo commented Apr 24, 2023 •
edited
Loading

Append some data with and without this PR - Triggering completion via 'S' in Spring Petclinic project

Without this PR With this PR
323 300
132 121
115 106
105 102
94 91
89 84
82 84
111 78
87 75
82 74

unit: ms

@jdneo jdneo mentioned this pull request Nov 29, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@snjeza snjeza snjeza approved these changes

@testforstephen testforstephen testforstephen approved these changes

@fbricon fbricon Awaiting requested review from fbricon

@rgrunber rgrunber Awaiting requested review from rgrunber

Assignees
No one assigned
Labels
content-assist performance
Projects
None yet
Milestone
End April 2023
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@jdneo @snjeza @testforstephen