Language server startup time improvement #1299

MikhailArkhipov · 2018-04-04T20:50:24Z

Fixes #1283

This pull request:

Has a title summarizes what is changing
Rest is N/A, work in progress. Tests see Unit tests for VS engine activator #1159

This reverts commit e240c3f.

This reverts commit af573be.

…nto vsc

MikhailArkhipov · 2018-04-04T20:50:49Z

Will rebase after merge, too much history

codecov · 2018-04-04T21:15:09Z

Codecov Report

Merging #1299 into master will decrease coverage by 0.08%.
The diff coverage is 11.2%.

@@            Coverage Diff            @@
##           master   #1299      +/-   ##
=========================================
- Coverage   71.09%     71%   -0.09%     
=========================================
  Files         267     269       +2     
  Lines       12350   12383      +33     
  Branches     2188    2190       +2     
=========================================
+ Hits         8780    8793      +13     
- Misses       3431    3454      +23     
+ Partials      139     136       -3

Impacted Files	Coverage Δ
src/client/common/types.ts	`100% <ø> (ø)`	⬆️
src/client/common/configuration/service.ts	`94.11% <100%> (+24.88%)`	⬆️
src/client/activation/interpreterDataService.ts	`12.08% <12.08%> (ø)`
src/client/activation/analysis.ts	`16.16% <6.06%> (+4.1%)`	⬆️
src/client/linters/baseLinter.ts	`87.5% <0%> (-1.05%)`	⬇️
...rc/client/debugger/PythonProcessCallbackHandler.ts	`52.96% <0%> (ø)`	⬆️
src/client/debugger/Common/constants.ts	`100% <0%> (ø)`
src/client/debugger/PythonProcess.ts	`48.33% <0%> (+0.41%)`	⬆️
src/client/debugger/mainV2.ts	`76.03% <0%> (+0.82%)`	⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3e40ce2...e1d2b15. Read the comment docs.

DonJayamanne · 2018-04-05T03:23:46Z

src/client/activation/analysis.ts

@@ -96,7 +88,7 @@ export class AnalysisExtensionActivator implements IExtensionActivator {
        return false;
    }

