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

Improve performance of completions #1750

Closed
DanTup opened this issue May 27, 2019 · 4 comments
Closed

Improve performance of completions #1750

DanTup opened this issue May 27, 2019 · 4 comments
Labels
important Something important! is performance
Milestone

Comments

@DanTup
Copy link
Member

DanTup commented May 27, 2019

As well as probably being the cause of #1721, the performance of completions with unimported symbols is generally poor.

Testing on Windows PC with the following test (inside test\flutter_only\providers\completion_item_provider.test.ts):

	describe("with SuggestionSet support", () => {
		beforeEach("ensure SuggestionSets are supported", function () {
			if (!extApi.analyzerCapabilities.supportsAvailableSuggestions)
				this.skip();
		});

		it.only("includes unimported symbols", async () => {
			await setTestContent(`
main() {
  ProcessInf
}
		`);
			const count = 50;
			const start = Date.now();
			for (let i = 0; i < 50; i++) {
				const completions = await getCompletionsAt("ProcessInf^");
				ensureCompletion(completions, vs.CompletionItemKind.Class, "ProcessInfo", "ProcessInfo");
			}
			const end = Date.now();
			console.log(`Took ${end - start}ms to do ${count} completion requests`);
		});
	});

Output from a few runs:

Took 56426ms to do 50 completion requests
Took 57034ms to do 50 completion requests
Took 57607ms to do 50 completion requests
@DanTup DanTup added is performance important Something important! labels May 27, 2019
@DanTup DanTup modified the milestones: v3.1.0, v3.2.0 May 27, 2019
@DanTup
Copy link
Member Author

DanTup commented May 27, 2019

Some (terrible) figures from the committed test on initial code:

Iteration  #0 took 1387 ms to return 8809 results, heap change was   93 MB
Iteration  #1 took 1218 ms to return 8809 results, heap change was   35 MB
Iteration  #2 took 1023 ms to return 8809 results, heap change was  103 MB
Iteration  #3 took 1035 ms to return 8809 results, heap change was   97 MB
Iteration  #4 took 1017 ms to return 8809 results, heap change was  102 MB
Iteration  #5 took 1319 ms to return 8809 results, heap change was -211 MB
Iteration  #6 took 1049 ms to return 8809 results, heap change was  103 MB
Iteration  #7 took 1039 ms to return 8809 results, heap change was  102 MB
Iteration  #8 took 1052 ms to return 8809 results, heap change was   96 MB
Iteration  #9 took 1025 ms to return 8809 results, heap change was  102 MB
Iteration #10 took 1038 ms to return 8809 results, heap change was  102 MB
Iteration #11 took 1025 ms to return 8809 results, heap change was  103 MB
Iteration #12 took 1449 ms to return 8809 results, heap change was -415 MB
Iteration #13 took 1090 ms to return 8809 results, heap change was  102 MB
Iteration #14 took 1066 ms to return 8809 results, heap change was   96 MB
Iteration #15 took 1030 ms to return 8809 results, heap change was  102 MB
Iteration #16 took 1028 ms to return 8809 results, heap change was  102 MB
Iteration #17 took 1037 ms to return 8809 results, heap change was  103 MB
Iteration #18 took 1025 ms to return 8809 results, heap change was  102 MB
Iteration #19 took 1035 ms to return 8809 results, heap change was   97 MB
Iteration #20 took 1621 ms to return 8809 results, heap change was -507 MB
Iteration #21 took 1077 ms to return 8809 results, heap change was   97 MB
Iteration #22 took 1047 ms to return 8809 results, heap change was  102 MB
Iteration #23 took 1062 ms to return 8809 results, heap change was  102 MB
Iteration #24 took 1028 ms to return 8809 results, heap change was  103 MB
Iteration #25 took 1031 ms to return 8809 results, heap change was  102 MB
Iteration #26 took 1037 ms to return 8809 results, heap change was   97 MB
Iteration #27 took 1966 ms to return 8809 results, heap change was -439 MB
Iteration #28 took 1076 ms to return 8809 results, heap change was  103 MB
Iteration #29 took 1068 ms to return 8809 results, heap change was   97 MB
Iteration #30 took 1079 ms to return 8809 results, heap change was  101 MB
Iteration #31 took 1044 ms to return 8809 results, heap change was  102 MB
Iteration #32 took 1029 ms to return 8809 results, heap change was  104 MB
Iteration #33 took 1856 ms to return 8809 results, heap change was -373 MB
Iteration #34 took 1134 ms to return 8809 results, heap change was  103 MB
Iteration #35 took 1089 ms to return 8809 results, heap change was   97 MB
Iteration #36 took 1055 ms to return 8809 results, heap change was  101 MB
Iteration #37 took 1040 ms to return 8809 results, heap change was  102 MB
Iteration #38 took 1040 ms to return 8809 results, heap change was  102 MB
Iteration #39 took 2024 ms to return 8809 results, heap change was -326 MB
Iteration #40 took 1129 ms to return 8809 results, heap change was  102 MB
Iteration #41 took 1130 ms to return 8809 results, heap change was  103 MB
Iteration #42 took 1120 ms to return 8809 results, heap change was  101 MB
Iteration #43 took 1077 ms to return 8809 results, heap change was   96 MB
Iteration #44 took 2009 ms to return 8809 results, heap change was -270 MB
Iteration #45 took 1044 ms to return 8809 results, heap change was   97 MB
Iteration #46 took 1003 ms to return 8809 results, heap change was  101 MB
Iteration #47 took  984 ms to return 8809 results, heap change was   102 MB
Iteration #48 took 2588 ms to return 8809 results, heap change was -219 MB
Iteration #49 took 1024 ms to return 8809 results, heap change was  103 MB

Total run took 59530 ms heap change was 1403 MB

Memory does go back down, so we're probably not leaking, just allocating a lot for every big completion request.

memory

@DanTup
Copy link
Member Author

DanTup commented May 27, 2019

Without the new completions, numbers are tiny, confirming it's that work that's causing the issues:

Iteration # 0 took 66 ms to return 32 results, heap change was 1 MB
Iteration # 1 took 11 ms to return 32 results, heap change was 1 MB
Iteration # 2 took 13 ms to return 32 results, heap change was 1 MB
Iteration # 3 took 24 ms to return 32 results, heap change was -13 MB
Iteration # 4 took 15 ms to return 32 results, heap change was 1 MB
Iteration # 5 took 16 ms to return 32 results, heap change was 1 MB

@DanTup
Copy link
Member Author

DanTup commented May 27, 2019

It seems that a lot of this time may be within VS Code for passing the data between the extension host and UI (if we do all the computation but just don't return the results, it goes super fast). I've opened microsoft/vscode#74418 to see if it's possible this could be improved there or not.

@DanTup
Copy link
Member Author

DanTup commented Jun 19, 2019

Going to close this since a lot of work was done in this version. There's still a branch that has some cleanup that might improve things further, but that's for a future milestone.

@DanTup DanTup closed this as completed Jun 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
important Something important! is performance
Projects
None yet
Development

No branches or pull requests

1 participant