Eagerly escape RegExp.source.
authoryangguo <yangguo@chromium.org>
Thu, 9 Apr 2015 13:22:17 +0000 (06:22 -0700)
committerCommit bot <commit-bot@chromium.org>
Thu, 9 Apr 2015 13:22:22 +0000 (13:22 +0000)
Escaping used to happen lazily, implemented in an accessor property.
However, native implementation of RegExp methods use .source as well.
This leads to performance regressions. Now we do it eagerly instead.

R=jkummerow@chromium.org
BUG=chromium:436447
LOG=N

Review URL: https://codereview.chromium.org/1070093002

Cr-Commit-Position: refs/heads/master@{#27705}

src/accessors.cc
src/accessors.h
src/bootstrapper.cc
src/objects.h
src/runtime/runtime-regexp.cc

index fd770ea16be5bc1befe01daab72b26d041c0010b..5ef8bf15f7e0d00e2724ed0d65748e19a9af2395 100644 (file)
@@ -312,98 +312,6 @@ Handle<AccessorInfo> Accessors::StringLengthInfo(
 }
 
 
-template <typename Char>
-inline int CountRequiredEscapes(Handle<String> source) {
-  DisallowHeapAllocation no_gc;
-  int escapes = 0;
-  Vector<const Char> src = source->GetCharVector<Char>();
-  for (int i = 0; i < src.length(); i++) {
-    if (src[i] == '/' && (i == 0 || src[i - 1] != '\\')) escapes++;
-  }
-  return escapes;
-}
-
-
-template <typename Char, typename StringType>
-inline Handle<StringType> WriteEscapedRegExpSource(Handle<String> source,
-                                                   Handle<StringType> result) {
-  DisallowHeapAllocation no_gc;
-  Vector<const Char> src = source->GetCharVector<Char>();
-  Vector<Char> dst(result->GetChars(), result->length());
-  int s = 0;
-  int d = 0;
-  while (s < src.length()) {
-    if (src[s] == '/' && (s == 0 || src[s - 1] != '\\')) dst[d++] = '\\';
-    dst[d++] = src[s++];
-  }
-  DCHECK_EQ(result->length(), d);
-  return result;
-}
-
-
-MaybeHandle<String> EscapeRegExpSource(Isolate* isolate,
-                                       Handle<String> source) {
-  String::Flatten(source);
-  if (source->length() == 0) return isolate->factory()->query_colon_string();
-  bool one_byte = source->IsOneByteRepresentationUnderneath();
-  int escapes = one_byte ? CountRequiredEscapes<uint8_t>(source)
-                         : CountRequiredEscapes<uc16>(source);
-  if (escapes == 0) return source;
-  int length = source->length() + escapes;
-  if (one_byte) {
-    Handle<SeqOneByteString> result;
-    ASSIGN_RETURN_ON_EXCEPTION(isolate, result,
-                               isolate->factory()->NewRawOneByteString(length),
-                               String);
-    return WriteEscapedRegExpSource<uint8_t>(source, result);
-  } else {
-    Handle<SeqTwoByteString> result;
-    ASSIGN_RETURN_ON_EXCEPTION(isolate, result,
-                               isolate->factory()->NewRawTwoByteString(length),
-                               String);
-    return WriteEscapedRegExpSource<uc16>(source, result);
-  }
-}
-
-
-// Implements ECMA262 ES6 draft 21.2.5.9
-void Accessors::RegExpSourceGetter(
-    v8::Local<v8::Name> name, const v8::PropertyCallbackInfo<v8::Value>& info) {
-  i::Isolate* isolate = reinterpret_cast<i::Isolate*>(info.GetIsolate());
-  HandleScope scope(isolate);
-
-  Handle<Object> holder =
-      Utils::OpenHandle(*v8::Local<v8::Value>(info.Holder()));
-  Handle<JSRegExp> regexp = Handle<JSRegExp>::cast(holder);
-  Handle<String> result;
-  if (regexp->TypeTag() == JSRegExp::NOT_COMPILED) {
-    result = isolate->factory()->empty_string();
-  } else {
-    Handle<String> pattern(regexp->Pattern(), isolate);
-    MaybeHandle<String> maybe = EscapeRegExpSource(isolate, pattern);
-    if (!maybe.ToHandle(&result)) {
-      isolate->OptionalRescheduleException(false);
-      return;
-    }
-  }
-  info.GetReturnValue().Set(Utils::ToLocal(result));
-}
-
-
-void Accessors::RegExpSourceSetter(v8::Local<v8::Name> name,
-                                   v8::Local<v8::Value> value,
-                                   const v8::PropertyCallbackInfo<void>& info) {
-  UNREACHABLE();
-}
-
-
-Handle<AccessorInfo> Accessors::RegExpSourceInfo(
-    Isolate* isolate, PropertyAttributes attributes) {
-  return MakeAccessor(isolate, isolate->factory()->source_string(),
-                      &RegExpSourceGetter, &RegExpSourceSetter, attributes);
-}
-
-
 //
 // Accessors::ScriptColumnOffset
 //
index e4ad691f7331b7f033c743ea1fd306ec3a7af26c..6b586a0dc7ce7a48f2454647d7e9a1657ba959e3 100644 (file)
@@ -21,7 +21,6 @@ namespace internal {
   V(FunctionName)                 \
   V(FunctionLength)               \
   V(FunctionPrototype)            \
-  V(RegExpSource)                 \
   V(ScriptColumnOffset)           \
   V(ScriptCompilationType)        \
   V(ScriptContextData)            \
index af75efd543a215b645e29484a0c9c91b2a272e38..2d9a13f19a6bc25a74d301c9e8ce07a58a9fc79e 100644 (file)
@@ -1066,11 +1066,10 @@ void Genesis::InitializeGlobal(Handle<GlobalObject> global_object,
 
     {
       // ECMA-262, section 15.10.7.1.
-      Handle<AccessorInfo> regexp_source(
-          Accessors::RegExpSourceInfo(isolate, final));
-      AccessorConstantDescriptor d(factory->source_string(), regexp_source,
-                                   final);
-      initial_map->AppendDescriptor(&d);
+      DataDescriptor field(factory->source_string(),
+                           JSRegExp::kSourceFieldIndex, final,
+                           Representation::Tagged());
+      initial_map->AppendDescriptor(&field);
     }
     {
       // ECMA-262, section 15.10.7.2.
@@ -1114,6 +1113,8 @@ void Genesis::InitializeGlobal(Handle<GlobalObject> global_object,
     Handle<Map> proto_map = Map::Copy(initial_map, "RegExpPrototype");
     DCHECK(proto_map->prototype() == *isolate->initial_object_prototype());
     Handle<JSObject> proto = factory->NewJSObjectFromMap(proto_map);
+    proto->InObjectPropertyAtPut(JSRegExp::kSourceFieldIndex,
+                                 heap->query_colon_string());
     proto->InObjectPropertyAtPut(JSRegExp::kGlobalFieldIndex,
                                  heap->false_value());
     proto->InObjectPropertyAtPut(JSRegExp::kIgnoreCaseFieldIndex,
index 7ba625442369de1f1f0dd57ac262c03196c90bae..1718b154f0ee8a553eb313b598334b53377539a5 100644 (file)
@@ -8120,11 +8120,12 @@ class JSRegExp: public JSObject {
       FixedArray::kHeaderSize + kIrregexpCaptureCountIndex * kPointerSize;
 
   // In-object fields.
-  static const int kGlobalFieldIndex = 0;
-  static const int kIgnoreCaseFieldIndex = 1;
-  static const int kMultilineFieldIndex = 2;
-  static const int kLastIndexFieldIndex = 3;
-  static const int kInObjectFieldCount = 4;
+  static const int kSourceFieldIndex = 0;
+  static const int kGlobalFieldIndex = 1;
+  static const int kIgnoreCaseFieldIndex = 2;
+  static const int kMultilineFieldIndex = 3;
+  static const int kLastIndexFieldIndex = 4;
+  static const int kInObjectFieldCount = 5;
 
   // The uninitialized value for a regexp code object.
   static const int kUninitializedValue = -1;
index 5846881f90ad5b68505a000753d8fd2f7324ce5f..d3212f464ff9839b13a8cb4ab107c8145561ecfa 100644 (file)
@@ -844,6 +844,60 @@ static JSRegExp::Flags RegExpFlagsFromString(Handle<String> flags,
 }
 
 
+template <typename Char>
+inline int CountRequiredEscapes(Handle<String> source) {
+  DisallowHeapAllocation no_gc;
+  int escapes = 0;
+  Vector<const Char> src = source->GetCharVector<Char>();
+  for (int i = 0; i < src.length(); i++) {
+    if (src[i] == '/' && (i == 0 || src[i - 1] != '\\')) escapes++;
+  }
+  return escapes;
+}
+
+
+template <typename Char, typename StringType>
+inline Handle<StringType> WriteEscapedRegExpSource(Handle<String> source,
+                                                   Handle<StringType> result) {
+  DisallowHeapAllocation no_gc;
+  Vector<const Char> src = source->GetCharVector<Char>();
+  Vector<Char> dst(result->GetChars(), result->length());
+  int s = 0;
+  int d = 0;
+  while (s < src.length()) {
+    if (src[s] == '/' && (s == 0 || src[s - 1] != '\\')) dst[d++] = '\\';
+    dst[d++] = src[s++];
+  }
+  DCHECK_EQ(result->length(), d);
+  return result;
+}
+
+
+MaybeHandle<String> EscapeRegExpSource(Isolate* isolate,
+                                       Handle<String> source) {
+  String::Flatten(source);
+  if (source->length() == 0) return isolate->factory()->query_colon_string();
+  bool one_byte = source->IsOneByteRepresentationUnderneath();
+  int escapes = one_byte ? CountRequiredEscapes<uint8_t>(source)
+                         : CountRequiredEscapes<uc16>(source);
+  if (escapes == 0) return source;
+  int length = source->length() + escapes;
+  if (one_byte) {
+    Handle<SeqOneByteString> result;
+    ASSIGN_RETURN_ON_EXCEPTION(isolate, result,
+                               isolate->factory()->NewRawOneByteString(length),
+                               String);
+    return WriteEscapedRegExpSource<uint8_t>(source, result);
+  } else {
+    Handle<SeqTwoByteString> result;
+    ASSIGN_RETURN_ON_EXCEPTION(isolate, result,
+                               isolate->factory()->NewRawTwoByteString(length),
+                               String);
+    return WriteEscapedRegExpSource<uc16>(source, result);
+  }
+}
+
+
 RUNTIME_FUNCTION(Runtime_RegExpInitializeAndCompile) {
   HandleScope scope(isolate);
   DCHECK(args.length() == 3);
@@ -865,6 +919,10 @@ RUNTIME_FUNCTION(Runtime_RegExpInitializeAndCompile) {
         isolate, NewSyntaxError("invalid_regexp_flags", args));
   }
 
+  Handle<String> escaped_source;
+  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, escaped_source,
+                                     EscapeRegExpSource(isolate, source));
+
   Handle<Object> global = factory->ToBoolean(flags.is_global());
   Handle<Object> ignore_case = factory->ToBoolean(flags.is_ignore_case());
   Handle<Object> multiline = factory->ToBoolean(flags.is_multiline());
@@ -877,6 +935,7 @@ RUNTIME_FUNCTION(Runtime_RegExpInitializeAndCompile) {
       constructor->IsJSFunction() &&
       JSFunction::cast(constructor)->initial_map() == map) {
     // If we still have the original map, set in-object properties directly.
+    regexp->InObjectPropertyAtPut(JSRegExp::kSourceFieldIndex, *escaped_source);
     // Both true and false are immovable immortal objects so no need for write
     // barrier.
     regexp->InObjectPropertyAtPut(JSRegExp::kGlobalFieldIndex, *global,
@@ -898,6 +957,8 @@ RUNTIME_FUNCTION(Runtime_RegExpInitializeAndCompile) {
     PropertyAttributes writable =
         static_cast<PropertyAttributes>(DONT_ENUM | DONT_DELETE);
     Handle<Object> zero(Smi::FromInt(0), isolate);
+    JSObject::SetOwnPropertyIgnoreAttributes(regexp, factory->source_string(),
+                                             escaped_source, final).Check();
     JSObject::SetOwnPropertyIgnoreAttributes(regexp, factory->global_string(),
                                              global, final).Check();
     JSObject::SetOwnPropertyIgnoreAttributes(