Make block writes in the serializer more efficient.

Make block writes in the serializer more efficient.

And also fix a OOB read in SerializeExternalString.

R=vogelheim@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=24858

[Yang, 2014-10-23 15:51:40]

note to self. use Lis::Resize and memcpy instead of List::AddAll

[vogelheim, 2014-10-23 16:48:32]

On 2014/10/23 15:51:40, Yang wrote:
> note to self. use Lis::Resize and memcpy instead of List::AddAll

Maybe do that inside List::AddAll? If this is a benefit for our case, it's
likely a benefit for all callers.

[vogelheim, 2014-10-23 16:48:45]

lgtm

https://codereview.chromium.org/671633004/diff/1/src/serialize.cc
File src/serialize.cc (right):

https://codereview.chromium.org/671633004/diff/1/src/serialize.cc#newcode1635
src/serialize.cc:1635: for (int i = 0; i < padding_size; i++)
sink_->PutSection(0, "StringPadding");
If I get this correctly, this makes sure that you can use PutRaw (rather than
many 1-byte PutSection) but maintain correct padding. An alternative might be to
add SnapshotByteSink::PutRawPadded, which hides this and returns the actual
number of bytes written. Then you could just call PutRawPadded, and DCHECK
whether the return value matches bytes_to_output.

As usual... you decide. :)

https://codereview.chromium.org/671633004/diff/1/test/cctest/test-serialize.cc
File test/cctest/test-serialize.cc (right):

https://codereview.chromium.org/671633004/diff/1/test/cctest/test-serialize.c...
test/cctest/test-serialize.cc:1057: STATIC_CHAR_VECTOR(""), 999999);
What's the significance of this change? Is this meant to trigger the padding
logic, since this string would now require padding?

[Yang, 2014-10-24 07:14:24]

https://codereview.chromium.org/671633004/diff/1/src/serialize.cc
File src/serialize.cc (right):

https://codereview.chromium.org/671633004/diff/1/src/serialize.cc#newcode1635
src/serialize.cc:1635: for (int i = 0; i < padding_size; i++)
sink_->PutSection(0, "StringPadding");
On 2014/10/23 16:48:44, vogelheim wrote:
> If I get this correctly, this makes sure that you can use PutRaw (rather than
> many 1-byte PutSection) but maintain correct padding. An alternative might be
to
> add SnapshotByteSink::PutRawPadded, which hides this and returns the actual
> number of bytes written. Then you could just call PutRawPadded, and DCHECK
> whether the return value matches bytes_to_output.
> 
> As usual... you decide. :)

This is a single use case, so I'd leave it at this.

https://codereview.chromium.org/671633004/diff/1/test/cctest/test-serialize.cc
File test/cctest/test-serialize.cc (right):

https://codereview.chromium.org/671633004/diff/1/test/cctest/test-serialize.c...
test/cctest/test-serialize.cc:1057: STATIC_CHAR_VECTOR(""), 999999);
On 2014/10/23 16:48:44, vogelheim wrote:
> What's the significance of this change? Is this meant to trigger the padding
> logic, since this string would now require padding?

yes.

[Yang, 2014-10-24 07:29:48]

Committed patchset #1 (id:1) manually as 24858 (presubmit successful).