-    private async tryStartLanguageClient(context: ExtensionContext, lc: LanguageClient): Promise<Error> {
+    private async tryStartLanguageClient(context: ExtensionContext, lc: LanguageClient): Promise<Error | undefined> {


oops, not sure how I missed this. Why are we returning a promise?
Shouldn't we return an error as part of the rejection. I.e. change this to Promise<void> and errors will be captured in catch?

Yeah, you are right. Returning exception is questionable pattern.

DonJayamanne · 2018-04-05T03:30:06Z

src/client/activation/interpreterDataService.ts

+    return s.length > 0 && s[0] !== '[';
+  }
+
+  private verifySearchPathsAsync(currentPaths: string, interpreterPath: string, execService: IPythonExecutionService): void {


Please remove the Async suffix.

OK. This was mostly for the calling code to show that this is actually async and is not awaited for.

DonJayamanne · 2018-04-05T03:37:44Z

src/client/activation/interpreterDataService.ts

+      return;
+    }
+
+    let interpreterData = this.context.globalState.get(interpreterPath) as InterpreterData;


Please use another key for the cache storage. the python path would be too generic, easy to use this else where for other information related to the python path. Let use:

const cacheKey = `InterpreterData-${interpreterPath}`;

My plan is to do something very similar for interpreter information displayed in the status bar. though I might use this same class (slightly altered). Either way, lets use a non-generic key.

Sure. I was not too convinced about raw path myself

DonJayamanne · 2018-04-05T03:39:34Z

src/client/activation/interpreterDataService.ts

+    return new InterpreterData(DataVersion, interpreterPath, `${majorMatches[1]}.${minorMatches[1]}`, prefix, searchPaths, hash);
+  }
+
+  private getInterpreterHash(interpreterPath: string): Promise<string> {


Just a thought, no change necessary:
Looks like a generic function that can go into IFileSystem for as a method named getFileHash ?

…into vsc

* Add support for multi roots for formatting and linting (microsoft#1281) * update to use latest api * config changes for multiroot workspace * linting support with multi roots * multi root support for formatters * determine workspace root path * revert change * support multiple configs per workspace folder * modify formatters to use resource specific settings * modified to settings are resolved using document uri * 1228 multi root master (#1) * fix microsoft#1280 handle env in shebang (microsoft#1290) * handle shebangs that resolve paths from env * oops * make test more specific * handle promise * fix microsoft#1282 use PYTHONIOENCODING variable (microsoft#1291) * fix microsoft#1270 debugger contribution changes (microsoft#1288) * add onDebug activation * use debug config provider for non workspace debugging * forgot to save file before commiting a merge * ability to opt out of telemetry using vscode settings (microsoft#1297) * Fix microsoft#1284 debugging parameterized tests (microsoft#1299) * fix microsoft#1298 remove vscode.startDebug command (microsoft#1300) * fix microsoft#1298 remove vscode.startDebug command * fix code review comments * added period as per code review comments * microsoft#1288 installer config and tests (microsoft#1302) * update to use latest api * config changes for multiroot workspace * linting support with multi roots * multi root support for formatters * determine workspace root path * revert change * support multiple configs per workspace folder * modify formatters to use resource specific settings * modified installer to pass resource for workspace resolution * null test in installer * canges to config settings to support multiroot workspace * changes to code refactoring to support workspace symbols * oops * modified to settings are resolved using document uri * unit tests for multi root support * fix unittests for multiroot * exclude files * add new line * config changes for multiroot workspace * new lines and enabled multi root linter tests * fix sys variables * added unit test to resolve ${workspaceRoot} in settings.json * fixed code review comments * fixed code review comments * fix microsoft#1276 Pre-commit hooks to ensure code complies with standards (microsoft#1277) * sanitize code * gulp file to check code hygiene * fix preLaunchTask in launch.json * added missing packages * enabled pre-commit using husky * enabled a few checks for precommit hook * fix tslint warnings when running tslint via gulp * exclude webpack building, else tries to pull in tests as well * improved checks for commits (strict) * added new lines * 1228 multi root workspace symbols (microsoft#1307) * update to use latest api * config changes for multiroot workspace * linting support with multi roots * multi root support for formatters * determine workspace root path * revert change * support multiple configs per workspace folder * modify formatters to use resource specific settings * modified installer to pass resource for workspace resolution * null test in installer * canges to config settings to support multiroot workspace * changes to code refactoring to support workspace symbols * oops * modified to settings are resolved using document uri * unit tests for multi root support * fix unittests for multiroot * exclude files * add new line * config changes for multiroot workspace * new lines and enabled multi root linter tests * fix sys variables * added unit test to resolve ${workspaceRoot} in settings.json * microsoft#1228 workspace symbols with multiroot support * fix test * added some data for workspace symbol tests * data for unit tests * fixed to add support for multit roots with unit tests * account for mutiroot files in sub directory * disable all but multiroot tests * fixed tests * include files for tests * Fixed travis tests for multi root workspace symbols (microsoft#1306) * added logging * more logging * yay * fixed * more fixes * fix tests * removed logging * enable all tests * uncommented * Added brackets around print statements (for p3) * Fixed travis unit tests (microsoft#1308) * update to use latest api * config changes for multiroot workspace * linting support with multi roots * multi root support for formatters * determine workspace root path * revert change * support multiple configs per workspace folder * modify formatters to use resource specific settings * modified installer to pass resource for workspace resolution * null test in installer * canges to config settings to support multiroot workspace * changes to code refactoring to support workspace symbols * oops * modified to settings are resolved using document uri * unit tests for multi root support * fix unittests for multiroot * exclude files * add new line * config changes for multiroot workspace * new lines and enabled multi root linter tests * fix sys variables * added unit test to resolve ${workspaceRoot} in settings.json * microsoft#1228 workspace symbols with multiroot support * fix test * added some data for workspace symbol tests * data for unit tests * fixed to add support for multit roots with unit tests * account for mutiroot files in sub directory * disable all but multiroot tests * fixed tests * fix tests * test where failing * properly determine root workspace * fix pytest unit test * delete files * add awaiter * use a path that works on multiple os * fixes * uncomment * invert * debug statements * use default workspace * reverted unwanted changes * oops * test unittests only * more logging * partial fixes to unit tests * run all tests * changes not to set paths for shebang tests * remove comments * update settings only if necessary * fix test * include files for tests * Fixed travis tests for multi root workspace symbols (microsoft#1306) * added logging * more logging * yay * fixed * more fixes * fix tests * removed logging * enable all tests * uncommented * Added brackets around print statements (for p3) * use resource when getting settings * fix microsoft#1315 unit tests need to wait for extension to activate (microsoft#1316) * fix microsoft#1314 allow for simultaneous language features (microsoft#1317) * microsoft#1228 support multi roots in language service (microsoft#1309) * update to use latest api * config changes for multiroot workspace * linting support with multi roots * multi root support for formatters * determine workspace root path * revert change * support multiple configs per workspace folder * modify formatters to use resource specific settings * modified installer to pass resource for workspace resolution * null test in installer * canges to config settings to support multiroot workspace * changes to code refactoring to support workspace symbols * oops * modified to settings are resolved using document uri * unit tests for multi root support * fix unittests for multiroot * exclude files * add new line * config changes for multiroot workspace * new lines and enabled multi root linter tests * fix sys variables * added unit test to resolve ${workspaceRoot} in settings.json * microsoft#1228 workspace symbols with multiroot support * fix test * added some data for workspace symbol tests * data for unit tests * fixed to add support for multit roots with unit tests * account for mutiroot files in sub directory * disable all but multiroot tests * fixed tests * fix tests * test where failing * properly determine root workspace * fix pytest unit test * delete files * add awaiter * use a path that works on multiple os * fixes * uncomment * invert * debug statements * use default workspace * reverted unwanted changes * oops * test unittests only * more logging * partial fixes to unit tests * run all tests * changes not to set paths for shebang tests * remove comments * update settings only if necessary * fix test * include files for tests * Fixed travis tests for multi root workspace symbols (microsoft#1306) * added logging * more logging * yay * fixed * more fixes * fix tests * removed logging * enable all tests * uncommented * Added brackets around print statements (for p3) * use resource when getting settings * support multiroot in language services * add additional tests for microsoft#1314 (microsoft#1318) * microsoft#1228 run all tests under multiroot (microsoft#1322) * modifications to fix tests to run under multi root setup * log errors * fix return type * fix linter messages * fix linter errors * changes to ensure code is formatted correctly * fixed comments * delete unwanted file * hide unwanted folders * fixes to linters to run on multiroot setup * udpate settings sequentially * log the output * show errors in deleting dir * removed prospector test, to be completed in microsoft#1319 * fixes to tests and sorting provider * fixed test for interpreter display * undo commenting of code * add new line * fix code review issues * ensure else is properly formatted * fix code review comments * fix microsoft#1304 preserve empty lines (microsoft#1329) * microsoft#1228 multiroot interpreter display (microsoft#1339) * modifications to fix tests to run under multi root setup * log errors * fix return type * fix linter messages * fix linter errors * changes to ensure code is formatted correctly * fixed comments * delete unwanted file * hide unwanted folders * fixes to linters to run on multiroot setup * udpate settings sequentially * log the output * show errors in deleting dir * removed prospector test, to be completed in microsoft#1319 * fixes to tests and sorting provider * fixed test for interpreter display * undo commenting of code * add new line * support multi root in interpreter display * fix linter * changed package version * disabled multiroot test * backwards compatible change * fix nose tests * revert change * enable test but disable it * multi root support in utils.ts * fixed microsoft#1328 * retries for flaky unit tests * retry beforeEach * common retry decorator * enable telemetry for extension loads * disable jupyter tests in multiroot tests * clean up python Path before and after testsclean up python Path before and after tests * rename test env variable * dispose cfg settings * dispose cfg settings * update comment * clean up * rearrange to ensurfe launching ext is first debug option * bug fix for display name * resolved code review comment * Fixed typp * 1228 multiroot interpreter ui changes (microsoft#1345) * fixes to unit tests and forgotten multiroot * globally retry all tests 3 times * refactor changing interpreters * added tests * fixed linter * removed redundant files * removed unwanted grep * remove blank line * fix 948 remove hardcoding of port number (microsoft#1353) * fix microsoft#1041 when debugging a test do not cancel it when re-discovering tests (microsoft#1354) * fix 1041 when debugging a test do not cancel it when re-discovering tests * create enum for creation of cancellation token * dispose correct cancellationToken * bug fix - in unit tests * bug fix - in unit tests * microsoft#1228 multiroot unit test runner (microsoft#1357) * fixes to unit tests and forgotten multiroot * globally retry all tests 3 times * refactor changing interpreters * added tests * fixed linter * removed redundant files * temp changes * more changes * lots of refactoring * adding support for multiroot workspaces * removed grep * copy changes for microsoft#948 and microsoft#1353 into multroot * replicate solution for microsoft#1041 and microsoft#1354 * microsoft#1041 create enum for creation of cancellation token * multiroot support for unit tests * remove empty line (linter warning) * delete pyc before making changes to py file * delete pyc file in teardown * merged multiroot master * pass uri of workspace when displaing prompt for configuration * pass uri to commands * fixed typos based on code review * prefix path with forward slash, as is done in the extension unit tests * include version number into display name

MikhailArkhipov added 30 commits March 27, 2018 14:58

Undo changes

a764bc9

Test fixes

9d1b2cc

Increase timeout

a91291a

Remove double event listening

bf266af

Remove test

7bc6bd6

Revert "Remove test"

8ce8b48

This reverts commit e240c3f.

Revert "Remove double event listening"

e3a549e

This reverts commit af573be.

Undo changes

c879aa6

Test fixes

716968f

.NET Core check

63a1385

Better find dotnet

7ea7717

Fix pip test

75be0da

Linting tests

2701768

Undo accidental changes

a22e705

Add clone and build PTVS

5441644

Appveyor PTVS build

97175e1

Fix slashes

1e97e6a

Enable build

86ac269

Try absolute path

7b4e73c

Fix xcopy switch

bd1fd77

Activate Analysis Engine test on Appveyor

72b91f1

Temporary only run new tests

e32320b

Disable PEP hint tests

55bb349

Test fix

3826ffa

Disable appveyor build and tests for PTVS for now

b62ba25

Remove analysis engine test from the set

a13770b

Remove VS image for now

ac646c8

Build/sign VSXI project

f754812

Run vsce from cmd

826dcee

Rename

8cb0c44

MikhailArkhipov added 18 commits April 4, 2018 12:51

Remove analysis engine test from the set

9345b9f

Build/sign VSXI project

82f9657

Run vsce from cmd

8f16e9a

Rename

a15cedf

Abs path vsce

3245d56

Path

35bf343

Move project

2ba533e

Ignore publishing project

3529ec2

Try csproj

0e57ab7

Add framework

1e0df43

Ignore build output folder

d7a5d78

Package before build

5bd5a71

More lost changes

0414216

Change setting type to bool

462a19d

Merge branch 'master' of https://github.com/Microsoft/vscode-python i…

446faaf

…nto vsc

Merge issues

3666dee

Merge issues

2e2d4e6

Merge issues

0a0e66c

Check search paths only if using cache

d972ff3

Undo change

ccf8eaf

DonJayamanne approved these changes Apr 5, 2018

View reviewed changes

Mikhail Arkhipov added 3 commits April 4, 2018 21:18

PR feedback

5a21ea9

Add async startup to PTVS

03b0bfd

Merge branch 'vsc' of https://github.com/MikhailArkhipov/vscode-python …

e1d2b15

…into vsc

MikhailArkhipov merged commit d213a79 into microsoft:master Apr 5, 2018

MikhailArkhipov deleted the vsc branch May 3, 2018 19:00

lock bot locked as resolved and limited conversation to collaborators Jul 31, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Language server startup time improvement #1299

Language server startup time improvement #1299

MikhailArkhipov commented Apr 4, 2018

MikhailArkhipov commented Apr 4, 2018

codecov bot commented Apr 4, 2018 •

edited

Loading

DonJayamanne Apr 5, 2018

MikhailArkhipov Apr 5, 2018

DonJayamanne Apr 5, 2018

MikhailArkhipov Apr 5, 2018

DonJayamanne Apr 5, 2018

MikhailArkhipov Apr 5, 2018

DonJayamanne Apr 5, 2018

Language server startup time improvement #1299

Language server startup time improvement #1299

Conversation

MikhailArkhipov commented Apr 4, 2018

MikhailArkhipov commented Apr 4, 2018

codecov bot commented Apr 4, 2018 • edited Loading

Codecov Report

DonJayamanne Apr 5, 2018

Choose a reason for hiding this comment

MikhailArkhipov Apr 5, 2018

Choose a reason for hiding this comment

DonJayamanne Apr 5, 2018

Choose a reason for hiding this comment

MikhailArkhipov Apr 5, 2018

Choose a reason for hiding this comment

DonJayamanne Apr 5, 2018

Choose a reason for hiding this comment

MikhailArkhipov Apr 5, 2018

Choose a reason for hiding this comment

DonJayamanne Apr 5, 2018

Choose a reason for hiding this comment

codecov bot commented Apr 4, 2018 •

edited

Loading