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

[3.6.3] tsc --watch extremely slow (regular tsc compiles fine). #34916

Closed
canonic-epicure opened this issue Nov 5, 2019 · 10 comments
Closed
Assignees
Labels
Fix Available A PR has been opened for this issue Needs More Info The issue still hasn't been fully clarified Rescheduled This issue was previously scheduled to an earlier milestone

Comments

@canonic-epicure
Copy link

For my codebase, compilation with --watch is extremely slow and sometimes results in the out-of-memory segfault. Compilation w/o --watch take 4s.

Can be reproduced in this repo: https://github.com/bryntum/chronograph/ (publicly available, branch quark_master, revision c58bc18120021423c730c9c2287058066157b8fa).

I found a similar issue, #34119 and tried few recommendations from it. Notably, tried to enable

        "declaration"               : true,
        "emitDeclarationOnly"       : true,

which produced 251 compilation errors. W/o those options all compiles fine.

It seems the incremental compilation uses some different data structures, than normal compilation. Or, may be, the serialization format is incorrect.

@canonic-epicure
Copy link
Author

This probably explains the fact, that my IDE (PhpStorm) very often goes into "broken" state, with false errors. See the screenshot - the npx tsc produces no errors, however, IDE thinks there's plenty of errors. I believe this is because IDE uses incremental compilation, which, as we saw, behaves differently from normal compilation.
image

This is a major usability hit - I basically had to revert to plain command line compilation, instead of using IDE facilities, which would bring me immediately to the source line with error.

I believe this issue should receive priority as having an incremental compilation that produces different results from normal compilation beats the purpose of the former. Basically the whole feature is broken and all recent claims in the TS changelog, about supporting faster builds are not correct.

@canonic-epicure
Copy link
Author

I also tried the following:

        "declaration"               : true,
        "emitDeclarationOnly"       : true,
  • and get 251 errors, most of them like:
src/replica/Replica.ts:54:37 - error TS4020: 'extends' clause of exported class 'MinimalReplica' has or is using private name 'UnsafeProposedOrPreviousValueOfSymbol'.

54 export class MinimalReplica extends Replica(MinimalChronoGraph) {}
                                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~

src/replica/Replica.ts:54:37 - error TS4020: 'extends' clause of exported class 'MinimalReplica' has or is using private name 'WriteSeveralSymbol'.

54 export class MinimalReplica extends Replica(MinimalChronoGraph) {}
  • conclusion: incremental compilation only works partially

@RyanCavanaugh RyanCavanaugh added this to the TypeScript 3.8.0 milestone Nov 8, 2019
@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Nov 8, 2019
@sheetalkamat
Copy link
Member

@samuraijack It is intentional that incremental depends on declaration emit to decide minimal dependency graph to build.. Your project reports errors with typescript@next so I cant see whats going on in there but I tried with the 3.4 release that it uses and declaration takes long because of circularity and it shows up in the declaration emit run which takes long..
I would recommend checking with typescript@next if this has improved and get back.
If you can repro this, @weswigham would be best person to let you know what you can do to improve it.

@sheetalkamat sheetalkamat added Needs More Info The issue still hasn't been fully clarified and removed Needs Investigation This issue needs a team member to investigate its status. labels Jan 29, 2020
@sheetalkamat
Copy link
Member

Probably same as #34119

@weswigham
Copy link
Member

Just a FYI, even if you can't figure out how to isolate a repro, explicitly writing return types on all of your exported functions can usually improve your perf. Now, when you can figure out which return type, specifically, shaves off the most time; then you have a repro. ;)

@canonic-epicure
Copy link
Author

@sheetalkamat Well, point is, that incremental compilation produces a result, different from the regular compilation. This feels like a serious design limitation somewhere in the incremental compilation process (I guess in the declaration serialization format). I really believe both compilation modes should produce exactly the same results. Is this fact admitted as a problem by the TS team? Should this be filed as a separate issue?

@weswigham Thanks for the info!

@RyanCavanaugh RyanCavanaugh added the Rescheduled This issue was previously scheduled to an earlier milestone label Aug 31, 2020
@grumd
Copy link

grumd commented Oct 22, 2020

@sheetalkamat Do you know if it's possible to do the --watch build without declaration emit and serialization? Even theoretically, if it can be implemented in TypeScript. It's notoriously slow to do the emit phase. In my 600k LOC project just running tsc without emit takes ~15-30 seconds. Running it with --watch takes 20-60 minutes, 99% of that time is taken by transformTime/printTime/Emit time. Incremental builds don't help this. Editing a file can result in a recompilation to take another 20 minutes depending on which file is edited.

I don't think it's intended for the --watch --noEmit incremental builds to be slower than just running tsc with --noEmit.

Would be great if Typescript could move away from emitting serialized declarations for the dependency graph and rely on something else instead. Could it be possible to rely on webpack somehow? I'm not sure. I just need an input from someone who knows the insides of Typescript.

Edit: I've tried finding which parts of my code cause the build to be so slow with emit, but had no success. Most of the time seems to be spent on making the redux store typings (CombinedStore stuff). I have a lot of redux modules. I couldn't isolate one type or one file that's causing issues, seems to just be the sheer amount of code.

@sheetalkamat
Copy link
Member

@grumd we use d.ts emit to determine if the shape of the file has changed to figure out if we need to emit and re-query semantic diagnostics only the changed file or its dependency graph. While this cannot be change, created #41219 which could help where you pass a new flag to say dont generate .d.ts file to determine change in shape of the file and assume that all changes affect the whole dependency tree rather than being local change.
This might help you because the perf is not going to be worse than running tsc again but could be faster depending on file you are changing which could result in smaller subset of files to refresh. Obviously incremental local change to file is going be slower than without this flag in watch/incremental mode.

@grumd
Copy link

grumd commented Oct 24, 2020

@sheetalkamat How does ts-server work in this regard? Recompilation in ts-server seems to be very fast in my project (when running typechecking in VSCode), but "tsc --watch" is very slow, and "tsc" is okay-ish, but slower than VSCode TS recompilation. I guess ts-server somehow recompiles fast without emitting d.ts? Why can't tsc --watch use the same approach as ts-server?

@RyanCavanaugh
Copy link
Member

We've done a lot of changes in the watch space since this was last looked at. Please open a new issue with concrete repo steps if you're still hitting this. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fix Available A PR has been opened for this issue Needs More Info The issue still hasn't been fully clarified Rescheduled This issue was previously scheduled to an earlier milestone
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants