Compile TypeScript incrementally

[mzdunek93]

As described here: #1692 (comment)

This is very buggy and unpolished yet, doesn't work with HTTP URLs (sorry), however, for relative paths, it works by only updating the files that need to be updated - including dependent files

[CLAassistant]

All committers have signed the CLA.

[ry]

How much faster are incremental compiles?

[kitsonk]

Not that this is a bad idea (it is one of the things @bartlomieju and I talked about looking into for a while) and thanks for taking a stab at it @mzdunek93 , I do think we have to have reasonable expectations about performance improvement (it is only certain situations where it will benefit), as well as make sure that the information we present to TypeScript works properly to populate tsbuildinfo... this coupled with the desire to move all the dependency analysis outside to Rust, it feels like we need to get that sorted first before making an attempt at this.

As @ry is implying, we should be data driven on this too... we should add a benchmark first with a complex TypeScript compilation scenario. deno cache could be used to populate the cache and do the first compilation, and then a change/modification to a fixture could be used to trigger a new compilation.

Also, instead of #1692 (transitory dependencies), this really is covered by #4323 (performance of TypeScript compiling). Transitory dependencies is likely still an issue that this PR doesn't specifically address.

[mzdunek93]

How much faster are incremental compiles?

@ry That should depend on the specific case. Since incremental doesn't recompile files that haven't changed, it should be especially useful if you're importing a big library and only changing your small main file.

Also, instead of #1692 (transitory dependencies), this really is covered by #4323 (performance of TypeScript compiling). Transitory dependencies is likely still an issue that this PR doesn't specifically address.

@kitsonk It does adress it, since the TS compiler takes care of determining which files need to be recompiled after changes. The scenario you decribed in the issue is fixed.

[bartlomieju]

I don't know much about buildinfo file, but it does seem it might be a solution to transitive dependency problem. There's one thing that immediately pops into my mind: how do we store this build info file? Should it be scoped to "cwd"?

Eg. if I'm in /dev/deno-project and run /dev/deno-project/mod.ts then buildinfo file should be probably cached in $DENO_DIR/gen/file/dev/deno-project/buildinfo.json. The problem is that changing cwd would require a new buildinfo. That's pretty simple and I believe that's actually how cargo work as well (for example cargo build from one of nested directories causes full recompilation).

Anyway, like @kitsonk said, we're refactoring TS compiler at the moment quite furiously, so it might be a good idea to hold off with this PR for a few days.

[mzdunek93]

There's one thing that immediately pops into my mind: how do we store this build info file? Should it be scoped to "cwd"?

@bartlomieju Currently I store it under the name {root TS file}.json next to the compilation result in the cache. That way you can have multiple files in the same location with separate build infos. Changing cwd shouldn't trigger recompilation.

[ry]

I’d be quite interested to see a time comparison of doing incremental compile with and without this patch.

[mzdunek93]

I’d be quite interested to see a time comparison of doing incremental compile with and without this patch.

@ry I made up a contrived example of a small file with a 500k lines-long import. Compilation times were as follows:

uncached - ~61s
after changing calc.ts - ~59s
after changing index.ts - 19s
without changes - ~13s

This is a nice improvement, but there are a few observations. First, compilation of the same project using tsc took me much less time, uncached was just ~17s and with just index.ts changed ~5s. Not sure why (maybe it was the config), but I'll have to investigate. Second, looks like typescript still does a lot of work even if there are no changes - on one hand there still needs to be some analysis of the unchanged dependencies to get type information for the imports, on the other hand if there are no changes at all anywhere it should be all skipped. I think the process can be sped up by saving declaration files of the dependencies and substituting the original source with them in case the file is unchanged - that should significantly shorten processing time while still giving us the type information necessary for typechecking the recompiled files. Also, unchanged files which are not depended on by recompiled files should have their source removed completely that's wrong, every file in the dep tree is either recompiled or depended on by a recompiled file, sorry.

[ry]

uncached - ~61s
after changing calc.ts - ~59s
after changing index.ts - 19s
without changes - ~13s

I'm not sure I understand this. Without the patch incremental compile takes 13 seconds (best) and with the patch it takes 19s (slower)?

[mzdunek93]

I'm not sure I understand this. Without the patch incremental compile takes 13 seconds (best) and with the patch it takes 19s (slower)?

No, what I meant is that without any changes to the TS sources (nothing is emitted, compilation shouldn't actually happen at all), compilation takes 13 seconds. If I change only index.ts (the small main file which imports the big file), it takes 19s. If I change only calc.ts (the big import), it takes 59s, and without cache, or with both of the files invalidated, it takes 61s.

The real benefit of the PR is seen in the second case. Currently on master, if I change the root file, all of its dependency tree is compiled and emitted, no matter what's in the cache. That means it takes about 61s - with incremental compilation this only happens if we have no cache or if both sources are invalidated. With this code, the unchanged dependencies are not recompiled and therefore compilation in this case takes 19s - a 42s gain, or 3x reduction. Of course this all will depend on how big and complicated the import is - you will probably rarely import files that are 500k lines long.

I think the real issue here is that compilation in deno takes about 4x longer than with tsc.

[ry]

No, what I meant is that without any changes to the TS sources (nothing is emitted, compilation shouldn't actually happen at all), compilation takes 13 seconds

I see, so 13 seconds just to parse the already cached JS.

Did you perform the test without your patch? What are the numbers in that situation?

[mzdunek93]

I see, so 13 seconds just to parse the already cached JS.

The TS compiler doesn't actually read the cached JS - it first gets the TS source, hashes it, and then compares the hash with the value in the build info file. If it's the same, the file is not emitted. However something is obviously still done with it since it takes 13 seconds - I'm guessing some sort of parsing and type binding - but if there are no changes in the entire dep tree it should end at hashing. I think this case should be handled in Rust and if there are no changes in the whole tree, we don't launch the compiler at all.

Did you perform the test without your patch? What are the numbers in that situation?

It all took about 57s - with the exception of both files being in cache, in which case it took 0s, since the TS compiler wasn't launched at all. Like I said, the biggest gain is in the case when a small root file, that imports large dependencies, has to be recompiled - currently all of the dependency tree is recompiled and emitted, while with incremental compilation that's only necessary for the root file.

[mzdunek93]

@ry I wanted to apologize for making a big mistake in my measurements - I was measuring on the debug version rather than the release version, which caused the compilation to be many times slower than it should have been. The actual measurements look like this: 9.35s for uncached compile, 3.5s when recompiling index.ts, 2.5s for full cache. On current master, the recompile takes 8.8s, which means that incremental compilation has about 8.5% overhead in the general case, but can cause significant speed up when only needing to compile a part of the source, which I suspect will be a frequent occurrence during development.

[kitsonk]

which means that incremental compilation has about 8.5% overhead in the general case, but can cause significant speed up when only needing to compile a part of the source, which I suspect will be a frequent occurrence during development

Then it feels like this should be flagged, rather than the default.

[wongjiahau]

I'm looking forward to this, now the development cycle of using Deno is just too slow to be productive.

[bartlomieju]

Thanks @mzdunek93! I used your PR as a basis for this #6428 which has already landed.