Introduce short external strings without pointer cache.

Introduce short external strings without pointer cache.

BUG=
TEST=

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

[Yang, 2011-11-22 16:30:45]

Please take a look at this before any of my other pending CLs as they will have
to be changed after this one lands.

- Short external strings behave the same way as the old implementation of
external string does (regarding size and handling)
- External*String::GetChars() uses resource()->data() for the sake of simplicity
(no check for shortness)
- Changes to test cases merely revert them to pre-r10023.
- Data pointer caches are now updated when the resource is set, so we don't
expect any pointer caches that are not yet set.

[Lasse Reichstein, 2011-11-23 08:43:20]

LGTM

http://codereview.chromium.org/8635011/diff/1/src/api.cc
File src/api.cc (right):

http://codereview.chromium.org/8635011/diff/1/src/api.cc#newcode4563
src/api.cc:4563: if (size < i::ExternalString::kShortSize)
Put braces around the then-statement.

http://codereview.chromium.org/8635011/diff/1/src/ia32/codegen-ia32.cc
File src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/8635011/diff/1/src/ia32/codegen-ia32.cc#newcod...
src/ia32/codegen-ia32.cc:535: __ mov(result, FieldOperand(string,
HeapObject::kMapOffset));
We're absolutely sure that only strings reach here, right?

http://codereview.chromium.org/8635011/diff/1/src/ia32/codegen-ia32.cc#newcod...
src/ia32/codegen-ia32.cc:571: __ test_b(scratch, kShortExternalStringTag);
Test with kShortExternalStringMask instead of ...Tag.

http://codereview.chromium.org/8635011/diff/1/src/objects.cc
File src/objects.cc (right):

http://codereview.chromium.org/8635011/diff/1/src/objects.cc#newcode958
src/objects.cc:958: ASSERT(size >= ExternalString::kShortSize);
This assert can safely be dropped.

http://codereview.chromium.org/8635011/diff/1/src/objects.cc#newcode983
src/objects.cc:983: ExternalTwoByteString::kHashFieldOffset);
These two asserts are unnecessary, since ExternalAsciiString doesn't have a
static kLengthOffset or kHashFieldOffset property. This is comparing
String::kLengthOffset with itself. Since it's placed on String, it should be
assumed to apply to all string subtypes.

http://codereview.chromium.org/8635011/diff/1/src/objects.cc#newcode1013
src/objects.cc:1013: ASSERT(size >= ExternalString::kShortSize);
Ditto.

http://codereview.chromium.org/8635011/diff/1/src/objects.cc#newcode1029
src/objects.cc:1029: ExternalAsciiString::kHashFieldOffset);
As above.

http://codereview.chromium.org/8635011/diff/1/src/objects.cc#newcode1030
src/objects.cc:1030: if (is_symbol) self->Hash();  // Force regeneration of the
hash value.
Is there an explanation of how the hash can possibly change?
It should be determined solely by the character codes and length of the string,
both of which are retained by externalization.

http://codereview.chromium.org/8635011/diff/1/src/objects.h
File src/objects.h (right):

http://codereview.chromium.org/8635011/diff/1/src/objects.h#newcode234
src/objects.h:234: V(EXTERNAL_ASCII_SYMBOL_TYPE)                                
               \
No SHORT_EXTERNAL_SYMBOL_TYPE etc.?

http://codereview.chromium.org/8635011/diff/1/src/objects.h#newcode6829
src/objects.h:6829: inline void update_data_cache();
Put a comment saying that this function is only used by deserialization, and
that the data cache is otherwise always set correctly

http://codereview.chromium.org/8635011/diff/1/test/cctest/test-api.cc
File test/cctest/test-api.cc (right):

http://codereview.chromium.org/8635011/diff/1/test/cctest/test-api.cc#newcode494
test/cctest/test-api.cc:494: uint16_t* two_byte_string =
AsciiToTwoByteString("small");
Whether the externalized version of this string is short or not depends on the
size of pointers. On 64-bit it will be short, on 32-bit it won't.
Maybe we should select a string based on kPointerSize instead, for tests like
this, so it tests the same thing on all platforms.

[Yang, 2011-11-23 09:58:48]

Landing...

http://codereview.chromium.org/8635011/diff/1/src/api.cc
File src/api.cc (right):

