Streamline the scanner for external two byte string input.

Streamline the scanner for external two byte string input.


Committed: http://code.google.com/p/v8/source/detail?r=2703

[Kasper Lund, 2009-08-18 06:49:41]

I'm okay with change as a first step - but I think we should consider changing
the internal format of the CharacterStream to TwoByte instead of having two
separate UTF16Buffers and virtual calls for each character.

I'm looking forward to seeing the impact on page cyclers, but I'm (ever so
slightly) worried that the performance of scanning of our builtin JS code will
be negatively impacted (because of the virtual Advance calls) - so it might be
good to measure that too.

LGTM.

http://codereview.chromium.org/165403/diff/1004/1005
File src/api.cc (right):

http://codereview.chromium.org/165403/diff/1004/1005#newcode1049
Line 1049: return i::PreParse(i::Handle<i::String>(), &buf, NULL);
i::Handle<i::String>::null() to make it more explicit?

http://codereview.chromium.org/165403/diff/1004/1007
File src/scanner.cc (right):

http://codereview.chromium.org/165403/diff/1004/1007#newcode95
Line 95: : pos_(0), size_(0) { }
4 space indent.

http://codereview.chromium.org/165403/diff/1004/1007#newcode105
Line 105: : pushback_buffer_(0), last_(0), stream_(NULL) { }
4 space indent.

http://codereview.chromium.org/165403/diff/1004/1007#newcode157
Line 157: : raw_data_(NULL) { }
4 space indent.

http://codereview.chromium.org/165403/diff/1004/1007#newcode162
Line 162: ASSERT(!data.is_null() && StringShape(*data).IsExternalTwoByte());
StringShape(*data).IsExternalTwoByte() -> data->IsExternalTwoByteString()

http://codereview.chromium.org/165403/diff/1004/1007#newcode176
Line 176: // note: currently the following increment is necessary to avoid a
note -> Note

http://codereview.chromium.org/165403/diff/1004/1007#newcode206
Line 206: if (!source.is_null() && StringShape(*source).IsExternalTwoByte()) {
StringShape(*source).IsExternalTwoByte() -> source->IsExternalTwoByteString()?

http://codereview.chromium.org/165403/diff/1004/1008
File src/scanner.h (right):

http://codereview.chromium.org/165403/diff/1004/1008#newcode78
Line 78: virtual void PushBack(uc32 ch) = 0;
Ideally, these functions could be made non-virtuals again if we change the
internal format of the CharacterStream to be two byte encodings.

[Feng Qian, 2009-08-18 07:14:10]

http://codereview.chromium.org/165403/diff/1004/1005
File src/api.cc (right):

http://codereview.chromium.org/165403/diff/1004/1005#newcode1049
Line 1049: return i::PreParse(i::Handle<i::String>(), &buf, NULL);
On 2009/08/18 06:49:41, Kasper Lund wrote:
> i::Handle<i::String>::null() to make it more explicit?

Ah, didn't know this trick, but it is a bit confusing that one may think
i::Handle<i::String>::null() is a null string.

http://codereview.chromium.org/165403/diff/1004/1007
File src/scanner.cc (right):

http://codereview.chromium.org/165403/diff/1004/1007#newcode95
Line 95: : pos_(0), size_(0) { }
On 2009/08/18 06:49:41, Kasper Lund wrote:
> 4 space indent.

Done.

http://codereview.chromium.org/165403/diff/1004/1007#newcode95
Line 95: : pos_(0), size_(0) { }
On 2009/08/18 06:49:41, Kasper Lund wrote:
> 4 space indent.

Done.

http://codereview.chromium.org/165403/diff/1004/1007#newcode105
Line 105: : pushback_buffer_(0), last_(0), stream_(NULL) { }
On 2009/08/18 06:49:41, Kasper Lund wrote:
> 4 space indent.

Done.

http://codereview.chromium.org/165403/diff/1004/1007#newcode105
Line 105: : pushback_buffer_(0), last_(0), stream_(NULL) { }
On 2009/08/18 06:49:41, Kasper Lund wrote:
> 4 space indent.

Done.

http://codereview.chromium.org/165403/diff/1004/1007#newcode157
Line 157: : raw_data_(NULL) { }
On 2009/08/18 06:49:41, Kasper Lund wrote:
> 4 space indent.

Done.

http://codereview.chromium.org/165403/diff/1004/1007#newcode162
Line 162: ASSERT(!data.is_null() && StringShape(*data).IsExternalTwoByte());
On 2009/08/18 06:49:41, Kasper Lund wrote:
> StringShape(*data).IsExternalTwoByte() -> data->IsExternalTwoByteString()

IsExternalTwoByte check is unnecessary here, removed.

http://codereview.chromium.org/165403/diff/1004/1007#newcode176
Line 176: // note: currently the following increment is necessary to avoid a
On 2009/08/18 06:49:41, Kasper Lund wrote:
> note -> Note

Done.

http://codereview.chromium.org/165403/diff/1004/1007#newcode206
Line 206: if (!source.is_null() && StringShape(*source).IsExternalTwoByte()) {
On 2009/08/18 06:49:41, Kasper Lund wrote:
> StringShape(*source).IsExternalTwoByte() -> source->IsExternalTwoByteString()?

IsExternalTwoByteString is only implemented in #if DEBUG, and comments in
objects-inl.h states that one should consider use StringShape instead of moving
the implementation out of #if DEBUG

http://codereview.chromium.org/165403/diff/1004/1008
File src/scanner.h (right):

http://codereview.chromium.org/165403/diff/1004/1008#newcode78
Line 78: virtual void PushBack(uc32 ch) = 0;
Let's do it later.

On 2009/08/18 06:49:41, Kasper Lund wrote:
> Ideally, these functions could be made non-virtuals again if we change the
> internal format of the CharacterStream to be two byte encodings.