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

RFC: Concurrency limit on zlib #1202

Closed
STRML opened this issue Sep 9, 2017 · 4 comments
Closed

RFC: Concurrency limit on zlib #1202

STRML opened this issue Sep 9, 2017 · 4 comments

Comments

@STRML
Copy link
Contributor

STRML commented Sep 9, 2017

Using permessage-deflate is difficult on long-running servers due to nodejs/node#8871 (comment).

I've spent some time with this subject and found that on Linux, whether or not Transparent Huge Pages is enabled, catastrophic memory fragmentation occurs and cannot be recovered from.

Unfortunately this is the case for any processes that queue up large numbers of jobs to zlib. The more concurrency you use, the more fragmentation.

I've measured this using the following script:

const zlib = require('zlib');
const Promise = require('bluebird');

const deflateAsync = Promise.promisify(zlib.deflate);
const concurrency = Number(process.argv[2]) || 1;
console.log('Running with concurrency', concurrency);

const payload = new Buffer(JSON.stringify({some: 'data'}));

const arr = [];
for(let i = 0; i < 30000; ++i){
  arr.push(payload);
}

console.time('Deflate')
Promise.map(arr, (item) => deflateAsync(item), {concurrency: concurrency})
.then(() => { 
  console.timeEnd('Deflate'); 
  console.log('Memory:', process.memoryUsage());
});

setTimeout(()=>{}, 2000000);

And here are the results:

# node indexBluebird.js 5
Running with concurrency 5
Deflate: 1474.900ms
Memory: { rss: 68628480,
  heapTotal: 27262976,
  heapUsed: 14881792,
  external: 13049916 }
# node indexBluebird.js 10
Running with concurrency 10
Deflate: 1436.186ms
Memory: { rss: 69718016,
  heapTotal: 27262976,
  heapUsed: 11312656,
  external: 5070908 }
# node indexBluebird.js 100
Running with concurrency 100
Deflate: 1464.645ms
Memory: { rss: 122105856,
  heapTotal: 44040192,
  heapUsed: 13695992,
  external: 12656700 }
# node indexBluebird.js 500
Running with concurrency 500
Deflate: 1698.649ms
Memory: { rss: 231297024,
  heapTotal: 47185920,
  heapUsed: 15657224,
  external: 23633980 }
# node indexBluebird.js 1000
Running with concurrency 1000
Deflate: 2193.191ms
Memory: { rss: 407408640,
  heapTotal: 64487424,
  heapUsed: 28238696,
  external: 244801596 }
# node indexBluebird.js Infinity
Running with concurrency Infinity
Deflate: 3337.215ms
Memory: { rss: 3140907008,
  heapTotal: 191889408,
  heapUsed: 156638008,
  external: 492249148 }

To summarize:

  • With only 5 concurrent calls to zlib.deflate() we see only 68 MB of RAM used.
  • With 1000, we see 407MB.
  • With no concurrency control at all, we see 3GB of RAM usage. This happens whether or not we use Promises, to be clear.

This manifests in horrible ways in production:

screen shot 2017-09-07 at 6 16 47 pm

The above corresponds to a load spike, which compressed a very large number of messages.


If 'ws' is open to it, I'd like to open a PR integrating or creating a plugin (suggestions?) that uses queue or a similar library to limit zlib concurrency to a reasonable value.

I've opened this issue first to track the problem and see if others have seen it, and to determine if such a PR would be accepted.

@lpinca
Copy link
Member

lpinca commented Sep 9, 2017

Refs: #804

@lpinca
Copy link
Member

lpinca commented Sep 9, 2017

I think the issue should be fixed upstream (Node.js core) using a different allocator but if we can work around the problem I'm open to it.

cc: @websockets/admin

@3rd-Eden
Copy link
Member

3rd-Eden commented Sep 9, 2017

If the origin of the problem comes from node, it would make sense to fix it there. But i'm not against landing a build-in queue in the mean time.

STRML added a commit to STRML/ws that referenced this issue Sep 11, 2017
STRML added a commit to STRML/ws that referenced this issue Sep 11, 2017
STRML added a commit to STRML/ws that referenced this issue Sep 11, 2017
@lpinca
Copy link
Member

lpinca commented Sep 15, 2017

Fixed by #1204.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants