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

Added caches to tsc's CompilerHost #27068

Closed
wants to merge 2 commits into from
Closed

Added caches to tsc's CompilerHost #27068

wants to merge 2 commits into from

Conversation

timocov
Copy link
Contributor

@timocov timocov commented Sep 13, 2018

Related issues: #26871, TypeStrong/ts-loader#825.

After debugging the compiler I have been noticed that some functions are called multiple times with the same arguments (for example, in our project tsc tried to check that tslib file exists almost 3k times).

Quite possibly that such behavior is related to set baseUrl compiler option.

For example, I have observe the following situation in our project (I don't understand what exactly affects on this):

Let's say that some file is placed /project/src/sub-folder folder, baseUrl option is set to ./src, importHelpers option is enabled, and the file needs to import helpers from tslib.
In our case we get the next requests in tryFile function:

/project/src/tslib.ts
/project/src/tslib.tsx
/project/src/tslib.d.ts
/project/src/tslib/index.ts
/project/src/tslib/index.tsx
/project/src/tslib/index.d.ts
/project/src/sub-folder/node_modules/tslib.ts
/project/src/sub-folder/node_modules/tslib.tsx
/project/src/sub-folder/node_modules/tslib.d.ts
/project/src/sub-folder/node_modules/tslib/index.ts
/project/src/sub-folder/node_modules/tslib/index.tsx
/project/src/sub-folder/node_modules/tslib/index.d.ts
/project/src/sub-folder/node_modules/@types/tslib.d.ts
/project/src/sub-folder/node_modules/@types/tslib/index.d.ts

This requests are made before tslib is resolved from /project/node_modules folder.

tsc makes at least 14 extra syscalls. But yeah, we can have local tslib and then resolving will stop.

The same resolving will be performed for all other files from /project/src/sub-folder folder.

Perhaps this example is related to tslib only, and for other imported modules the compiler makes less requests.

This PR adds caches for some CompilerHost's functions while compiling via tsc.

Below are some diagnostics for our project (average of 3 runs).

(our project is not compiled with master branch yet due #26978, but I believe the differences will be very similar)

typescript@3.0.1 (from npm):

Files:            1777
Lines:          227581
Nodes:         1042040
Identifiers:    316854
Symbols:        422329
Types:          158437
Memory used:   530938K (550667K, 491876K, 550273K)
I/O Read time:   1.91s (1.95s, 1.88s, 1.89s)
Parse time:      2.29s (2.34s, 2.22s, 2.30s)
Program time:    11.3s (11.26s, 11.32s, 11.32s)
Bind time:       1.98s (2.00s, 1.94s, 1.99s)
Check time:     14.22s (14.39s, 14.23s, 14.04s)
Total time:     27.39s (27.65s, 27.16s, 27.36s)

release-3.0 branch with applied fixes:

Files:                                 1777
Lines:                               227581
Nodes:                              1042040
Identifiers:                         316854
Symbols:                             422329
Types:                               158437
Memory used:                        535512K (531989K, 534535K, 540012K)
I/O Read time:                        2.06s (2.27s, 1.87s, 2.03s)
Parse time:                           2.42s (2.40s, 2.45s, 2.42s)
Program time:                         7.37s (7.46s, 7.30s, 7.36s)
Bind time:                            2.03s (1.99s, 2.05s, 2.05s)
Check time:                          14.88s (14.65s, 15.94s, 14.04s)
fileExists cache hits count:           9310
fileExists cache misses count:         6126
directoryExists cache hits count:     28806
directoryExists cache misses count:    2381
Total time:                          24.27s (24.09s, 25.28s, 23.45s)

Difference:

Memory used:   -4574K (-0.8%)
I/O Read time: -0.15s (-7.8%)
Parse time:    -0.13s (-5.7%)
Program time:  -3.93s (-35.0%)
Bind time:     -0.04s (-2.5%)
Check time:    -0.66s (-4.5%)
Total time:    -3.12s (-11.4%)

I believe that here only Program time and Total time are valuable.

I hope this fix can help someone improve its build speed.

Open questions:

  1. Perhaps we need to put these caches inside createCompilerHost function instead? If so, what about createSolutionBuilderHost in tsbuild.ts - it seems that it uses createCompilerHost as host for watch mode?
  2. Do we need to have performance marks for caches and output them with diagnostics?
  3. What about createWatchCompilerHost? It is possible that we can use caches while compilation and clear it right before compilation start.
  4. Do we need to cache any other function from compiler host?

Copy link
Member

@sheetalkamat sheetalkamat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we want this in compiler host. Just like #26259 the user of createProgram can always cache the results it wants. Eg. You can move this to tsLoader to cache this instead of of affecting usage of tsc

@timocov
Copy link
Contributor Author

timocov commented Sep 13, 2018

@sheetalkamat

I don't think we want this in compiler host

Now it is in tsc (not in the compiler host), is it ok?

Just like #26259 the user of createProgram can always cache the results it wants

I totally agree with it. I guess that tsc is such user too, right? We do not add cache to the compiler, only in command line tsc.

You can move this to tsLoader to cache this instead of of affecting usage of tsc

But what if we want to compile via cli tsc, without using the webpack?

@RyanCavanaugh RyanCavanaugh added this to the Community milestone Sep 17, 2018
@timocov
Copy link
Contributor Author

timocov commented Sep 18, 2018

@sheetalkamat you marked this PR as "requested changes", but can you please say what kind of changed I need to do? Or it means that this PR never will be merged?

@timocov
Copy link
Contributor Author

timocov commented Sep 21, 2018

@RyanCavanaugh @DanielRosenwasser anybody? 🙁

@timocov
Copy link
Contributor Author

timocov commented Oct 10, 2018

@RyanCavanaugh Any updates?

@sheetalkamat
Copy link
Member

@timocov closing this in favor of #28629

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants