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

tsc --watch --incremental recompiles are too slow for use with VS Code source #33329

Closed
mjbvz opened this issue Sep 9, 2019 · 7 comments · Fixed by #35711
Closed

tsc --watch --incremental recompiles are too slow for use with VS Code source #33329

mjbvz opened this issue Sep 9, 2019 · 7 comments · Fixed by #35711
Assignees
Labels
Domain: Performance Reports of unusually slow behavior Fix Available A PR has been opened for this issue Needs Investigation This issue needs a team member to investigate its status. VS Code Priority Critical issues that VS Code needs fixed in the current TypeScript milestone

Comments

@mjbvz
Copy link
Contributor

mjbvz commented Sep 9, 2019

From microsoft/vscode#80074

TypeScript Version: 3.6.2

Search Terms:

  • tsc
  • build
  • watch
  • incremental

Repo
Clone and start building VS Code with --incremental --watch

git clone https://github.com/microsoft/vscode.git
cd vscode
yarn
node --max-old-space-size=4096 ./node_modules/.bin/tsc --incremental -w -p src/tsconfig.json
  1. Let first full build complete
  2. Open src/vs/base/common/linkedList.ts
  3. Rename the public method unshift to deshift and save

Expected behavior:
Recompile is fairly quick. Our custom gulp-tsb builder takes a second or two for this case

Actual behavior:
Rebuild with tsc takes considerable time. This blocks VS Code from migrating to use standard tsc --watch instead of gulp-tsb

@mjbvz mjbvz added the VS Code Priority Critical issues that VS Code needs fixed in the current TypeScript milestone label Sep 9, 2019
@mjbvz mjbvz changed the title tsc --watch --incremental to slow for building VS Code Sep 9, 2019
@mheiber
Copy link
Contributor

mheiber commented Sep 18, 2019

@mjbvz I'd be curious if we're bumping into the same issue with version 3.6.2 as us.

The issue we're seeing is not related to --watch.

If I understand correctly, --incremental does not seem to do incremental compilation in v3.6.2:

Stats for a codebase with ~9K CLOC:

v3.4.5 and v3.5.3 with --incremental

  • ~7 seconds cold
  • ~3.5 seconds warm

3.6.2 with --incremental

  • ~8 seconds cold
  • ~8 seconds warm
@amcasey amcasey added the Domain: Performance Reports of unusually slow behavior label Sep 18, 2019
@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Sep 19, 2019
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 3.8.0 milestone Sep 19, 2019
@sheetalkamat
Copy link
Member

sheetalkamat commented Sep 19, 2019

From email chain::

tsc handles scenario where you might introduce errors through indirect import change where as gulp-tsb doesn't (which at that point was intentional`.
You can indirectly introduce error or fix it even if the signature of middle dependency doesn’t change .. eg look at tests in https://github.com/microsoft/TypeScript/blob/master/src/testRunner/unittests/tscWatch/emitAndErrorUpdates.ts

The gulp tsb is rescanning diagnostics only for (apart from linkedList.ts)

0: "c:/github/vscode/src/vs/workbench/services/decorations/browser/decorationsService.ts"
1: "c:/github/vscode/src/vs/workbench/api/common/extHostDocumentSaveParticipant.ts"
2: "c:/github/vscode/src/vs/platform/commands/common/commands.ts"
3: "c:/github/vscode/src/vs/editor/contrib/smartSelect/bracketSelections.ts"
4: "c:/github/vscode/src/vs/editor/contrib/format/format.ts"
5: "c:/github/vscode/src/vs/editor/browser/services/openerService.ts"
6: "c:/github/vscode/src/vs/editor/browser/core/keybindingCancellation.ts"
7: "c:/github/vscode/src/vs/base/test/common/linkedList.test.ts"
8: "c:/github/vscode/src/vs/base/common/event.ts"

While tsc does it mostly for all files since they could be affected.

Instead of linkedList I changed C:\github\vscode\src\vs\editor\browser\view\viewImpl.ts to change removeOverlayWidget to just OverlayWidget and the time is around 6s.

Files:                        2136
Lines:                      639085
Nodes:                     3400884
Identifiers:               1058817
Symbols:                    524076
Types:                      105538
Memory used:              1131422K
Assignability cache size:    34056
Identity cache size:         42025
Subtype cache size:          12649
I/O Read time:               0.00s
Parse time:                  0.00s
Program time:                0.04s
Bind time:                   0.01s
Check time:                  5.87s
transformTime time:          0.37s
commentTime time:            0.05s
printTime time:              0.95s
Emit time:                   0.96s
I/O Write time:              0.02s
Total time:                  6.88s

You will notice in most cases the time would be in checking than emitting and that accounts for the time spent in rescanning semantic diagnostics for files that could be impacted because signature of the file changed.

@mheiber
Copy link
Contributor

mheiber commented Sep 20, 2019

We're seeing no speedup from --incremental in 3.6 #33329 (comment), it looks like the state from the .tsbuildinfo file isn't being used.

@sheetalkamat
Copy link
Member

@mheiber Did you try typescript@3.6.3?

@mheiber
Copy link
Contributor

mheiber commented Oct 3, 2019

@sheetalkamat I found the same with typescript@3.6.3. Tested on both Windows 10 and Linux. Sorry for the late reply.

@sheetalkamat
Copy link
Member

sheetalkamat commented Oct 3, 2019

@mheiber Please share a repro so we can investigate it, Also please create separate issue for that since this is tracking vscode's issue of rebuild taking longer which happens to be working as intended at moment but we are still thinking about what can be done here.

@mheiber
Copy link
Contributor

mheiber commented Oct 4, 2019

Thanks @sheetalkamat . I added steps to reproduce to an existing issue for --incremental not speeding up compilation: #33667

sheetalkamat added a commit that referenced this issue Dec 16, 2019
…ts files for files indirectly importing affected file

Fixes #33329
sheetalkamat added a commit that referenced this issue Dec 16, 2019
…ts files for files indirectly importing affected file

Fixes #33329
@sheetalkamat sheetalkamat added the Fix Available A PR has been opened for this issue label Dec 16, 2019
sheetalkamat added a commit that referenced this issue Jan 3, 2020
…cking and generating .d.ts files for files indirectly importing affected file (#35711)

* Baselining tsc --watch output

* Add noIndirectImports as a option to skip checking and generating .d.ts files for files indirectly importing affected file
Fixes #33329

* Rename option to assumeChangesOnlyAffectDirectDependencies

* Description change as per feedback
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Domain: Performance Reports of unusually slow behavior Fix Available A PR has been opened for this issue Needs Investigation This issue needs a team member to investigate its status. VS Code Priority Critical issues that VS Code needs fixed in the current TypeScript milestone
5 participants