Make non-copying version of BytesBuidler and make file-reads faster.

Make non-copying version of BytesBuidler and make file-reads faster.

The non-copying version is internally, as this is always the behaviour
we want, when receiving data from the system.

File-reads are also further improved, by only reading data in chunks, if
initial length is 0. This is a special case where the file can be a
character device. Overall improvement is around a factor of 6.

BUG=
R=sgjesse@google.com

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

[Anders Johnsen, 2014-04-01 08:36:37]

Now fix a better readAsBytes version (no long future chain on completion).

[Søren Gjesse, 2014-04-01 08:58:39]

https://codereview.chromium.org/218493012/diff/20001/sdk/lib/io/bytes_builder...
File sdk/lib/io/bytes_builder.dart (right):

https://codereview.chromium.org/218493012/diff/20001/sdk/lib/io/bytes_builder...
sdk/lib/io/bytes_builder.dart:12: * buffer. Note that it's optimized for IO,
using an initial buffer of 1K bytes.
I am not sure the "optimized of IO" is actually true. And the mentioning of the
1K is irrelevant. If we where to talk about initial size it should be an
optional argument to the constructor.

https://codereview.chromium.org/218493012/diff/20001/sdk/lib/io/bytes_builder...
sdk/lib/io/bytes_builder.dart:19: * `true`.
I think some more documentation on what happens if the non-copied data is
changed after it is added. Maybe the message should just be that that behavior
is undefined.

https://codereview.chromium.org/218493012/diff/20001/sdk/lib/io/bytes_builder...
sdk/lib/io/bytes_builder.dart:21: factory BytesBuilder({bool copy: true}) {
The argument name 'copy' is not totally correct. It might will end up copying
the data, and in most cases it will.

How about adding a separate named constructor instead with a more precise name
and a separate comment.

BytesBuilder.cacheBuffers
BytesBuilder.ownBuffers

https://codereview.chromium.org/218493012/diff/20001/sdk/lib/io/bytes_builder...
sdk/lib/io/bytes_builder.dart:158: if (bytes is! Uint8List) {
This might be surprising, as for the non-copying we sometimes copy anyway. If
the user think that whatever is added can be changed until takeBytes is called
this might not always be the case. We could just postpone this copy to takeBytes
- if we even need to copy.

[Anders Johnsen, 2014-04-01 09:29:07]

https://codereview.chromium.org/218493012/diff/20001/sdk/lib/io/bytes_builder...
File sdk/lib/io/bytes_builder.dart (right):

https://codereview.chromium.org/218493012/diff/20001/sdk/lib/io/bytes_builder...
sdk/lib/io/bytes_builder.dart:12: * buffer. Note that it's optimized for IO,
using an initial buffer of 1K bytes.
On 2014/04/01 08:58:40, Søren Gjesse wrote:
> I am not sure the "optimized of IO" is actually true. And the mentioning of
the
> 1K is irrelevant. If we where to talk about initial size it should be an
> optional argument to the constructor.

Removed.

https://codereview.chromium.org/218493012/diff/20001/sdk/lib/io/bytes_builder...
sdk/lib/io/bytes_builder.dart:19: * `true`.
On 2014/04/01 08:58:40, Søren Gjesse wrote:
> I think some more documentation on what happens if the non-copied data is
> changed after it is added. Maybe the message should just be that that behavior
> is undefined.

Done.

https://codereview.chromium.org/218493012/diff/20001/sdk/lib/io/bytes_builder...
sdk/lib/io/bytes_builder.dart:21: factory BytesBuilder({bool copy: true}) {
On 2014/04/01 08:58:40, Søren Gjesse wrote:
> The argument name 'copy' is not totally correct. It might will end up copying
> the data, and in most cases it will.
> 
> How about adding a separate named constructor instead with a more precise name
> and a separate comment.
> 
> BytesBuilder.cacheBuffers
> BytesBuilder.ownBuffers

I think cacheBuffers and ownBuffers is confusing. Also, we can not change the
default constructor.

We could add one of

- BytesBuilder.nonCopying
- BytesBuilder.lazyCopy

or something, but I'm not sure I see the gain. I think 'copy' captures the idea
very precise, even though exceptions exists. It's more of a hint than a rule,
that it may choose to not copy.

https://codereview.chromium.org/218493012/diff/20001/sdk/lib/io/bytes_builder...
sdk/lib/io/bytes_builder.dart:158: if (bytes is! Uint8List) {
On 2014/04/01 08:58:40, Søren Gjesse wrote:
> This might be surprising, as for the non-copying we sometimes copy anyway. If
> the user think that whatever is added can be changed until takeBytes is called
> this might not always be the case. We could just postpone this copy to
takeBytes
> - if we even need to copy.

We do say that there is an internal list and promise that as a result. It should
not be a surprise that if you add anything else than typed-data, we'll have to
copy. I have changed it to say it will copy if needed.

[Søren Gjesse, 2014-04-01 10:29:29]

lgtm

[Anders Johnsen, 2014-04-01 10:56:31]

Committed patchset #3 manually as r34589 (presubmit successful).