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

Performance with isolatedModules=true #32294

Closed
deftomat opened this issue Jul 8, 2019 · 11 comments
Closed

Performance with isolatedModules=true #32294

deftomat opened this issue Jul 8, 2019 · 11 comments
Assignees
Labels
Fix Available A PR has been opened for this issue

Comments

@deftomat
Copy link

deftomat commented Jul 8, 2019

TypeScript Version: 3.6.0-dev.20190704 | 3.5.2

Search Terms: isolatedModules, performance

We discovered that using isolatedModules=false in React apps drastically decrease type-check times. For example, in our application, tsc --watch --noEmit can took 5 seconds with isolatedModules=true. However, when we change it to isolatedModules=false, then the incremental type-check took 1 second or less.

The obvious solution is to disable isolated modules. Unfortunately, our application us create-react-app which use Babel to compile TS into JS and fork-ts-checker-webpack-plugin to run type-checks.

Due to Babel limitations, isolatedModules=true is a recommended option. TypeScript is definitely fast enough to make developers happy but we are forced to wait a lot of seconds just to catch a few edge cases.

So, my question is: Is this a bug or just a limitation of isolatedModules option?

Being able to use TypeScript in existing Babel toolchains is a huge thing but we are missing the last thing: SPEED 😞

(Related to facebook/create-react-app#7309)

@fatcerberus
Copy link

I'm thinking this probably happens because isolatedModules forces tsc to compile each file independently - so it has to recreate all the types (including ones from imports) every time, whereas with isolated modules turned off, it can more easily share types between different files, since the project is compiled as a unit.

@deftomat
Copy link
Author

deftomat commented Jul 9, 2019

@fatcerberus That's make sense, but I'm wondering if TypeScript is able to use some kind of cache as its probably know, which files changed.

The performance difference between these two modes is huge.

@fatcerberus
Copy link

Maybe --incremental? Just guessing here so don’t shoot me if it doesn’t help 😄

@deftomat
Copy link
Author

deftomat commented Jul 9, 2019

Well, as I know, --incremental can reduce the first-compilation time when user restart the tsc. It has no effect in watch mode.

@RyanCavanaugh
Copy link
Member

Is there a repo that demonstrates this behavior we could use for testing? --isolatedModules should not have measurable impact on performance

@sandersn sandersn added the Needs Investigation This issue needs a team member to investigate its status. label Jul 9, 2019
@sheetalkamat
Copy link
Member

@RyanCavanaugh https://github.com/microsoft/TypeScript/blob/master/src/compiler/builder.ts#L163 (added by #29252) we do not cache semantic diagnostics with isolated modules. isolatedModules and d.ts emit is disabled right now and is tracked by #29490. Once that's enabled we would be able to change that logic here instead.

@chibicode
Copy link

chibicode commented Jul 10, 2019

I can confirm this happening on my repo on TypeScript 3.5.3. I turned on isolated modules yesterday when Next v9 was released and its instructions told us to, then the typechecking performance degraded significantly on incremental compilation.

Isolated modules true, second compilation (incremental true):

$ tsc --diagnostics
Files:           197
Lines:         87013
Nodes:           NaN
Identifiers:     NaN
Symbols:      107686
Types:         26599
Memory used: 156889K
I/O read:      0.07s
I/O write:     0.04s
Parse time:    1.01s
Bind time:     0.39s
Check time:   56.41s
Emit time:     0.87s
Total time:   58.68s
✨  Done in 59.03s.

Isolated modules false, second compilation (incremental true):

$ tsc --diagnostics
Files:          197
Lines:        87013
Nodes:          NaN
Identifiers:    NaN
Symbols:      58662
Types:           73
Memory used: 97030K
I/O read:     0.09s
I/O write:    0.00s
Parse time:   0.81s
Bind time:    0.32s
Check time:   0.00s
Emit time:    0.00s
Total time:   1.13s
✨  Done in 1.43s.

There's no effect on the first compilation but there's a big difference on the second compilation (when incremental is true).

I can try to pick out some files from my repo for testing if necessary - @sheetalkamat let me know if you need it.

EDIT: See @russelldavis's reproducible repo below!

chibicode added a commit to chibicode/Y-Combinator-for-Non-programmers that referenced this issue Jul 10, 2019
@milesj
Copy link

milesj commented Jul 10, 2019

FWIW, I build all my projects with Babel and type check all of them without isolatedModules. You just have to be cautious of certain things, like re-exporting types.

@fatcerberus
Copy link

fatcerberus commented Jul 10, 2019

Nodes:          NaN
Identifiers:    NaN

Is this a bug?

@russelldavis
Copy link
Contributor

In case it helps, this can be easily reproduced with just a single file:

$ mkdir repro
$ cd repro
$ echo "export let foo = 1;" > foo.ts
$ time tsc --incremental foo.ts --tsBuildInfoFile tsconfig.tsbuildinfo

real  0m1.345s

$ time tsc --incremental foo.ts --tsBuildInfoFile tsconfig.tsbuildinfo

real  0m0.639s

$ time tsc --incremental foo.ts --tsBuildInfoFile tsconfig.tsbuildinfo --isolatedModules

real  0m1.314s

$ time tsc --incremental foo.ts --tsBuildInfoFile tsconfig.tsbuildinfo --isolatedModules

real  0m1.315s

(Ignore the first and third invocation of tsc, because of caching)

@russelldavis
Copy link
Contributor

Until this gets fixed, can someone update the docs at https://www.typescriptlang.org/docs/handbook/compiler-options.html to indicate that --isolatedModules will hurt performance when combined with --watch or --incremental?

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
Projects
None yet
Development

No branches or pull requests

8 participants