stream: buffering and highWaterMark

[ronag]

The highWaterMark allows us to limit the buffering of a writable stream. This is great. If I set it to e.g. 16k I would expect the memory usage of the stream to be ~16kb. However this is not always the case, e.g:

const w = new Writable({
  highWaterMark: 16384,
  write (buf, encoding, cb) {
    process.nextTick(cb)
  }
})

while(w.write('a'))

assert.strictEqual(w.writableLength, 16384)

I would expect the memory usage of the stream to be ~16kB (+ some constant stream size). However, since I'm writing a small amount every time, the memory overhead of buffering takes over and I'm actually closer to ~1MB (I think the overhead of every buffered item is roughly 64 bytes). Maybe worse if a cb with unfortunate closure is passed.

If I setup a http server with a memory limit under the assumption of the highWaterMark and number of request with a reasonable extra. I can still end up significantly exceeding the memory limit depending on how other parts behave.

I'm not actually sure how to resolve this without breaking the public API, e..g we could change writableLength to something like:

state.length + state.bufferedRequestCount * BUFFER_OVERHEAD;

But if users assume that writableLength correlates to the number of written bytes (as my example above) we introduce breakage.

Or we could just update the state.length < state.highWaterMark check to take the overhead into account, but then we will maybe surprise the user by returning false in write() before writableLength introspection says it's necessary.

We have something possibly related with pendingcb as well. However, I need to think more about that.

[ronag]

ping @addaleax

[addaleax]

I think whatever we do might end up being a breaking change, but I’d prefer not to change the semantics of writableLength.

[ronag]

Another non-breaking solution could be to collapse/compact buffered items into a single item.

This would of course only work while the encoding is the same and no callback is provided. I'm a little unsure about the performance implication.

Maybe such compaction should occur when state.length + bufferedCount * 64 > state.highWaterMark. Effectively compacting the buffer when the overhead causes it to exceed the expected limit.

i.e. something like:

const kBufferedRequestSize = 64;

state.bufferedRequestCount += 1;
const overflow = state.length + 
	state.bufferedRequestCount * kBufferedRequestSize - 
	state.highWaterMark;
if (overflow > 0) {
  compactBuffer(state, overflow);
} 

[ronag]

The current buffering logic is a little hard to follow. I'll try to look into this and come with a proposal once/if #29026 is merged.

[ronag]

@mscdex: I think we might have a similar problem with Readable and BufferList where a lot of small items can make the memory usage significantly exceed what is expected. Maybe we need a compact for BufferList?