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 fd770ea..5ef8bf1 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 e4ad691..6b586a0 100644 (file)
@@ -21,7 +21,6 @@ namespace internal {
   V(FunctionName)                 \
   V(FunctionLength)               \
   V(FunctionPrototype)            \
-  V(RegExpSource)                 \
   V(ScriptColumnOffset)           \
   V(ScriptCompilationType)        \
   V(ScriptContextData)            \
index af75efd..2d9a13f 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 7ba6254..1718b15 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 5846881..d3212f4 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(