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

Try to move LanguageService to separate process #830

Closed
timocov opened this issue Aug 29, 2018 · 21 comments
Closed

Try to move LanguageService to separate process #830

timocov opened this issue Aug 29, 2018 · 21 comments
Labels

Comments

@timocov
Copy link
Contributor

timocov commented Aug 29, 2018

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).

It is possible that we can 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).

(moved from #825 (comment))

@johnnyreilly
Copy link
Member

Feel free to spike this and send a PR to collaborate on. Could be interesting!

@felixfbecker
Copy link

Doesn’t this work with thread-loader?

@johnnyreilly
Copy link
Member

@felixfbecker also my original thought; there is a distinction - see discussion in linked thread. This is the relevant quote from @timocov:

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).

@timocov
Copy link
Contributor Author

timocov commented Sep 10, 2018

Doesn’t this work with thread-loader?

If I understand correctly how thread-loader works, I do not think that it may help us. If we'll use thread-loader - we'll have some processes with ts-loader in each one and we still may block main process.

@felixfbecker
Copy link

thread-loader is not the same as fork-ts-checker-webpack-plugin:
https://github.com/webpack-contrib/thread-loader
https://github.com/Realytics/fork-ts-checker-webpack-plugin

We use thread-loader with ts-loader together. @timocov could you clarify what part of ts-loader still blocks the main process with that setup?

@timocov
Copy link
Contributor Author

timocov commented Sep 10, 2018

@felixfbecker I cannot tell you for sure, but compilation TypeScript is not async and every time when you request to compile some file it blocks the process until compilation is finished.

In our case we have the following situation:

  • webpack starts bundling
  • progress shows something like 12% building modules 23/55 modules 32 active PATH
  • then, when webpack meets the first TypeScript file, the progress stops for some time (every time when I observe this waiting it was on file with *.ts extension - also profiling shows the same)
  • in our case after 1-2 minutes webpack continues process files and progress continue too
  • every time when webpack meet TypeScript file it blocks for some time again, but only the first block is such long (the second and further blocks takes much less, but still it is noticeable time)

@felixfbecker
Copy link

Yes, but isn't the point of thread-loader that it runs the whole loader (i.e. ts-loader) in a completely separate process? So how would ts-loader then still be able to block the main process?

Have you tried using thread-loader in your setup?

@timocov
Copy link
Contributor Author

timocov commented Sep 10, 2018

Hm, good point - I didn't think about it.

So how would ts-loader then still be able to block the main process?

I need to check it. Quite possible that it can help here. 👍

@timocov
Copy link
Contributor Author

timocov commented Sep 10, 2018

I just have played with thread-loader and here are results:

  • It seems that it does the same thing I suggest to do in ts-loader. Thanks @felixfbecker!
  • After compilation we have a lot of errors like Module build failed: TypeError: Cannot read property 'plugin' of undefined - I guess they are related to thread-loader's limitation Loaders cannot use custom loader API (i. e. by plugins).. @felixfbecker do you have such errors in your build?
  • Bundling time (even with errors) decreased down from 550s to 480s. <- @johnnyreilly

@felixfbecker
Copy link

No, we don't have these errors. Here are the options we use for ts-loader, maybe this helps?

loader: 'ts-loader',
                        options: {
                            compilerOptions: {
                                target: 'es6',
                                module: 'esnext',
                                noEmit: false,
                            },
                            experimentalWatchApi: true,
                            happyPackMode: true, // typecheck in fork-ts-checker-webpack-plugin for build perf
                        },

@timocov
Copy link
Contributor Author

timocov commented Sep 10, 2018

We use LoaderOptionsPlugin to pass options. But I believe that we need to update to latest webpack/ts-loader first...

@piecyk
Copy link

piecyk commented Sep 14, 2018

Using webpack4 + thread-loader in mixed js/ts project, and it's working quite nicely... Basic have
thread-loader, ts-loader, babel-loader, fork-ts-checker-webpack-plugin

Would recommend to have some custom code for configuring how many works use, interesting parts of the config...

