Improve SeqString::Truncate for latest allocated strings.

Improve SeqString::Truncate for latest allocated strings.


R=hpayer@chromium.org
BUG=

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

[Hannes Payer (out of office), 2013-03-21 12:48:29]

https://codereview.chromium.org/12440061/diff/1/src/json-parser.h
File src/json-parser.h (right):

https://codereview.chromium.org/12440061/diff/1/src/json-parser.h#newcode588
src/json-parser.h:588: // Shrink seq_string length to count and return.
See comment in src/objects.h. I would let Truncate return the right string
object, i.e. the empty string in the new_length==0 and the original string
object otherwise.

https://codereview.chromium.org/12440061/diff/1/src/json-stringifier.h
File src/json-stringifier.h (right):

https://codereview.chromium.org/12440061/diff/1/src/json-stringifier.h#newcod...
src/json-stringifier.h:693: if (current_index_ == 0) {
Again, Truncate() should do the right thing for you.

https://codereview.chromium.org/12440061/diff/1/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/12440061/diff/1/src/objects.cc#newcode7630
src/objects.cc:7630: ASSERT_LT(0, new_length);
new_length = 0 should be possible

https://codereview.chromium.org/12440061/diff/1/src/objects.cc#newcode7655
src/objects.cc:7655: heap->new_space()->allocation_info_.top = start_of_string +
new_size;
I think you also want to adjust MemoryChunk::IncrementLiveBytesFromMutator, as
it was before.

https://codereview.chromium.org/12440061/diff/1/src/objects.cc#newcode7659
src/objects.cc:7659: heap->CreateFillerObjectAt(start_of_string + new_size,
delta);
Do we also have to adjust  MemoryChunk::IncrementLiveBytesFromMutator here?

https://codereview.chromium.org/12440061/diff/1/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/12440061/diff/1/src/objects.h#newcode7756
src/objects.h:7756: // truncating the original string.
I would revert this change and truncate all strings >= length 0 and return in
the new_length==0 case the empty string.

https://codereview.chromium.org/12440061/diff/1/src/runtime.cc
File src/runtime.cc (right):

https://codereview.chromium.org/12440061/diff/1/src/runtime.cc#newcode5143
src/runtime.cc:5143: if (new_length == 0) return
isolate->heap()->empty_string();
Again, this could be handled in Truncate().

[Yang, 2013-03-21 14:18:34]

Updated the CL according to comments. Please take a second look.

https://codereview.chromium.org/12440061/diff/1/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/12440061/diff/1/src/objects.cc#newcode7655
src/objects.cc:7655: heap->new_space()->allocation_info_.top = start_of_string +
new_size;
On 2013/03/21 12:48:30, Hannes Payer wrote:
> I think you also want to adjust MemoryChunk::IncrementLiveBytesFromMutator, as
> it was before.

I'm doing this summarily below.

https://codereview.chromium.org/12440061/diff/1/src/objects.cc#newcode7659
src/objects.cc:7659: heap->CreateFillerObjectAt(start_of_string + new_size,
delta);
On 2013/03/21 12:48:30, Hannes Payer wrote:
> Do we also have to adjust  MemoryChunk::IncrementLiveBytesFromMutator here?

I'm doing this summarily below.

[Hannes Payer (out of office), 2013-03-22 08:12:48]

LGTM, with one final nit

https://codereview.chromium.org/12440061/diff/6001/src/json-parser.h
File src/json-parser.h (right):

https://codereview.chromium.org/12440061/diff/6001/src/json-parser.h#newcode589
src/json-parser.h:589: return SeqString::Truncate(seq_str, count);
Either change the variable name to seq_string or change the comment to seq_str.
I would prefer changing the variable name, according to the coding standard.

[Yang, 2013-03-28 11:19:49]

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