Revert "[intl] Switch to using declared accessors"

This reverts commit 4968b2c4.

Reason for revert: Speculative revert for severe perf regression
https://bugs.chromium.org/p/chromium/issues/detail?id=716468#c3

Original change's description:
> [intl] Switch to using declared accessors
> 
> This patch cleans up the Intl code by switching to using declared
> accessors, rather than embedder fields, for holding references to
> ICU objects. Additionally:
> - Rename classes to be more similar to how other classes are named
> - Make some unreachable paths into check-fails, rather than throwing
>   JS exceptions
> - Move some macros from objects-inl.h into object-macros.h, to allow
>   the implementation here to not touch objects.h
> - Some setup logic is moved from runtime-i18n.cc to i18n.cc.
> 
> This patch leaves type tags as they are; a future patch should move
> from a special Intl type tagging system to object types as other system
> objects use. Future patches should also move more logic to i18n.cc
> 
> BUG=v8:5402,v8:5751,v8:6057
> CQ_INCLUDE_TRYBOTS=master.tryserver.v8:v8_linux_noi18n_rel_ng
> 
> Change-Id: Ia9cbb25cf8f52662e3deb15e64179d792c10842c
> Reviewed-on: https://chromium-review.googlesource.com/479651
> Commit-Queue: Daniel Ehrenberg <littledan@chromium.org>
> Reviewed-by: Adam Klein <adamk@chromium.org>
> Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#44804}

TBR=adamk@chromium.org,marja@chromium.org,mstarzinger@chromium.org,littledan@chromium.org,jwolfe@igalia.com
# Not skipping CQ checks because original CL landed > 1 day ago.
BUG=v8:5402,v8:5751,v8:6057
CQ_INCLUDE_TRYBOTS=master.tryserver.v8:v8_linux_noi18n_rel_ng

Change-Id: I7a45d7def1f1de0f21e3efb7de9b31f6bcfea46d
Reviewed-on: https://chromium-review.googlesource.com/490328Reviewed-by: Daniel Ehrenberg <littledan@chromium.org>
Commit-Queue: Daniel Ehrenberg <littledan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#44992}

BUILD.gn

