Revert "[objects] No longer create short external strings."

This reverts commit e42e7fc800.

Reason for revert: Speculative revert for:
https://crbug.com/v8/7149

Original change's description:
> [objects] No longer create short external strings.
> 
> This fixes String::MakeExternal() to bail out if the subject string
> doesn't fit a regular ExternalString, instead of creating a short
> external string. The observation here is that for short external strings
> the overhead of having to have the StringResource plus going to the
> runtime/C++ for each and every character access from JavaScript land
> is probably bigger than the anticipated benefits.
> 
> If this turns out to be wrong and there's a real benefit, we should make
> use of ThinStrings instead of having a separate way to represent
> external strings.
> 
> Bug: v8:6621, v8:7109, v8:7145
> Change-Id: I4b75da08b82a72027c782a69de9c8eaf3cca1d4d
> Reviewed-on: https://chromium-review.googlesource.com/799750
> Reviewed-by: Yang Guo <yangguo@chromium.org>
> Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#49735}

TBR=yangguo@chromium.org,bmeurer@chromium.org

Change-Id: I3f5cfa9ab5c99ddce1d61ede9ed9515cb3936cdd
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:6621, v8:7109, v8:7145, v8:7149
Reviewed-on: https://chromium-review.googlesource.com/801675
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49770}

src/objects.cc

@@ -2739,7 +2739,7 @@ bool String::MakeExternal(v8::String::ExternalStringResource* resource) {
 #endif  // DEBUG
   int size = this->Size();  // Byte size of the original string.
   // Abort if size does not allow in-place conversion.
-  if (size < ExternalString::kSize) return false;
+  if (size < ExternalString::kShortSize) return false;
   Heap* heap = GetHeap();
   bool is_one_byte = this->IsOneByteRepresentation();
   bool is_internalized = this->IsInternalizedString();
@@ -2753,16 +2753,25 @@ bool String::MakeExternal(v8::String::ExternalStringResource* resource) {
   // Instead, we resort to a short external string instead, omitting
   // the field caching the address of the backing store.  When we encounter
   // short external strings in generated code, we need to bailout to runtime.
-  Map* new_map =
-      is_internalized
-          ? (is_one_byte
-                 ? heap->external_internalized_string_with_one_byte_data_map()
-                 : heap->external_internalized_string_map())
-          : (is_one_byte ? heap->external_string_with_one_byte_data_map()
-                         : heap->external_string_map());
+  Map* new_map;
+  if (size < ExternalString::kSize) {
+    new_map = is_internalized
+        ? (is_one_byte
+           ? heap->short_external_internalized_string_with_one_byte_data_map()
+           : heap->short_external_internalized_string_map())
+        : (is_one_byte ? heap->short_external_string_with_one_byte_data_map()
+                       : heap->short_external_string_map());
+  } else {
+    new_map = is_internalized
+        ? (is_one_byte
+           ? heap->external_internalized_string_with_one_byte_data_map()
+           : heap->external_internalized_string_map())
+        : (is_one_byte ? heap->external_string_with_one_byte_data_map()
+                       : heap->external_string_map());
+  }
 
   // Byte size of the external String object.
-  int new_size = ExternalString::kSize;
+  int new_size = this->SizeFromMap(new_map);
   heap->CreateFillerObjectAt(this->address() + new_size, size - new_size,
                              ClearRecordedSlots::kNo);
   if (has_pointers) {
@@ -2803,7 +2812,7 @@ bool String::MakeExternal(v8::String::ExternalOneByteStringResource* resource) {
 #endif  // DEBUG
   int size = this->Size();  // Byte size of the original string.
   // Abort if size does not allow in-place conversion.
-  if (size < ExternalString::kSize) return false;
+  if (size < ExternalString::kShortSize) return false;
   Heap* heap = GetHeap();
   bool is_internalized = this->IsInternalizedString();
   bool has_pointers = StringShape(this).IsIndirect();
@@ -2818,12 +2827,19 @@ bool String::MakeExternal(v8::String::ExternalOneByteStringResource* resource) {
   // Instead, we resort to a short external string instead, omitting
   // the field caching the address of the backing store.  When we encounter
   // short external strings in generated code, we need to bailout to runtime.
-  Map* new_map = is_internalized
-                     ? heap->external_one_byte_internalized_string_map()
-                     : heap->external_one_byte_string_map();
+  Map* new_map;
+  if (size < ExternalString::kSize) {
+    new_map = is_internalized
+                  ? heap->short_external_one_byte_internalized_string_map()
+                  : heap->short_external_one_byte_string_map();
+  } else {
+    new_map = is_internalized
+                  ? heap->external_one_byte_internalized_string_map()
+                  : heap->external_one_byte_string_map();
+  }
 
   // Byte size of the external String object.
-  int new_size = ExternalString::kSize;
+  int new_size = this->SizeFromMap(new_map);
   heap->CreateFillerObjectAt(this->address() + new_size, size - new_size,
                              ClearRecordedSlots::kNo);
   if (has_pointers) {

test/cctest/test-serialize.cc

@@ -1669,18 +1669,18 @@ TEST(CodeSerializerExternalString) {
   v8::HandleScope scope(CcTest::isolate());
 
   // Obtain external internalized one-byte string.
-  SerializerOneByteResource one_byte_resource("one_byte_string", 15);
+  SerializerOneByteResource one_byte_resource("one_byte", 8);
   Handle<String> one_byte_string =
-      isolate->factory()->NewStringFromAsciiChecked("one_byte_string");
+      isolate->factory()->NewStringFromAsciiChecked("one_byte");
   one_byte_string = isolate->factory()->InternalizeString(one_byte_string);
   one_byte_string->MakeExternal(&one_byte_resource);
   CHECK(one_byte_string->IsExternalOneByteString());
   CHECK(one_byte_string->IsInternalizedString());
 
   // Obtain external internalized two-byte string.
-  SerializerTwoByteResource two_byte_resource("two_byte_string", 15);
+  SerializerTwoByteResource two_byte_resource("two_byte", 8);
   Handle<String> two_byte_string =
-      isolate->factory()->NewStringFromAsciiChecked("two_byte_string");
+      isolate->factory()->NewStringFromAsciiChecked("two_byte");
   two_byte_string = isolate->factory()->InternalizeString(two_byte_string);
   two_byte_string->MakeExternal(&two_byte_resource);
   CHECK(two_byte_string->IsExternalTwoByteString());
@@ -1688,9 +1688,9 @@ TEST(CodeSerializerExternalString) {
 
   const char* source =
       "var o = {}               \n"
-      "o.one_byte_string = 7;          \n"
-      "o.two_byte_string = 8;          \n"
-      "o.one_byte_string + o.two_byte_string; \n";
+      "o.one_byte = 7;          \n"
+      "o.two_byte = 8;          \n"
+      "o.one_byte + o.two_byte; \n";
   Handle<String> source_string = isolate->factory()
                                      ->NewStringFromUtf8(CStrVector(source))
                                      .ToHandleChecked();

test/mjsunit/ignition/string-concat-external-thin-string.js

@@ -5,7 +5,7 @@
 // Flags: --allow-natives-syntax --expose-externalize-string
 
 // Calculate string so that it isn't internalized.
-var string = ((a,b) => { return a + b; })('foo', 'barbaz');
+var string = ((a,b) => { return a + b; })('foo', 'bar');
 
 // Now externalize it.
 externalizeString(string, false);
@@ -16,4 +16,4 @@ var obj = {};
 obj[string];
 
 // Perform a string concatination.
-assertEquals('"' + string + '"', '"foobarbaz"');
+assertEquals('"' + string + '"', '"foobar"');

test/mjsunit/string-externalize.js

@@ -29,7 +29,7 @@
 
 var size = 1024;
 
-function dont_inline() { return "AAAAAAA"; }
+function dont_inline() { return "A"; }
 %NeverOptimizeFunction(dont_inline);
 
 function dont_inline2() { return "\u1234"; }