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

Recommendation for multi-module setup? Memory leak with 18+ Rollup watchers #177

Closed
marijnh opened this issue Oct 4, 2019 · 7 comments
Closed
Labels
help wanted kind: optimization Performance, space, size, etc improvement priority: backlog Low priority priority: in progress problem: stale Issue has not been responded to in some time scope: upstream Issue in upstream dependency scope: watch mode Related to Rollup's watch mode solution: workaround available There is a workaround available for this issue

Comments

@marijnh
Copy link
Contributor

marijnh commented Oct 4, 2019

I have a project with a bunch (18) separate (TS) modules, each of which is compiled down to its own .js file with rollup and this plugin. I'd like to set up a single rollup process to watch and recompile the lot of them, which I'm currently doing with a rollup config file that instantiates this plugin 18 times, each time with a different include setting, because otherwise the plugin will emit .d.ts files for all the files in each project's dist directory.

This regularly (but only after a bunch of recompile cycles) leads to node running out of memory:

==== JS stack trace =========================================

    0: ExitFrame [pc: 0x134e879]
Security context: 0x2ed275600919 <JSObject>
    1: getDeclarationName(aka getDeclarationName) [0x3270c86f3011] [/home/marijn/src/cm6/node_modules/typescript/lib/typescript.js:~27802] [pc=0x34f390cf2ea2](this=0x2b31f95404d1 <undefined>,0x2df9bfc13a39 <NodeObject map = 0x2513288541d9>)
    2: declareSymbol(aka declareSymbol) [0x3270c86f3091] [/home/marijn/src/cm6/node_modules/typescript/lib/typescript.js:~27...

FATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory

I guess that has something to do with the TypeScript compiler building up its type/code graphs multiple times, once for every instance of the plugin. Is that correct?

Is there a way to set up the plugin once, but use it to build several projects, each of which emits .d.ts files only for its own files?

Or is there maybe some other recommended way to approach a use case like this?

@ezolenko
Copy link
Owner

ezolenko commented Oct 7, 2019

You could try using single instance of typescript with multiple instances of the plugin. See typescript option:

const ts = require("typescript");
// ...
[
    rpt2_1 ( { typescript: ts } ),
]

// ...
[
    rpt2_2 ( { typescript: ts } ),
]

Let me know if that helps. If it doesn't consider making a repo with reproduction, I'll see if there is anything on the plugin end that can be doe for this scenario.

@ezolenko ezolenko added the kind: question This is a usage or similar question label Oct 7, 2019
@marijnh
Copy link
Contributor Author

marijnh commented Oct 8, 2019

Since require will return the already-loaded module when called multiple times on the same package, I'm not sure how requiring typescript in my rollup.config is any different from the require("typescript") that the plugin itself does.

You can set up the process I'm working with cloning https://github.com/codemirror/codemirror.next, and running npm install && npm watch in it. If you then touch, for example, view/src/index.ts a few times, you see the memory consumption of the node process grow every rebuild cycle, and then eventually the whole thing crashes. (Or at least, it does on my machine, after about 20 cycles.)

@ezolenko
Copy link
Owner

ezolenko commented Oct 8, 2019

Indeed.

Looks like there is a memory leak somewhere. Rollup had a leak in watch mode before, but it seems to be fixed. (rollup/rollup#883)

Each instance of the plugin will create it's own instance of typescript.LanguageService and will not reset it during watch mode. This is how IDEs are operating I think, so typescript itself shouldn't have obvious leaks, or somebody would notice.

You can try running plugin with clean: true, this disables cache (but I don't see what could be leaking there either).

@ezolenko ezolenko added kind: bug Something isn't working properly help wanted and removed kind: question This is a usage or similar question labels Oct 10, 2019
@zouhangwithsweet
Copy link

This problem is also bothering me. I have 60+ ui components to build. Actually, I just can build 10 components one time.

@zouhangwithsweet
Copy link

I find way to bypass the problem. Use child_process to build a small amount components, then kill this process. With a recursion call, everything is ok. :)

main.js

const buildChild = (a, b) => {
  const cp = require('child_process')
  const c1 = cp.spawn('node', ['./build/build.component.js', a, b])
  c1.stdout.on('data', function (data) {
    spinner.info(`${chalk.blue(data)}`)
  })

  c1.stderr.on('data', function (data) {
    spinner.warn(`${chalk.red(data)}`)
  })

  c1.on('close', function (code) {
    a += 5
    b += 5
    if (a > pkgs.length && b > pkgs.length) {
      spinner.succeed(`${chalk.green('Build done. Exit code ' + code)}`)
      return
    }
    buildChild(a, b)
  })
}

build.component.js

const runBuild = async () => {
  let index = 0
  const pkgs = await getPackages()
  const inputs = pkgs
    .map(pkg => pkg.name)
    .slice(process.argv[2], process.argv[3])

  build(inputs[index])

  async function build(name) {
    const inputOptions = {
      input: pathToEntry,
      plugins: [
        nodeResolve(),
        css(),
        vue({
          target: 'browser',
          css: false,
        }),
        typescript({
          tsconfigOverride: {
            compilerOptions: {
              declaration: false,
            },
            'exclude': [
              'node_modules',
              '__tests__',
            ],
          },
          abortOnError: false,
        }),
      ],
      external(id) {
        return /^vue/.test(id)
          || deps.some(k => new RegExp('^' + k).test(id))
      },
    }
    const outOptions = {
      format: 'es',
      file: [filename],
    }

    const bundle = await rollup.rollup(inputOptions)
    console.log(name, 'done')
    await bundle.write(outOptions)
    index++
    if (index < inputs.length) {
      await build(inputs[index])
    } else {
      console.log('batch done')
    }
  }
}

runBuild()

@agilgur5 agilgur5 changed the title Recommendation for multi-module setup? Recommendation for multi-module setup? Memory leak with 18+ Rollup warchers Apr 30, 2022
@agilgur5 agilgur5 changed the title Recommendation for multi-module setup? Memory leak with 18+ Rollup warchers Recommendation for multi-module setup? Memory leak with 18+ Rollup watchers Apr 30, 2022
@agilgur5 agilgur5 added scope: watch mode Related to Rollup's watch mode solution: workaround available There is a workaround available for this issue kind: optimization Performance, space, size, etc improvement problem: stale Issue has not been responded to in some time solution: out-of-scope This is out of scope for this project scope: upstream Issue in upstream dependency and removed kind: bug Something isn't working properly help wanted labels Jun 9, 2022
@agilgur5 agilgur5 added help wanted priority: backlog Low priority and removed solution: out-of-scope This is out of scope for this project labels Jun 17, 2022
@agilgur5
Copy link
Collaborator

agilgur5 commented Jun 17, 2022

So I'm not sure if there is anything we can really do within rpt2 to resolve this.

Each instance of the plugin will create it's own instance of typescript.LanguageService and will not reset it during watch mode. This is how IDEs are operating I think, so typescript itself shouldn't have obvious leaks, or somebody would notice.

The OOM error is also coming from within TS itself, so I'm not sure that it's a memory leak in rpt2 either, if a memory leak at all. It also may have been resolved in newer versions of TS.

Really the only thing I could think that we could do with rpt2 is share a LanguageService between instances of the plugin. But that is pretty hacky in and of itself, as it really should have its own instance, as it should be one per Rollup project. Its setting are also configured based on your tsconfig, options, etc.
As ezolenko said too, I believe that IDEs have multiple instances of it too, but I suppose one could look through OSS VSCode and check that statement and see if there are any optimizations therein.

Being able to share a LanguageService would also add a good bit of complexity to the codebase, so if that's possible to do, it may also not be a worthwhile endeavor, especially as this issue hasn't gotten much attention.

As this is high complexity, for a fairly specific use-case, stale, and has a workaround above, I'm going to close this issue for now. If someone is interested in investigating such a feature, we can potentially re-open then.

@agilgur5
Copy link
Collaborator

I implemented one optimization for watch mode that relates to this in #388 which was released in 0.33.0. Hoping that might resolve the OOM error

Between #388 and my above comment, there's really only one more possible optimization for this: avoiding recreating the LanguageService if the config has not changed between watch cycles (i.e. if only the source files were changed and not tsconfig etc). That's the last thing I can think of, and that only applies under certain circumstances.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted kind: optimization Performance, space, size, etc improvement priority: backlog Low priority priority: in progress problem: stale Issue has not been responded to in some time scope: upstream Issue in upstream dependency scope: watch mode Related to Rollup's watch mode solution: workaround available There is a workaround available for this issue
Projects
None yet
Development

No branches or pull requests

4 participants