Don't use ConsStringIterator to compute string hashes

Don't use ConsStringIterator to compute string hashes

R=yangguo@chromium.org

BUG=

[dcarney, 2014-11-26 10:14:17]

dcarney@chromium.org changed reviewers:
+ yangguo@chromium.org

[dcarney, 2014-11-26 10:15:52]

ptal - the absolute worst case for hashing a constring is only a 15ms or so, but
it's about 0 doing this

[Yang, 2014-11-26 10:46:09]

On 2014/11/26 10:15:52, dcarney wrote:
> ptal - the absolute worst case for hashing a constring is only a 15ms or so,
but
> it's about 0 doing this

How much is the performance penalty if we allocate on-demand? Keeping 16k per
isolate for this seems excessive.

[Yang, 2014-11-26 10:46:15]

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

https://codereview.chromium.org/762773002/diff/1/src/objects.cc#newcode9291
src/objects.cc:9291: String::WriteToFlat(cons_string, one_byte_buffer, 0,
length);
Wouldn't this cause OOB writes for large strings? I think what you want is
Min(length, String::kMaxHashCalcLength).

[dcarney, 2014-11-26 11:16:51]

On 2014/11/26 10:46:15, Yang wrote:
> https://codereview.chromium.org/762773002/diff/1/src/objects.cc
> File src/objects.cc (right):
> 
> https://codereview.chromium.org/762773002/diff/1/src/objects.cc#newcode9291
> src/objects.cc:9291: String::WriteToFlat(cons_string, one_byte_buffer, 0,
> length);
> Wouldn't this cause OOB writes for large strings? I think what you want is
> Min(length, String::kMaxHashCalcLength).


right... that got lost in a refactor somewhere

ptal now

[Yang, 2014-11-26 11:25:44]

On 2014/11/26 11:16:51, dcarney wrote:
> On 2014/11/26 10:46:15, Yang wrote:
> > https://codereview.chromium.org/762773002/diff/1/src/objects.cc
> > File src/objects.cc (right):
> > 
> > https://codereview.chromium.org/762773002/diff/1/src/objects.cc#newcode9291
> > src/objects.cc:9291: String::WriteToFlat(cons_string, one_byte_buffer, 0,
> > length);
> > Wouldn't this cause OOB writes for large strings? I think what you want is
> > Min(length, String::kMaxHashCalcLength).
> 
> 
> right... that got lost in a refactor somewhere
> 
> ptal now

lgtm.

[dcarney, 2014-11-26 11:25:57]

The CQ bit was checked by dcarney@chromium.org

[commit-bot: I haz the power, 2014-11-26 11:26:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/762773002/20001

[commit-bot: I haz the power, 2014-11-26 11:53:34]

Committed patchset #2 (id:20001)