Buffer the entire http header to one packet, and buffer other data in chunks of 4-16 kb.

Buffer the entire http header to one packet, and buffer other data in chunks of 4-16 kb.


BUG=https://code.google.com/p/dart/issues/detail?id=8871

Committed: https://code.google.com/p/dart/source/detail?r=19857

[Søren Gjesse, 2013-03-12 11:30:55]

lgtm

https://codereview.chromium.org/12655003/diff/3001/sdk/lib/io/http_impl.dart
File sdk/lib/io/http_impl.dart (right):

https://codereview.chromium.org/12655003/diff/3001/sdk/lib/io/http_impl.dart#...
sdk/lib/io/http_impl.dart:749: var buffer = new _BufferList();
I am not sure that using a _BufferList for the header this way is a good idea,
but it is easy to change later.

E.g. adding CR LF is now adding a full Queue entry in the _BufferList for two
bytes of output. One option for the header is to calculate its size and then
create and fill a UInt8List.

https://codereview.chromium.org/12655003/diff/3001/tests/standalone/io/http_1...
File tests/standalone/io/http_10_test.dart (left):

https://codereview.chromium.org/12655003/diff/3001/tests/standalone/io/http_1...
tests/standalone/io/http_10_test.dart:83: socket.write("GET /
HTTP/1.0\r\n\r\n");
Oops

https://codereview.chromium.org/12655003/diff/3001/tests/standalone/io/http_1...
File tests/standalone/io/http_10_test.dart (right):

https://codereview.chromium.org/12655003/diff/3001/tests/standalone/io/http_1...
tests/standalone/io/http_10_test.dart:95: Expect.equals(-1,
s.indexOf("keep-alive"));
Shouldn't we still check for 'z' as the last character?

[Anders Johnsen, 2013-03-12 12:11:30]

https://codereview.chromium.org/12655003/diff/3001/sdk/lib/io/http_impl.dart
File sdk/lib/io/http_impl.dart (right):

https://codereview.chromium.org/12655003/diff/3001/sdk/lib/io/http_impl.dart#...
sdk/lib/io/http_impl.dart:749: var buffer = new _BufferList();
On 2013/03/12 11:30:55, Søren Gjesse wrote:
> I am not sure that using a _BufferList for the header this way is a good idea,
> but it is easy to change later.
> 
> E.g. adding CR LF is now adding a full Queue entry in the _BufferList for two
> bytes of output. One option for the header is to calculate its size and then
> create and fill a UInt8List.

Yes, we have to look into this once we really dive into the performance story.

https://codereview.chromium.org/12655003/diff/3001/tests/standalone/io/http_1...
File tests/standalone/io/http_10_test.dart (left):

https://codereview.chromium.org/12655003/diff/3001/tests/standalone/io/http_1...
tests/standalone/io/http_10_test.dart:83: socket.write("GET /
HTTP/1.0\r\n\r\n");
On 2013/03/12 11:30:55, Søren Gjesse wrote:
> Oops

Hehe yeah, funny typo :)

https://codereview.chromium.org/12655003/diff/3001/tests/standalone/io/http_1...
File tests/standalone/io/http_10_test.dart (right):

https://codereview.chromium.org/12655003/diff/3001/tests/standalone/io/http_1...
tests/standalone/io/http_10_test.dart:95: Expect.equals(-1,
s.indexOf("keep-alive"));
On 2013/03/12 11:30:55, Søren Gjesse wrote:
> Shouldn't we still check for 'z' as the last character?

Ahh, you are right. Adding back.

[Anders Johnsen, 2013-03-12 12:13:46]

Committed patchset #4 manually as r19857 (presubmit successful).