module: {
  rules: [
    {
      ...commonRule,
      test: /\.tsx?$/,
      use: [
        tsWorkers > 0 && {
          loader: 'thread-loader',
          options: tsWorkerPool,
        },
        {
          loader: 'babel-loader',
          options: babelOptions({ hotMode, isDevelopmentMode }),
        },
        {
          loader: 'ts-loader',
          options: {
            happyPackMode: tsWorkers > 0,
            transpileOnly: tsTranspileOnly,
            compilerOptions: {
              sourceMap,
            },
          },
        },
      ].filter(Boolean),
    },
    {
      ...commonRule,
      test: /\.jsx?$/,
      use: [
        jsWorkers > 0 && {
          loader: 'thread-loader',
          options: jsWorkerPool,
        },
        {
          loader: 'babel-loader',
          options: babelOptions({ hotMode, isDevelopmentMode }),
        },
      ].filter(Boolean),
    },
    // ...
  ]
},
// fork-ts-checker-webpack-plugin in plugins 
plugins: [
  ...(tsTranspileOnly && forkTsWorkers > 0
    ? [
        new ForkTsCheckerWebpackPlugin({
          async: true,
          watch: [srcPath],
          workers: forkTsWorkers,
          checkSyntacticErrors: tsWorkers > 0,
        }),
      ]
    : []),
]

Workers pools

const commonModulesToWarmup = [
  'babel-loader',
  'babel-preset-env',
  'babel-preset-react',
  'babel-plugin-syntax-dynamic-import',
  'babel-plugin-syntax-object-rest-spread',
  'babel-plugin-transform-object-rest-spread',
]

const tsWorkerPool = {
  workers: tsWorkers,
  poolTimeout: watchMode || devServer ? Infinity : 2000,
}
if (tsWorkers > 0) {
  threadLoader.warmup(tsWorkerPool, [...commonModulesToWarmup, 'ts-loader'])
}
const jsWorkerPool = {
  workers: jsWorkers,
  poolTimeout: watchMode || devServer ? Infinity : 2000,
}
if (jsWorkers > 0) {
  threadLoader.warmup(jsWorkerPool, commonModulesToWarmup)
}

Running it with configuration of "tsWorkers":1,"jsWorkers":2,"forkTsWorkers":1 no blocking just building as mad 😄Plus thanks to hard-source-webpack-plugin running build from cache takes ~5s

@felixfbecker
Copy link

Can this be closed then? I don't think ts-loader should include this functionality if it can be achieved by composition of other loaders

@piecyk
Copy link

piecyk commented Sep 14, 2018

@felixfbecker Exactly 👍 There is no need for this logic in ts-loader

@timocov
Copy link
Contributor Author

timocov commented Sep 14, 2018

Actually there is one difference between using thread-loader and LanguageService in different thread - in case usage thread-loader the state between services is not shared and quite possible that some instances of ts-loader (and LanguageService) will make the same work independently.

@piecyk
Copy link

piecyk commented Sep 14, 2018

Yes that is true, just wondering in scenario that you use fork-ts-checker-webpack-plugin for type checking and thread-loader + ts-loader for transpileOnly will be it worth to run the LanguageService in other process 🤔

@timocov
Copy link
Contributor Author

timocov commented Sep 14, 2018

that you use fork-ts-checker-webpack-plugin for type checking

In some cases you just cannot use it - #825 (comment)

@piecyk
Copy link

piecyk commented Sep 14, 2018

Hmm what exactly this means ?

global const enums to inline some common constants

But yeah, then it make sens to run it in separate process... When running build without thread-loader & fork-ts-checker-webpack-plugin also gets blocked for few seconds

@timocov
Copy link
Contributor Author

timocov commented Sep 14, 2018

When running build without thread-loader & fork-ts-checker-webpack-plugin also gets blocked for few seconds

Could you please share a little bit about your project: how many files ts/js do you have, cloc value?

Hmm what exactly this means ?

Instead of declaring constants or importing const enums from module we just created local types, which contains global const enum declaration like this:

declare const enum KeyboardEventKeyCode {
	Backspace = 8,
	Tab = 9,
	Enter = 13,
	Shift = 16,
	Control = 17,
	Alt = 18,
	CapsLock = 20,
	Escape = 27,
	Space = 32,
	PageUp = 33,
	PageDown = 34,
	End = 35,
}

(this file is added to build the same as typing from @types folder)

And use it like this: if (e.keyCode === KeyboardEventKeyCode.End) {} where it needed in TypeScript code, and TypeScript will inline 35 instead of KeyboardEventKeyCode.End.

@stale
Copy link

stale bot commented Jan 19, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Jan 19, 2019
@stale
Copy link

stale bot commented Jan 26, 2019

Closing as stale. Please reopen if you'd like to work on this further.

@stale stale bot closed this as completed Jan 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants