Bluebird: Memory leaking with Promise.map concurrency

[samclearman]

1) What version of bluebird is the issue happening on?
Bluebird 3.5.20 with bluebird-global 3.5.5

2) What platform and version? (For example Node.js 0.12 or Google Chrome 32)
Node.js 8.9.

3) Did this issue happen with earlier version of bluebird?
As far as I know

I have code that looks like this:

const aBunchOfIds = [ /* a few hundred thousand elements */ ]
Promise.map(aBunchOfIds, async id => {
    const anObj = await getFromDatastore(id);
    const anotherObj = await process(anObj);
    if (isSpecial(anotherObj)) { console.log('so special'); }
}, { concurrency: 1000 });

This dies because node runs out of heap space. However the following modification resolves the issue:

const aBunchOfIds = [ /* a few hundred thousand elements */ ]
for (let i = 0; i < aBunchOfIds.length; i += 10000) {
    const someIds = aBunchOfIds.slice(i, i + 10000);
    Promise.map(someIds, async id => {
        const anObj = await getFromDatastore(id);
        const anotherObj = await process(anObj);
        if (isSpecial(anotherObj)) { console.log('so special'); }
    }, { concurrency: 1000 });
}

So it seems that Promise.map doesn't free the memory used by its promises until all promises have returned, even when a concurrency is specified.

[benjamingr]

So it seems that Promise.map doesn't free the memory used by its promises until all promises have returned, even when a concurrency is specified.

That's the queue shrinking and growing - it's a static overhead and not nwe :)

If you run this multiple times it won't grow again - it's just reserving memory in the background so future executions are allocation free.

[samclearman]

Thanks!

Just to make sure there isn't a miscommunication: the intended behavior is for Promise.map(x, f, { concurrency: n }) to require x.length * [memory usage of f] memory (as opposed to n * [memory usage of f])?

[benjamingr]

I'll test it and make sure this is what we do. I thought it's just the queue shrinking - if you think it takes x.length * [memory usage of f] rather than x.length * promise_size then it should not do that.

[samclearman]

I guess it's possible that it's just 100000 * promise_size that is blowing up my heap but that doesn't seem right to me, I suspect that the memory allocated in the functions that promises call isn't getting garbage collected until the whole thing finishes?

[spion]

Seems to me that as long as memory usage is contained within the callback it gets GCed correctly: https://jsfiddle.net/w03ykabx/3/

[krunkosaurus]

Just a heads up that I got "Maximum call stack size exceeded" errors until I finally downgraded this package to 3.5.2

I think it could be related to this issue.

[gebrits]

I'm seeing memory-leak issues that are very similar, if not the same.

Came up with the following test-case which shows the issue clearly:

• running below code as-is will result in OOM in a couple of seconds
• running code with then() added (see below) will allow GC to free up mem.

Lodash 3.5.2

/*
 Below gives an OOM in a couple of seconds. 
 Adding the commented-out then() solves it. Why?
 */
const Promise = require("bluebird");
const obj = {a: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"};
return Promise.map(new Array(100000), () => {
  return Promise.resolve()
  .then(() => new Array(10000).map(() => Object.assign({},obj))) //consume some mem
  // .then(() => { //passing in an argument here, makes the OOM-issue reappear
  //   //adding this then(), will allow objects to be GC'ed, thus solving OOM-issue. WHY?
  // });
}, { concurrency: 10 })

It seems that since the result isn't passed in as argument to the last then() the object can be freed from mem. This seems to suggest that not only the promises themselves but all objects created inside those promises are kept locked in mem until the entire Promise.map is finished.

[samclearman]

Nice repro!

[shivaalroy]

@samclearman I believe I've found an easy fix for this:

repro code:

import 'bluebird-global';

const ids = fnThatGetsIds;
let done = 0;
await Promise.map(ids, async id => {
  const record = await getLargeRecord(id);
  if (done % 100 === 0) {
    console.log(done, new Date().getTime() - time);
    time = new Date().getTime();
  }
  done++;
  // starts slowing down after (~2mins)
}, { concurrency: 100 });

This slows down after 2 minutes.

Adding return Promise.resolve(); fixes the issue, and there's no slowdown.

import 'bluebird-global';

const ids = fnThatGetsIds;
let done = 0;
await Promise.map(ids, async id => {
  const record = await getLargeRecord(id);
  if (done % 100 === 0) {
    console.log(done, new Date().getTime() - time);
    time = new Date().getTime();
  }
  done++;
  return Promise.resolve(); // doesn't slow down
}, { concurrency: 100 });

Removing the return causes a slow down, and return null; also causes a slow down.

[samclearman]

Thanks @shivaalroy . Looks like that agrees with what @gebrits was seeing... @benjamingr, does this look like intended behavior to you or not?

[benjamingr]

That... makes no sense 😅

[w9jds]

I am seeing similar behavior as well. I have a Kubernetes service running with a map with concurrency running every ~6-10 seconds. After a couple hours the memory heap doubles (rinse repeat). Eventually it causes Kubernetes to try and boot up more containers and evicts all of them for lack of memory. When I remove the map command, and loop through them myself with Promise.all the memory issue goes away.

[petkaantonov]

I'm not sure what the problem is. .map will store the result of the mapping function until the entire map has run. If this result takes a lot of memory then you will run out of memory if you cannot hold all the results. If you don't need to store the result to use it from the final mapped array then don't return it from the mapping function.

The examples using async are interesting because they don't seem to return any memory consuming item. I'd need full code to investigate. Using code like this memory stays stable, it is going up and down and not constantly increasing:

Promise.map(new Array(100000), async function() {
   await Promise.delay(1)
}, { concurrency: 10 })

setInterval(function() {
    var used = process.memoryUsage().heapUsed / 1024 / 1024;
    console.log(`The script uses approximately ${Math.round(used * 100) / 100} MB`);
}, 20);

[benjamingr]

@petkaantonov then why would this leak memory?

import 'bluebird-global';

const ids = fnThatGetsIds();
let done = 0;
await Promise.map(ids, async id => {
  const record = await getLargeRecord(id);
  if (done % 100 === 0) {
    console.log(done, new Date().getTime() - time);
    time = new Date().getTime();
  }
  done++;
  // starts slowing down after (~2mins)
}, { concurrency: 100 });