http://codereview.chromium.org/8635011/diff/1/src/api.cc#newcode4563
src/api.cc:4563: if (size < i::ExternalString::kShortSize)
On 2011/11/23 08:43:20, Lasse Reichstein wrote:
> Put braces around the then-statement.

Done.

http://codereview.chromium.org/8635011/diff/1/src/ia32/codegen-ia32.cc
File src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/8635011/diff/1/src/ia32/codegen-ia32.cc#newcod...
src/ia32/codegen-ia32.cc:535: __ mov(result, FieldOperand(string,
HeapObject::kMapOffset));
On 2011/11/23 08:43:20, Lasse Reichstein wrote:
> We're absolutely sure that only strings reach here, right?

Yes.

http://codereview.chromium.org/8635011/diff/1/src/ia32/codegen-ia32.cc#newcod...
src/ia32/codegen-ia32.cc:571: __ test_b(scratch, kShortExternalStringTag);
On 2011/11/23 08:43:20, Lasse Reichstein wrote:
> Test with kShortExternalStringMask instead of ...Tag.

Done.

http://codereview.chromium.org/8635011/diff/1/src/objects.cc
File src/objects.cc (right):

http://codereview.chromium.org/8635011/diff/1/src/objects.cc#newcode958
src/objects.cc:958: ASSERT(size >= ExternalString::kShortSize);
On 2011/11/23 08:43:20, Lasse Reichstein wrote:
> This assert can safely be dropped.

Done.

http://codereview.chromium.org/8635011/diff/1/src/objects.cc#newcode983
src/objects.cc:983: ExternalTwoByteString::kHashFieldOffset);
On 2011/11/23 08:43:20, Lasse Reichstein wrote:
> These two asserts are unnecessary, since ExternalAsciiString doesn't have a
> static kLengthOffset or kHashFieldOffset property. This is comparing
> String::kLengthOffset with itself. Since it's placed on String, it should be
> assumed to apply to all string subtypes.

Done.

http://codereview.chromium.org/8635011/diff/1/src/objects.cc#newcode1013
src/objects.cc:1013: ASSERT(size >= ExternalString::kShortSize);
On 2011/11/23 08:43:20, Lasse Reichstein wrote:
> Ditto.

Done.

http://codereview.chromium.org/8635011/diff/1/src/objects.cc#newcode1029
src/objects.cc:1029: ExternalAsciiString::kHashFieldOffset);
On 2011/11/23 08:43:20, Lasse Reichstein wrote:
> As above.

Done.

http://codereview.chromium.org/8635011/diff/1/src/objects.cc#newcode1030
src/objects.cc:1030: if (is_symbol) self->Hash();  // Force regeneration of the
hash value.
On 2011/11/23 08:43:20, Lasse Reichstein wrote:
> Is there an explanation of how the hash can possibly change?
> It should be determined solely by the character codes and length of the
string,
> both of which are retained by externalization.

Looking into that later.

http://codereview.chromium.org/8635011/diff/1/src/objects.h
File src/objects.h (right):

http://codereview.chromium.org/8635011/diff/1/src/objects.h#newcode234
src/objects.h:234: V(EXTERNAL_ASCII_SYMBOL_TYPE)                                
               \
On 2011/11/23 08:43:20, Lasse Reichstein wrote:
> No SHORT_EXTERNAL_SYMBOL_TYPE etc.?

Must have forgotten that. Included now.

http://codereview.chromium.org/8635011/diff/1/src/objects.h#newcode6829
src/objects.h:6829: inline void update_data_cache();
On 2011/11/23 08:43:20, Lasse Reichstein wrote:
> Put a comment saying that this function is only used by deserialization, and
> that the data cache is otherwise always set correctly 

Done.

http://codereview.chromium.org/8635011/diff/1/test/cctest/test-api.cc
File test/cctest/test-api.cc (right):

http://codereview.chromium.org/8635011/diff/1/test/cctest/test-api.cc#newcode494
test/cctest/test-api.cc:494: uint16_t* two_byte_string =
AsciiToTwoByteString("small");
On 2011/11/23 08:43:20, Lasse Reichstein wrote:
> Whether the externalized version of this string is short or not depends on the
> size of pointers. On 64-bit it will be short, on 32-bit it won't.
> Maybe we should select a string based on kPointerSize instead, for tests like
> this, so it tests the same thing on all platforms.

I changed the string lengths to get short external for the first test and normal
external for the second test on all platforms.