Merge HTTP chunks during chunked encoding

[ronag]

Given the code below, we cork the socket, but still, we always add chunked encoding data to each chunk buffered instead of just the actual buffer sent during uncork.

We should move the chunked encoding data insertion to connectionCorkNT somehow and avoid a lot of unnecessary overhead.

What happens today is something like:

socket.cork()
writeLen(chunk1.length)
writeBody(chunk1)
writeLen(chunk2.length)
writeBody(chunk2)
socket.uncork()
// [len1, chunk1, len1, chunk1]

What we would like to achieve is:

// [len1 + len2, chunk1, chunk1]

We can achieve this by not writing the len before each chunk while corked. Instead, we can unshift the sum of all buffered chunks before we uncork.

// _http_outgoing.js
function write_(msg, chunk, encoding, callback, fromEnd) {
  if (typeof callback !== 'function')
    callback = nop;

  let len;
  if (chunk === null) {
    throw new ERR_STREAM_NULL_VALUES();
  } else if (typeof chunk === 'string') {
    len = Buffer.byteLength(chunk, encoding);
  } else if (isUint8Array(chunk)) {
    len = chunk.length;
  } else {
    throw new ERR_INVALID_ARG_TYPE(
      'chunk', ['string', 'Buffer', 'Uint8Array'], chunk);
  }

  let err;
  if (msg.finished) {
    err = new ERR_STREAM_WRITE_AFTER_END();
  } else if (msg.destroyed) {
    err = new ERR_STREAM_DESTROYED('write');
  }

  if (err) {
    if (!msg.destroyed) {
      onError(msg, err, callback);
    } else {
      process.nextTick(callback, err);
    }
    return false;
  }

  if (!msg._header) {
    if (fromEnd) {
      msg._contentLength = len;
    }
    msg._implicitHeader();
  }

  if (!msg._hasBody) {
    debug('This type of response MUST NOT have a body. ' +
          'Ignoring write() calls.');
    process.nextTick(callback);
    return true;
  }

  if (!fromEnd && msg.socket && !msg.socket.writableCorked) {
    msg.socket.cork();
    process.nextTick(connectionCorkNT, msg.socket);
  }

  let ret;
  if (msg.chunkedEncoding && chunk.length !== 0) {
    msg._send(NumberPrototypeToString(len, 16), 'latin1', null);
    msg._send(crlf_buf, null, null);
    msg._send(chunk, encoding, null);
    ret = msg._send(crlf_buf, null, callback);
  } else {
    ret = msg._send(chunk, encoding, callback);
  }

  debug('write ret = ' + ret);
  return ret;
}


function connectionCorkNT(conn) {
  conn.uncork();
}

[ronag]

@mcollina

[mertcanaltin]

@mcollina @targos
hi, i tried to change this, do you think it is a correct method? nodejs/node#47575

[gjgmenendez]

@ronag
So, yeah.. I've tried to understand the issue and I've the beginning of a solution. It seems to work if the response.write() method is only called once. I've got: "Malformed encoding found in chunked-encoding", when I try with several response.write() calls.
I've been trying to understand where the error is generated to see if I can modify the method that expect the buffered data in the [len1, chunk1, len1, chunk1] format.
Furthermore, I used the Array.splice() method before uncorking instead Array.unshift(). The unshift method puts the new len chunk before the header chunk...
I've also made a draft PR (from my fork), but I don't really know how I can link to it...
I'm feeling like a noob in so many aspects..

[ronag]

You could try to skip the socket cork/uncork and instead have your own buffer for the chunks that you then write in connectionCorkNT.

[gjgmenendez]

@ronag
This is my second take on the issue, I think I've got it more right this time (when I actually merge the chunks into a buffert). I tried to make it work in the connectionCorkNT but I couldn't manage to work so I ended using the end() instead...

I don't know if my last PR was incorrect in someway but I couldn't find it.

https://github.com/gjgmenendez/node/tree/issue%2357

[ronag]

@gjgmenendez I think your link is incorrect. I assume you wanted to link to your PR?