@@ -1715,7 +1715,6 @@ v8_source_set("v8_base") {
     "src/objects/frame-array-inl.h",
     "src/objects/frame-array.h",
     "src/objects/hash-table.h",
-    "src/objects/intl-objects-inl.h",
     "src/objects/intl-objects.cc",
     "src/objects/intl-objects.h",
     "src/objects/literal-objects.cc",
@@ -2383,7 +2382,6 @@ v8_source_set("v8_base") {
       "src/builtins/builtins-intl.cc",
       "src/intl.cc",
       "src/intl.h",
-      "src/objects/intl-objects-inl.h",
       "src/objects/intl-objects.cc",
       "src/objects/intl-objects.h",
       "src/runtime/runtime-intl.cc",

src/bootstrapper.cc

@@ -2641,7 +2641,7 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
         factory->Object_string(),
         static_cast<PropertyAttributes>(DONT_ENUM | READ_ONLY));
     Handle<JSFunction> date_time_format_constructor = InstallFunction(
-        intl, "DateTimeFormat", JS_OBJECT_TYPE, JSIntlDateTimeFormat::kSize,
+        intl, "DateTimeFormat", JS_OBJECT_TYPE, DateFormat::kSize,
         date_time_format_prototype, Builtins::kIllegal);
     JSObject::AddProperty(date_time_format_prototype,
                           factory->constructor_string(),
@@ -2658,7 +2658,7 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
         factory->Object_string(),
         static_cast<PropertyAttributes>(DONT_ENUM | READ_ONLY));
     Handle<JSFunction> number_format_constructor = InstallFunction(
-        intl, "NumberFormat", JS_OBJECT_TYPE, JSIntlNumberFormat::kSize,
+        intl, "NumberFormat", JS_OBJECT_TYPE, NumberFormat::kSize,
         number_format_prototype, Builtins::kIllegal);
     JSObject::AddProperty(number_format_prototype,
                           factory->constructor_string(),
@@ -2675,7 +2675,7 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
         factory->Object_string(),
         static_cast<PropertyAttributes>(DONT_ENUM | READ_ONLY));
     Handle<JSFunction> collator_constructor =
-        InstallFunction(intl, "Collator", JS_OBJECT_TYPE, JSIntlCollator::kSize,
+        InstallFunction(intl, "Collator", JS_OBJECT_TYPE, Collator::kSize,
                         collator_prototype, Builtins::kIllegal);
     JSObject::AddProperty(collator_prototype, factory->constructor_string(),
                           collator_constructor, DONT_ENUM);
@@ -2690,7 +2690,7 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
         factory->Object_string(),
         static_cast<PropertyAttributes>(DONT_ENUM | READ_ONLY));
     Handle<JSFunction> v8_break_iterator_constructor = InstallFunction(
-        intl, "v8BreakIterator", JS_OBJECT_TYPE, JSIntlV8BreakIterator::kSize,
+        intl, "v8BreakIterator", JS_OBJECT_TYPE, V8BreakIterator::kSize,
         v8_break_iterator_prototype, Builtins::kIllegal);
     JSObject::AddProperty(v8_break_iterator_prototype,
                           factory->constructor_string(),

src/objects-inl.h

@@ -1286,6 +1286,15 @@ bool JSObject::PrototypeHasNoElements(Isolate* isolate, JSObject* object) {
   return true;
 }
 
+#define FIELD_ADDR(p, offset) \
+  (reinterpret_cast<byte*>(p) + offset - kHeapObjectTag)
+
+#define FIELD_ADDR_CONST(p, offset) \
+  (reinterpret_cast<const byte*>(p) + offset - kHeapObjectTag)
+
+#define READ_FIELD(p, offset) \
+  (*reinterpret_cast<Object* const*>(FIELD_ADDR_CONST(p, offset)))
+
 #define ACQUIRE_READ_FIELD(p, offset)           \
   reinterpret_cast<Object*>(base::Acquire_Load( \
       reinterpret_cast<const base::AtomicWord*>(FIELD_ADDR_CONST(p, offset))))
@@ -1294,6 +1303,9 @@ bool JSObject::PrototypeHasNoElements(Isolate* isolate, JSObject* object) {
   reinterpret_cast<Object*>(base::NoBarrier_Load( \
       reinterpret_cast<const base::AtomicWord*>(FIELD_ADDR_CONST(p, offset))))
 
+#define WRITE_FIELD(p, offset, value) \
+  (*reinterpret_cast<Object**>(FIELD_ADDR(p, offset)) = value)
+
 #define RELEASE_WRITE_FIELD(p, offset, value)                     \
   base::Release_Store(                                            \
       reinterpret_cast<base::AtomicWord*>(FIELD_ADDR(p, offset)), \
@@ -8191,7 +8203,11 @@ SMI_ACCESSORS(JSStringIterator, index, kNextIndexOffset)
 #undef NOBARRIER_SMI_ACCESSORS
 #undef BOOL_GETTER
 #undef BOOL_ACCESSORS
+#undef FIELD_ADDR
+#undef FIELD_ADDR_CONST
+#undef READ_FIELD
 #undef NOBARRIER_READ_FIELD
+#undef WRITE_FIELD
 #undef NOBARRIER_WRITE_FIELD
 #undef WRITE_BARRIER
 #undef CONDITIONAL_WRITE_BARRIER

src/objects/intl-objects-inl.h

-// Copyright 2017 the V8 project authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef V8_INTL_SUPPORT
-#error Internationalization is expected to be enabled.
-#endif  // V8_INTL_SUPPORT
-
-#ifndef V8_OBJECTS_INTL_OBJECTS_INL_H_
-#define V8_OBJECTS_INTL_OBJECTS_INL_H_
-
-#include "src/objects/intl-objects.h"
-
-// Has to be the last include (doesn't have include guards):
-#include "src/objects/object-macros.h"
-
-#define PTR_ACCESSORS(holder, name, type, offset)   \
-  inline type* holder::name() const {               \
-    Object* obj = READ_FIELD(this, offset);         \
-    DCHECK(obj->IsSmi());                           \
-    return reinterpret_cast<type*>(obj);            \
-  }                                                 \
-  inline void holder::set_##name(type* value) {     \
-    Object* obj = reinterpret_cast<Object*>(value); \
-    DCHECK(obj->IsSmi());                           \
-    WRITE_FIELD(this, offset, obj);                 \
-  }
-
-namespace v8 {
-namespace internal {
-
-PTR_ACCESSORS(JSIntlDateTimeFormat, simple_date_format, icu::SimpleDateFormat,
-              kSimpleDateFormat)
-
-PTR_ACCESSORS(JSIntlNumberFormat, decimal_format, icu::DecimalFormat,
-              kDecimalFormat)
-
-PTR_ACCESSORS(JSIntlCollator, collator, icu::Collator, kCollator)
-
-PTR_ACCESSORS(JSIntlV8BreakIterator, break_iterator, icu::BreakIterator,
-              kBreakIterator)
-PTR_ACCESSORS(JSIntlV8BreakIterator, unicode_string, icu::UnicodeString,
-              kUnicodeString)
-
-}  // namespace internal
-}  // namespace v8
-
-#undef PTR_ACCESSORS
-
-#include "src/objects/object-macros-undef.h"
-
-#endif  // V8_OBJECTS_INTL_OBJECTS_INL_H_

src/objects/intl-objects.h

@@ -17,114 +17,113 @@ class BreakIterator;
 class Collator;
 class DecimalFormat;
 class SimpleDateFormat;
-class UnicodeString;
 }
 
 namespace v8 {
 namespace internal {
 
-#define DECL_PTR_ACCESSORS(name, type) \
-  inline type* name() const;           \
-  inline void set_##name(type* value);
+template <typename T>
+class Handle;
 
-// Intl.DateTimeFormat
-// ECMA-402#datetimeformat-objects
-class JSIntlDateTimeFormat : public JSObject {
+class DateFormat {
  public:
-  DECL_PTR_ACCESSORS(simple_date_format, icu::SimpleDateFormat)
-
-  // Constructor for Intl.DateTimeFormat(), based on the resolved locale
-  // and user options. Writes resolved options into resolved.
-  MUST_USE_RESULT static MaybeHandle<JSIntlDateTimeFormat> New(
+  // Create a formatter for the specificied locale and options. Returns the
+  // resolved settings for the locale / options.
+  static icu::SimpleDateFormat* InitializeDateTimeFormat(
       Isolate* isolate, Handle<String> locale, Handle<JSObject> options,
       Handle<JSObject> resolved);
 
+  // Unpacks date format object from corresponding JavaScript object.
+  static icu::SimpleDateFormat* UnpackDateFormat(Isolate* isolate,
+                                                 Handle<JSObject> obj);
+
+  // Release memory we allocated for the DateFormat once the JS object that
+  // holds the pointer gets garbage collected.
+  static void DeleteDateFormat(const v8::WeakCallbackInfo<void>& data);
+
   // Layout description.
   static const int kSimpleDateFormat = JSObject::kHeaderSize;
   static const int kSize = kSimpleDateFormat + kPointerSize;
 
  private:
-  // Finalizer responsible for freeing the icu::SimpleDateFormat
-  static void Delete(const v8::WeakCallbackInfo<void>& data);
-
-  DISALLOW_IMPLICIT_CONSTRUCTORS(JSIntlDateTimeFormat);
+  DateFormat();
 };
 
-// Intl.NumberFormat
-// ECMA-402#numberformat-objects
-class JSIntlNumberFormat : public JSObject {
+class NumberFormat {
  public:
-  DECL_PTR_ACCESSORS(decimal_format, icu::DecimalFormat)
-
-  // Constructor for Intl.NumberFormat(), based on the resolved locale
-  // and user options. Writes resolved options into resolved.
-  MUST_USE_RESULT static MaybeHandle<JSIntlNumberFormat> New(
-      Isolate* isolate, Handle<String> locale, Handle<JSObject> options,
+  // Create a formatter for the specificied locale and options. Returns the
+  // resolved settings for the locale / options.
+  static icu::DecimalFormat* InitializeNumberFormat(Isolate* isolate,
+                                                    Handle<String> locale,
+                                                    Handle<JSObject> options,
                                                     Handle<JSObject> resolved);
 
+  // Unpacks number format object from corresponding JavaScript object.
+  static icu::DecimalFormat* UnpackNumberFormat(Isolate* isolate,
+                                                Handle<JSObject> obj);
+
+  // Release memory we allocated for the NumberFormat once the JS object that
+  // holds the pointer gets garbage collected.
+  static void DeleteNumberFormat(const v8::WeakCallbackInfo<void>& data);
+
   // Layout description.
   static const int kDecimalFormat = JSObject::kHeaderSize;
   static const int kSize = kDecimalFormat + kPointerSize;
 
  private:
-  // Finalizer responsible for freeing the icu::DecimalFormat
-  static void Delete(const v8::WeakCallbackInfo<void>& data);
-
-  DISALLOW_IMPLICIT_CONSTRUCTORS(JSIntlNumberFormat);
+  NumberFormat();
 };
 
-// Intl.Collator
-// ECMA-402#collator-objects
-class JSIntlCollator : public JSObject {
+class Collator {
  public:
-  DECL_PTR_ACCESSORS(collator, icu::Collator)
-
-  // Constructor for Intl.Collator(), based on the resolved locale
-  // and user options. Writes resolved options into resolved.
-  MUST_USE_RESULT static MaybeHandle<JSIntlCollator> New(
-      Isolate* isolate, Handle<String> locale, Handle<JSObject> options,
+  // Create a collator for the specificied locale and options. Returns the
+  // resolved settings for the locale / options.
+  static icu::Collator* InitializeCollator(Isolate* isolate,
+                                           Handle<String> locale,
+                                           Handle<JSObject> options,
                                            Handle<JSObject> resolved);
 
+  // Unpacks collator object from corresponding JavaScript object.
+  static icu::Collator* UnpackCollator(Isolate* isolate, Handle<JSObject> obj);
+
+  // Release memory we allocated for the Collator once the JS object that holds
+  // the pointer gets garbage collected.
+  static void DeleteCollator(const v8::WeakCallbackInfo<void>& data);
+
   // Layout description.
   static const int kCollator = JSObject::kHeaderSize;
   static const int kSize = kCollator + kPointerSize;
 
  private:
-  // Finalizer responsible for freeing the icu::Collator
-  static void Delete(const v8::WeakCallbackInfo<void>& data);
-
-  DISALLOW_IMPLICIT_CONSTRUCTORS(JSIntlCollator);
+  Collator();
 };
 
-// Intl.v8BreakIterator, Custom non-standard V8 word break binding
-// TODO(littledan,jwolfe): Specify, implement and ship Intl.Segmenter,
-// allowing this interface to be deprecasted and removed.
-class JSIntlV8BreakIterator : public JSObject {
+class V8BreakIterator {
  public:
-  DECL_PTR_ACCESSORS(break_iterator, icu::BreakIterator)
-  DECL_PTR_ACCESSORS(unicode_string, icu::UnicodeString)
-
-  // Constructor for Intl.v8BreakIterator(), based on the resolved locale
-  // and user options. Writes resolved options into resolved.
-  MUST_USE_RESULT static MaybeHandle<JSIntlV8BreakIterator> New(
-      Isolate* isolate, Handle<String> locale, Handle<JSObject> options,
+  // Create a BreakIterator for the specificied locale and options. Returns the
+  // resolved settings for the locale / options.
+  static icu::BreakIterator* InitializeBreakIterator(Isolate* isolate,
+                                                     Handle<String> locale,
+                                                     Handle<JSObject> options,
                                                      Handle<JSObject> resolved);
 
+  // Unpacks break iterator object from corresponding JavaScript object.
+  static icu::BreakIterator* UnpackBreakIterator(Isolate* isolate,
+                                                 Handle<JSObject> obj);
+
+  // Release memory we allocated for the BreakIterator once the JS object that
+  // holds the pointer gets garbage collected.
+  static void DeleteBreakIterator(const v8::WeakCallbackInfo<void>& data);
+
   // Layout description.
   static const int kBreakIterator = JSObject::kHeaderSize;
   static const int kUnicodeString = kBreakIterator + kPointerSize;
   static const int kSize = kUnicodeString + kPointerSize;
 
  private:
-  // Finalizer responsible for freeing the icu::BreakIterator
-  // and icu::UnicodeString
-  static void Delete(const v8::WeakCallbackInfo<void>& data);
-
-  DISALLOW_IMPLICIT_CONSTRUCTORS(JSIntlV8BreakIterator);
+  V8BreakIterator();
 };
 
-#undef DECL_PTR_ACCESSORS
-
 }  // namespace internal
 }  // namespace v8
 

src/objects/object-macros-undef.h

@@ -8,7 +8,3 @@
 #undef DECL_ACCESSORS
 #undef DECLARE_CAST
 #undef DECLARE_VERIFIER
-#undef FIELD_ADDR
-#undef FIELD_ADDR_CONST
-#undef READ_FIELD
-#undef WRITE_FIELD

src/objects/object-macros.h

@@ -40,15 +40,3 @@
 #else
 #define DECLARE_VERIFIER(Name)
 #endif
-
-#define FIELD_ADDR(p, offset) \
-  (reinterpret_cast<byte*>(p) + offset - kHeapObjectTag)
-
-#define FIELD_ADDR_CONST(p, offset) \
-  (reinterpret_cast<const byte*>(p) + offset - kHeapObjectTag)
-
-#define READ_FIELD(p, offset) \
-  (*reinterpret_cast<Object* const*>(FIELD_ADDR_CONST(p, offset)))
-
-#define WRITE_FIELD(p, offset, value) \
-  (*reinterpret_cast<Object**>(FIELD_ADDR(p, offset)) = value)

src/v8.gyp

@@ -1125,7 +1125,6 @@
         'objects/frame-array-inl.h',
         'objects/hash-table.h',
         'objects/intl-objects.cc',
-        'objects/intl-objects-inl.h',
         'objects/intl-objects.h',
         'objects/literal-objects.cc',
         'objects/literal-objects.h',
@@ -1841,7 +1840,6 @@
             'intl.cc',
             'intl.h',
             'objects/intl-objects.cc',
-            'objects/intl-objects-inl.h',
             'objects/intl-objects.h',
             'runtime/runtime-intl.cc',
           ],