-
Notifications
You must be signed in to change notification settings - Fork 71
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
check types in a separate thread #113
Comments
This is a good idea, another speed up might be had by rewriting the plugin to return promises (now that rollup expects that from plugins) |
Thought I should add some notes here around some investigations and concerns I had over the past few months. Performance is not necessarily better when threaded/forkedThis is the first caveat I thought would be important to mention here. In Webpack-land, various TS loaders have had all sorts of performance issues and tried different solutions, and some ended up being de-optimizations. With things like caching and forking/threading, it is not a straightforward optimization. The answer of does it increase performance is "it depends", specifically on the characteristics of the machine, the project, and the bottlenecks. CachingCaching often uses more memory and/or uses the FS, so there is I/O, memory, and storage trade-offs. In modern computers, I/O is often the bottleneck, with disk being significantly slower (especially if still on HDD). So caching can actually make things slower for certain machines and smaller projects which do not need to do much compute. For larger projects, this trade-off might be worthwhile. But larger projects may also require more memory as well so that can bottleneck as well. "Storage is cheap" nowadays, so that one is rarely a concern. rpt2 actually does have a cache built-in and enabled by default. I've actually mentioned potentially doing some heuristics to maybe disable by default in #362. And there's still a few optimizations to be had in the cache too. Forking/ThreadingThreadingAs a baseline, Node doesn't have fully capable threads. Due to this limitation, we still can't share much memory between threads in Node. In rpt2's case, we'd want to share, at the very least, the TS So whether we were to use Forking ProcessesThe caveat with forking, as mentioned above, is that you're going to use more memory. Potentially significantly more, as each process has to duplicate some memory. That also means that we may need to use more CPU as well, because not only do we have to construct and fill more data structures, but we may also need to re-parse various TS source code multiple times since we can't pass around the parsed objects between processes. And message-passing adds overhead too. So if with that understanding of various trade-offs as our baseline, we can dive into more practical specifics. Prior Art in WebpackWe can look at prior work in Webpack-land as examples. The best example of forking actually causing a de-optimization would be the history of Some specific references: s-panferov/awesome-typescript-loader#497, s-panferov/awesome-typescript-loader#649, plus others. So forking has a checkered performance history in Webpack, probably due to the above theoretical trade-offs. TS's official Performance docs even mention some optimizations and these caveats in other tooling as well. Potential Next StepsWith that being said, it might be good to try some benchmark or add an experimental forking mode to rpt2. But, due to the above trade-offs and the possibility that this is actually a de-optimization, this is most certainly low priority -- it may not even be worthwhile to pursue something so experimental that may be thrown away. Something I mentioned in #148 (comment) that could be a very relevant optimization for some users and/or useful for this feature would be the introduction of Similarly, we might be able to utilize the There would still be more kinks to work out for sure, but that might simplify the work required a good bit if doable, as we wouldn't have to add nearly as much internal code to handle that. |
Wanted to note here that there is an older, alpha, unmaintained plugin: I have not tested to see if it works / still works, but it's ~around the same size as rpt2 in LoC plus significantly heavier dependencies, which may serve as a testament to the complexity of this issue. |
What happens and why it is wrong
rollup --watch
takes 14sec to rebuild withcheck: true
, and only 3sec withcheck: false
. I propose creating a separate node process just for type checking, similar to fork-ts-checker-webpack-plugin. This would allow us to have faster builds without losing realtime type errors.Versions
The text was updated successfully, but these errors were encountered: