Save memory by removing type from JSV8BreakIterator

Type is usually not used by Intl.v8BreakIterator unless
resolvedOptions() is called. Therefore we can save memory by
removing it from the object and using a slow test from the
iterator to find out the type when needed.

Bug: v8:10252
Change-Id: I7a8dfdc8310eab0d1c90278fbadfbae48e49668e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2067694
Commit-Queue: Frank Tang <ftang@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#66392}

src/diagnostics/objects-printer.cc

@@ -2007,7 +2007,6 @@ void Script::ScriptPrint(std::ostream& os) {  // NOLINT
 void JSV8BreakIterator::JSV8BreakIteratorPrint(std::ostream& os) {  // NOLINT
   JSObjectPrintHeader(os, *this, "JSV8BreakIterator");
   os << "\n - locale: " << Brief(locale());
-  os << "\n - type: " << TypeAsString();
   os << "\n - break iterator: " << Brief(break_iterator());
   os << "\n - unicode string: " << Brief(unicode_string());
   os << "\n - bound adopt text: " << Brief(bound_adopt_text());

src/objects/intl-objects.tq

@@ -82,7 +82,6 @@ extern class JSV8BreakIterator extends JSObject {
   bound_next: Undefined|JSFunction;
   bound_current: Undefined|JSFunction;
   bound_break_type: Undefined|JSFunction;
-  break_iterator_type: Smi;
 }
 
 extern class JSCollator extends JSObject {

src/objects/js-break-iterator-inl.h

@@ -20,14 +20,6 @@ namespace internal {
 
 OBJECT_CONSTRUCTORS_IMPL(JSV8BreakIterator, JSObject)
 
-inline JSV8BreakIterator::Type JSV8BreakIterator::type() const {
-  return static_cast<JSV8BreakIterator::Type>(raw_type());
-}
-
-inline void JSV8BreakIterator::set_type(Type type) {
-  set_raw_type(static_cast<int>(type));
-}
-
 ACCESSORS(JSV8BreakIterator, locale, String, kLocaleOffset)
 ACCESSORS(JSV8BreakIterator, break_iterator, Managed<icu::BreakIterator>,
           kBreakIteratorOffset)
@@ -39,8 +31,6 @@ ACCESSORS(JSV8BreakIterator, bound_next, Object, kBoundNextOffset)
 ACCESSORS(JSV8BreakIterator, bound_current, Object, kBoundCurrentOffset)
 ACCESSORS(JSV8BreakIterator, bound_break_type, Object, kBoundBreakTypeOffset)
 
-SMI_ACCESSORS(JSV8BreakIterator, raw_type, kBreakIteratorTypeOffset)
-
 CAST_ACCESSOR(JSV8BreakIterator)
 
 }  // namespace internal

src/objects/js-break-iterator.cc

@@ -15,6 +15,10 @@
 namespace v8 {
 namespace internal {
 
+namespace {
+enum class Type { CHARACTER, WORD, SENTENCE, LINE };
+}  // anonymous namespace
+
 MaybeHandle<JSV8BreakIterator> JSV8BreakIterator::New(
     Isolate* isolate, Handle<Map> map, Handle<Object> locales,
     Handle<Object> options_obj, const char* service) {
@@ -102,7 +106,6 @@ MaybeHandle<JSV8BreakIterator> JSV8BreakIterator::New(
           isolate->factory()->NewFastOrSlowJSObjectFromMap(map));
   DisallowHeapAllocation no_gc;
   break_iterator_holder->set_locale(*locale_str);
-  break_iterator_holder->set_type(type_enum);
   break_iterator_holder->set_break_iterator(*managed_break_iterator);
   break_iterator_holder->set_unicode_string(*managed_unicode_string);
 
@@ -110,17 +113,68 @@ MaybeHandle<JSV8BreakIterator> JSV8BreakIterator::New(
   return break_iterator_holder;
 }
 
+namespace {
+
+Type GetType(icu::BreakIterator* break_iterator) {
+  // Since the developer calling the Intl.v8BreakIterator already know the type,
+  // we usually do not need to know the type unless the resolvedOptions() is
+  // called, we use the following trick to figure out the type instead of
+  // storing it with the JSV8BreakIterator object to save memory.
+  // This routine is not fast but should be seldomly used only.
+
+  // We need to clone a copy of break iteator because we need to setText to it.
+  std::unique_ptr<icu::BreakIterator> cloned_break_iterator(
+      break_iterator->clone());
+  // Use a magic string "He is." to call next().
+  //  character type: will return 1 for "H"
+  //  word type: will return 2 for "He"
+  //  line type: will return 3 for "He "
+  //  sentence type: will return 6 for "He is."
+  icu::UnicodeString data("He is.");
+  cloned_break_iterator->setText(data);
+  switch (cloned_break_iterator->next()) {
+    case 1:  // After "H"
+      return Type::CHARACTER;
+    case 2:  // After "He"
+      return Type::WORD;
+    case 3:  // After "He "
+      return Type::LINE;
+    case 6:  // After "He is."
+      return Type::SENTENCE;
+    default:
+      UNREACHABLE();
+  }
+}
+
+Handle<String> TypeAsString(Isolate* isolate, Type type) {
+  switch (type) {
+    case Type::CHARACTER:
+      return ReadOnlyRoots(isolate).character_string_handle();
+    case Type::WORD:
+      return ReadOnlyRoots(isolate).word_string_handle();
+    case Type::SENTENCE:
+      return ReadOnlyRoots(isolate).sentence_string_handle();
+    case Type::LINE:
+      return ReadOnlyRoots(isolate).line_string_handle();
+  }
+  UNREACHABLE();
+}
+
+}  // anonymous namespace
+
 Handle<JSObject> JSV8BreakIterator::ResolvedOptions(
     Isolate* isolate, Handle<JSV8BreakIterator> break_iterator) {
   Factory* factory = isolate->factory();
 
+  Type type = GetType(break_iterator->break_iterator().raw());
+
   Handle<JSObject> result = factory->NewJSObject(isolate->object_function());
   Handle<String> locale(break_iterator->locale(), isolate);
 
   JSObject::AddProperty(isolate, result, factory->locale_string(), locale,
                         NONE);
   JSObject::AddProperty(isolate, result, factory->type_string(),
-                        break_iterator->TypeAsString(), NONE);
+                        TypeAsString(isolate, type), NONE);
   return result;
 }
 
@@ -135,20 +189,6 @@ void JSV8BreakIterator::AdoptText(
   break_iterator_holder->set_unicode_string(*unicode_string);
 }
 
-Handle<String> JSV8BreakIterator::TypeAsString() const {
-  switch (type()) {
-    case Type::CHARACTER:
-      return GetReadOnlyRoots().character_string_handle();
-    case Type::WORD:
-      return GetReadOnlyRoots().word_string_handle();
-    case Type::SENTENCE:
-      return GetReadOnlyRoots().sentence_string_handle();
-    case Type::LINE:
-      return GetReadOnlyRoots().line_string_handle();
-  }
-  UNREACHABLE();
-}
-
 Handle<Object> JSV8BreakIterator::Current(
     Isolate* isolate, Handle<JSV8BreakIterator> break_iterator) {
   return isolate->factory()->NewNumberFromInt(

src/objects/js-break-iterator.h

@@ -51,12 +51,6 @@ class JSV8BreakIterator : public JSObject {
   static String BreakType(Isolate* isolate,
                           Handle<JSV8BreakIterator> break_iterator);
 
-  enum class Type { CHARACTER, WORD, SENTENCE, LINE };
-  inline void set_type(Type type);
-  inline Type type() const;
-
-  Handle<String> TypeAsString() const;
-
   DECL_CAST(JSV8BreakIterator)
   DECL_PRINTER(JSV8BreakIterator)
   DECL_VERIFIER(JSV8BreakIterator)
@@ -75,8 +69,6 @@ class JSV8BreakIterator : public JSObject {
                                 TORQUE_GENERATED_JS_V8_BREAK_ITERATOR_FIELDS)
 
  private:
-  DECL_INT_ACCESSORS(raw_type)
-
   OBJECT_CONSTRUCTORS(JSV8BreakIterator, JSObject);
 };