-
-
Notifications
You must be signed in to change notification settings - Fork 433
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
Compilation significantly slower than TSC, and massive regression in 1.2.2? #393
Comments
Is it exactly 1.2.2 where the change occurred? What's performance with the previous release? |
Exactly 1.2.2 yes; The numbers above are for 1.2.1 and 1.2.2 respectively. |
It may be down to this: #377 |
If it's raw speed you're after do you want to try transpile mode? |
Have tried that but it loses all worthwhile type checking. It still takes around 11 seconds on 1.2.1, which is the same speed as tsc doing everything. |
Ts-loader will never be faster than tsc - that's a guarantee! I don't have suggestions you're going to love I'm afraid; I'd say fixing on 1.2.1 is probably your best bet since we're not planning to pull the Feature added in 1.2.2. it's possible in the future we may look at having 2 processes; one for type checking and one for transpilation but I couldn't say when that might land. PRs that improve the performance of ts-loader are always welcomed though! |
I understand it won't be as fast but 1.2.1 is already 1.5-2x slower than tsc. 1.2.2 is 50% slower than 1.2.1 for the initial compilation and gets seemingly no speed up from incremental at all, making incrementals nearly 5x slower than 1.2.1. |
I am curious as to what aspect of your project makes performance degrade so much between 1.2.1 and 1.2.2. Certainly my own projects seem OK and they have a significant size. If you can supply a simple repro which demonstrates the significant performance drop off when you use that switch it could be helpful. |
I don't really have any way to do that, our project is relatively large and proprietary. Trying to figure out what causes anything in a build system is a needle in haystack situation. Here's the diagnostics for raw tsc:
If there's any easy way to do profile or dump diagnostics for ts-loader itself, I'd be happy to provide that, but it doesn't appear to be instrumented in any way? |
Having played around a little more it does seem that we may have a performance regression on our hands. However, given that the choice is between losing performance and losing correctness I'm a little torn. BTW we should probably introduce some kind of benchmark tests to catch this sort of thing. Not sure how this would best be implemented; we'd want to cover initial compilation and subsequent compilations. Something to think about. |
Yeah 1.2.2 is certainly somewhat slower as in general it needs process more files for any given change compared to earlier versions. However, the only way I'd imagine the initial and incremental build times could be the same would be if somehow all the files in the project depend on all other files (directly or indirectly). @prencher does the incremental build time depend on which file you modify at all? Can you try creating a test file that possibly only one other file depends on and see what the incremental build performance is when modifying that? Also, if you enable the Webpack option that lists the files that are built for each recompilation you can see if all the files always end up getting rebuild for some reason. |
On 1.2.1, the compile time -- while still much slower than TSC directly -- does vary based on the file being saved. On 1.2.2, there seems to be no effect, and incrementals are almost as slow as initial. We do have a very import heavy code base with rollup files, which I'm sure contributes to having a ton of files for any single incremental. TSC itself seems to be able to handle it pretty well, though. |
Thanks for commenting @prencher. Although I'm really not keen on the idea I am considering rolling back the change in 1.2.2. The extra correctness is great but the performance cost is heavy; perhaps too heavy. I need to think about this some more but it's definitely something I'm considering. @smphhh do you have any ideas for how we could improve performance without reverting? |
My hunch is that there is no quick and easy fix for this, especially if we don't have a repro case. For one of my bigger projects the initial compilation time is around 25 seconds while the incremental compilation time is just 1-2 seconds depending a bit on the file that is modified (and roughly double that with devtool: "source-map". This is with babel loader wired after ts-loader. So clearly there must be something particular about the project of @prencher. I'd still be interested in the output of Webpack when run with --display-modules, that way you can see which files actually get rebuild for each change. Reverting the change is probably not too bad of an idea, ts-loader users have been able to live with the not-perfectly-correct output since the beginning (I guess) after all. I think improving the performance of ts-loader might require a bit more of an holistic approach with focus on overall architecture etc, which is probably best considered along with the merge with awesome-typescript-loader. BTW @prencher have you tried ATL? Should be pretty easy to test out the performance with that. P.S. As a bit of a disclaimer, I'm currently not even using ts-loader for the main project I'm working on because the setup is rather complex with React+Relay and the related GraphQL schema stuff. Instead I just have several different watch processes that watch each other's output. |
My own projects are also using babel-loader to transpile the emitted ES2016 code. @prencher could you provide the output @smphhh suggests please?
That's true. We have made improvements along the way (I think @Strate added the initial reverse dependency graph stuff which helped). |
ATL for Webpack 1, and the current stable version for Webpack 2 are both about the same as ts-loader, while the current As for the output, we invoke the Webpack API directly and I've not been able to find heads or tails in their docs about making this output show up. If you can assist there, I'd be able to enable it; I can't share the output for the same reason I can't share source code, but I would at least be able to confirm if a large number of files are being rebuilt. Edit: modules on stats seems to do the trick - it's actually only showing 6 of the 971 modules as built for the chunk when I change a file that causes a 30 second incremental on 1.2.1. I'll try with 1.3.0 next. |
I think this is what you need: https://github.com/webpack/docs/wiki/node.js-api
|
Oh and based on an initial look the revert would be a one line change; so simple to implement if we go that route. (and also would have to disable the |
So with 1.3.0 it builds 833 modules on incremental, compared to 6 on 1.2.1, out of 971 total. |
Okay that's pretty poor. I'll look to revert the change in the next release. I'll post once it's released (not sure when) |
@prencher If the performance with stable ATL is about the same as with ts-loader 1.3.0, I assume the number of modules rebuilt is also about the same? What if you only modify the entry file which presumably no other file depends on? You don't happen to have circular dependencies in your project? My current (somewhat speculative) interpretation of the issue is that:
|
@smphhh ATL pre-3.0 is roughly the same speed as ts-loader 1.2.1. 3.0 appears to be significantly faster from initial build times but again, it fails on incrementals on windows at the moment, so I can't confirm. I can check on the number of files affected with entry point, as well as ATL for webpack 1 tomorrow. As for the dense dependency structure, while that is true, TSC itself seems to be able to handle it very well; it's not fast at sub-30 initial and ~12 second incrementals, but it's a far cry from ts-loader which 2-3x slower with 1.2.1. ATL 3.0 seems to be getting close from the numbers I recall. |
@prencher Ok then I misinterpreted you saying that stable ATL is about the same as current ts-loader, and the above interpretation doesn't hold. As, AFAIK, stable ATL handles deep dependencies properly, this issue might very well be something specific about ts-loader's implementation. BTW, tsc's watch implementation itself is currently far from optimal, see microsoft/TypeScript#10879 |
@smphhh Yeah I am waiting with bated breath for improvements in TSC as well, but even in its current state, it is much faster than any loader - be it ts-loader, atl, or browserify with tsify. Unfortunately using it directly is not really an option, other than to get an idea of the raw performance of TSC and what the various build systems are adding in terms of overhead. That all being said, while I would love for improvements to ts-loader, this is more about restoring the performance level of 1.2.1. |
One more thing, I noticed you are compiling with isolatedModules: true, and ATL seems to handle dependencies a bit differently in that case, see https://github.com/s-panferov/awesome-typescript-loader/blob/master/src/index.ts#L79. That's one thing that probably makes a difference for the performance when comparing ts-loader to ATL, maybe we should do a similar check in ts-loader @johnnyreilly ? |
So I believe the way to resolve this would be to not define any Webpack dependencies when compiling with isolatedModules: true. This would essentially revert ts-loader's behavior back to pre-1.2.2 in that case, and still allow correct deep dependency handling when compiling with isolatedModules: false. I don't have time to completely think this through right now though so might be missing something. |
I'd be happy if you wanted to introduce that similar check. If you want to submit a PR to that effect let's give it a whirl. Hopefully @prencher could give it a try on his codebase and confirm. In the meantime I've discovered I need to do a little housekeeping around the test pack now that 2.1 has shipped. Whilst I'm doing that I'll plan to comment out the 1.2.2 approach and re-add the 1.2.1 approach. This will get us back to having reasonable performance. If your PR bears fruit then we can move to it instead. Hope that's all clear. |
@johnnyreilly Sounds reasonable. Not sure when I'll have time to work on that though. |
That's fine - whenever you get a chance we'll appreciate it! :Smile: |
And I've just learned emoji are case sensitive. 😋 |
@smphhh Editing the entry point is indeed much faster . We do have some circular dependency chains here and there due to rollup files. @johnnyreilly Can confirm that 1.3.1 has restored performance for our project. Thanks! Please feel free to reach out if you need me to test performance in the future, we seem to have a pretty extreme case. 😃 |
Just thought I'd list my own compile times, for anyone finding this page through search (as I did): tsc version: 2.1.4 Initial compileTSC + Webpack: (tsc outputs to folder, which webpack watches)
Webpack with ts-loader:
Subsequent (incremental) compilesTSC + Webpack:
Webpack with ts-loader:
So in my case, ts-loader is preferable for dev-mode, since most compiles are going to be recompiles. However, the same is not true for deploys, since you can't use webpack incremental compile there -- so I use tsc + webpack for deploys, which about halves the total deploy time there. (I keep a background console running with tsc, for incremental compiling whose output is then used by the deploy scripts) |
With TypeScript 2.0.10's TSC, our compilation takes around 30 seconds, with an incremental time around 13 seconds, using the following configuration, and outputting individual files with source maps:
However, when using ts-loader, the build takes around 70(!) seconds, with incrementals anywhere from 20-30 seconds.
That's bad enough, but with 1.2.2, it now takes around 98 seconds for both the initial compilation, and the incremental.
For reference, here's webpack configuration:
This is on Windows 10 using Node.js 6.9.1 and webpack 1.13.3.
Any idea? I've been completely unable to determine why.
The text was updated successfully, but these errors were encountered: