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

Add cache for some FS operations while compile #825

Closed
timocov opened this issue Aug 28, 2018 · 10 comments
Closed

Add cache for some FS operations while compile #825

timocov opened this issue Aug 28, 2018 · 10 comments

Comments

@timocov
Copy link
Contributor

timocov commented Aug 28, 2018

Hi there.

TypeScript makes a lot of identical FS operations such as:

  • checking that some file/directory exists (e.g. for the identification an extension of a file it can make up to 3 fileExists calls)
  • resolving original file path

To solve an issue described in microsoft/TypeScript#22576 in our project we have developed a small tool, which compiles all TypeScript files one-by-one as entry point and checks errors.
Now the project has grown to 1000+ TypeScript files and the test takes more than 20-30 minutes (sometimes it even fails because of timeout after 1 hour).
After investigating the problem we found out that the reason is a lot of FS operations made by TypeScript compiler.

(I believe that we can assume that compilation process is "atomic" operation and we can just add cache for some "heavy" syscall-operations like fileExists)

After adding cache for fileExists, realpath, directoryExists and getSourceFile (getSourceFile is specific for our tool) we get 20-30x decreasing of working time (to 1 minute).

I hope that this approach can be used in ts-loader too to speedup compilation time.

I roughly added some caches to fileExists, realpath and directoryExists functions and here are miss/hit values (for our project) in cold-compilation:

  • fileExists: miss=55847, hit=630450
  • directoryExists: miss=51, hit=1914
  • realpath: miss=39, hit=466

The compilation time is decreased from 850s to 330s.

@timocov
Copy link
Contributor Author

timocov commented Aug 28, 2018

Also, I'm not sure about correctness

const fileExists = (path: string) =>
compiler.sys.fileExists(path) || readFile(path) !== undefined;
lines - if compiler.sys.fileExists(path) returns false we'll try to read file (but file does not exist).

@timocov
Copy link
Contributor Author

timocov commented Aug 28, 2018

I have checked with ts-loader@3.5.0, in ts-loader@2.3.3 cold-compilation time is ~600s (I do not know why), but still can be more effective.

@johnnyreilly
Copy link
Member

If you'd like to open a PR that tests this out that could be an interesting basis for discussion. Question though: how do you deal with cache invalidation? Whilst there's clearly benefit in caching, how do you deal with file structure / directories changing over time?

@timocov
Copy link
Contributor Author

timocov commented Aug 28, 2018

If you'd like to open a PR that tests this out that could be an interesting basis for discussion

I'll try, but I'm not familiar with webpack plugins and it may take some time.

how do you deal with cache invalidation? Whilst there's clearly benefit in caching, how do you deal with file structure / directories changing over time?

If we can presume that compilation is "atomic" operation then, I believe, we can just clear caches before starting compilation.

@johnnyreilly
Copy link
Member

I'll try, but I'm not familiar with webpack plugins and it may take some time.

That's cool; take your time and give it a whirl!

If we can presume that compilation is "atomic" operation then, I believe, we can just clear caches before starting compilation.

So the cache lasts the duration of a single compilation? Each triggering of ts-loader would be working with a fresh cache? Okay; sounds reasonable.

@timocov
Copy link
Contributor Author

timocov commented Aug 29, 2018

Also, I noticed that ts-loader runs TypeScript synchronously in the main process what causes blocking webpack's main process and webpack stops doing other stuff with other files until TypeScript finish compiling (I guess it is applicable for projects with mixed typescript and javascript files).

Is it possible to move LanguageService to other process and handle compilation there while webpack doing other work in the main process? In this case, I believe, we will compile in other process while webpack doing own work in the main process without blocking (but yeah, at some time we'll wait until the compilation is finished because there are no other non-typescript files to process by webpack).

What do you think about it? Do we need to create another issue for that or it is hard to implement and we wouldn't do it?

@johnnyreilly
Copy link
Member

Is it possible to move LanguageService to other process and handle compilation there while webpack doing other work in the main process?

I'm not totally sure I get your meaning. But I wonder if this is covered by the fork-ts-checker-webpack-plugin? https://github.com/Realytics/fork-ts-checker-webpack-plugin

@timocov
Copy link
Contributor Author

timocov commented Aug 29, 2018

But I wonder if this is covered by the fork-ts-checker-webpack-plugin?

Not exactly. We can use fork-ts-checker-webpack-plugin along with transpileOnly: true flag to speedup performance, right? In some cases we can't use transpileOnly flag (for example if we use global const enums to inline some common constants).

Is it possible to move LanguageService to other process

For now LanguageService is created in main process and when webpack asks loader to process any file we ask LanguageService to compile this file, and it compiles it in the main process, right?

@johnnyreilly
Copy link
Member

For now LanguageService is created in main process and when webpack asks loader to process any file we ask LanguageService to compile this file, and it compiles it in the main process, right?

Exactly right. If you'd like to experiment with moving it to a different process that could be fun! But yes, please raise another issue / PR to cover that I think.

@timocov
Copy link
Contributor Author

timocov commented Aug 29, 2018

But yes, please raise another issue / PR to cover that I think.

Ok, I just created #830 for that.

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

No branches or pull requests

2 participants