Regexp caching

src/jsregexp.cc

Index: src/jsregexp.cc
diff --git a/src/jsregexp.cc b/src/jsregexp.cc
index 675f82ac1b4e19e683a7813499b7432dec666c51..bad7ee549182ba21b9167d9d88e2ceb553be5987 100644
--- a/src/jsregexp.cc
+++ b/src/jsregexp.cc
@@ -34,6 +34,7 @@
 #include "platform.h"
 #include "runtime.h"
 #include "top.h"
+#include "compilation-cache.h"
 
 namespace v8 { namespace internal {
 
@@ -143,29 +144,59 @@ Handle<String> RegExpImpl::StringToTwoByte(Handle<String> pattern) {
 }
 
 
+static int RegExpFlagsFromString(Handle<String> str) {
+  int flags = JSRegExp::NONE;

[Kasper Lund, 2008/10/24 06:42:50]

Would it make sense to have some sort of opaque data type for RegExp flags?
Using 'int' seems a bit too generic. Sometimes we use an enum with no entries;
see Code::Flags.

+  for (int i = 0; i < str->length(); i++) {
+    switch (str->Get(i)) {
+    case 'i':
+      flags |= JSRegExp::IGNORE_CASE;
+      break;
+    case 'g':
+      flags |= JSRegExp::GLOBAL;
+      break;
+    case 'm':
+      flags |= JSRegExp::MULTILINE;
+      break;
+    }
+  }
+  return flags;
+}
+
+
 unibrow::Predicate<unibrow::RegExpSpecialChar, 128> is_reg_exp_special_char;
 
 
 Handle<Object> RegExpImpl::Compile(Handle<JSRegExp> re,
                                    Handle<String> pattern,
-                                   Handle<String> flags) {
-  bool is_atom = true;
-  for (int i = 0; is_atom && i < flags->length(); i++) {
-    if (flags->Get(i) == 'i')
-      is_atom = false;
-  }
-  for (int i = 0; is_atom && i < pattern->length(); i++) {
-    if (is_reg_exp_special_char.get(pattern->Get(i)))
-      is_atom = false;
-  }
+                                   Handle<String> flag_str) {
+  int flags = RegExpFlagsFromString(flag_str);
+  Handle<Object> cached = CompilationCache::LookupRegExp(pattern, flags);
+  bool in_cache = cached->IsFixedArray();

[Kasper Lund, 2008/10/24 06:42:50]

I would move the IsFixedArray logic into the cache to make it more consistent
with the other CompilationCache lookup functions and change the check here to an
empty handle check. Then you could also change the return type of LookupRegExp
to Handle<FixedArray>.

   Handle<Object> result;
-  if (is_atom) {
-    result = AtomCompile(re, pattern);
+  if (in_cache) {
+    re->set_data(*cached);
+    result = re;
   } else {
-    result = JsreCompile(re, pattern, flags);
+    bool is_atom = ((flags & JSRegExp::IGNORE_CASE) == 0);
+    for (int i = 0; is_atom && i < pattern->length(); i++) {
+      if (is_reg_exp_special_char.get(pattern->Get(i)))
+        is_atom = false;
+    }
+    if (is_atom) {
+      result = AtomCompile(re, pattern, flags);
+    } else {
+      result = JsreCompile(re, pattern, flags);
+    }
+    Object* data = re->data();
+    if (data->IsFixedArray()) {
+      // If compilation succeeded then the data is set on the regexp
+      // and we can store it in the cache.
+      Handle<FixedArray> data(FixedArray::cast(re->data()));
+      CompilationCache::PutRegExp(pattern, flags, data);
+    }
   }
 
-  LOG(RegExpCompileEvent(re));
+  LOG(RegExpCompileEvent(re, in_cache));
   return result;
 }
 
@@ -173,7 +204,7 @@ Handle<Object> RegExpImpl::Compile(Handle<JSRegExp> re,
 Handle<Object> RegExpImpl::Exec(Handle<JSRegExp> regexp,
                                 Handle<String> subject,
                                 Handle<Object> index) {
-  switch (regexp->type_tag()) {
+  switch (regexp->TypeTag()) {
     case JSRegExp::JSCRE:
       return JsreExec(regexp, subject, index);
     case JSRegExp::ATOM:
@@ -187,7 +218,7 @@ Handle<Object> RegExpImpl::Exec(Handle<JSRegExp> regexp,
 
 Handle<Object> RegExpImpl::ExecGlobal(Handle<JSRegExp> regexp,
                                 Handle<String> subject) {
-  switch (regexp->type_tag()) {
+  switch (regexp->TypeTag()) {
     case JSRegExp::JSCRE:
       return JsreExecGlobal(regexp, subject);
     case JSRegExp::ATOM:
@@ -200,9 +231,9 @@ Handle<Object> RegExpImpl::ExecGlobal(Handle<JSRegExp> regexp,
 
 
 Handle<Object> RegExpImpl::AtomCompile(Handle<JSRegExp> re,
-                                       Handle<String> pattern) {
-  re->set_type_tag(JSRegExp::ATOM);
-  re->set_data(*pattern);
+                                       Handle<String> pattern,
+                                       int flags) {
+  Factory::SetRegExpData(re, JSRegExp::ATOM, pattern, flags, pattern);
   return re;
 }
 
@@ -210,7 +241,7 @@ Handle<Object> RegExpImpl::AtomCompile(Handle<JSRegExp> re,
 Handle<Object> RegExpImpl::AtomExec(Handle<JSRegExp> re,
                                     Handle<String> subject,
                                     Handle<Object> index) {
-  Handle<String> needle(String::cast(re->data()));
+  Handle<String> needle(String::cast(re->DataAt(JSRegExp::kAtomPatternIndex)));
 
   uint32_t start_index;
   if (!Array::IndexFromObject(*index, &start_index)) {
@@ -234,7 +265,7 @@ Handle<Object> RegExpImpl::AtomExec(Handle<JSRegExp> re,
 
 Handle<Object> RegExpImpl::AtomExecGlobal(Handle<JSRegExp> re,
                                           Handle<String> subject) {
-  Handle<String> needle(String::cast(re->data()));
+  Handle<String> needle(String::cast(re->DataAt(JSRegExp::kAtomPatternIndex)));
   Handle<JSArray> result = Factory::NewJSArray(1);
   int index = 0;
   int match_count = 0;
@@ -269,14 +300,13 @@ Handle<Object> RegExpImpl::AtomExecGlobal(Handle<JSRegExp> re,
 
 Handle<Object> RegExpImpl::JsreCompile(Handle<JSRegExp> re,
                                        Handle<String> pattern,
-                                       Handle<String> flags) {
-  JSRegExpIgnoreCaseOption case_option = JSRegExpDoNotIgnoreCase;
-  JSRegExpMultilineOption multiline_option = JSRegExpSingleLine;
-  FlattenString(flags);
-  for (int i = 0; i < flags->length(); i++) {
-    if (flags->Get(i) == 'i') case_option = JSRegExpIgnoreCase;
-    if (flags->Get(i) == 'm') multiline_option = JSRegExpMultiline;
-  }
+                                       int flags) {
+  JSRegExpIgnoreCaseOption case_option = (flags & JSRegExp::IGNORE_CASE)
+    ? JSRegExpIgnoreCase
+    : JSRegExpDoNotIgnoreCase;
+  JSRegExpMultilineOption multiline_option = (flags & JSRegExp::MULTILINE)
+    ? JSRegExpMultiline
+    : JSRegExpSingleLine;
 
   Handle<String> two_byte_pattern = StringToTwoByte(pattern);
 
@@ -328,8 +358,7 @@ Handle<Object> RegExpImpl::JsreCompile(Handle<JSRegExp> re,
     Handle<FixedArray> value = Factory::NewFixedArray(2);
     value->set(CAPTURE_INDEX, Smi::FromInt(number_of_captures));
     value->set(INTERNAL_INDEX, *internal);
-    re->set_type_tag(JSRegExp::JSCRE);
-    re->set_data(*value);
+    Factory::SetRegExpData(re, JSRegExp::JSCRE, pattern, flags, value);
 
     return re;
   }
@@ -499,16 +528,14 @@ Handle<Object> RegExpImpl::JsreExecGlobal(Handle<JSRegExp> regexp,
 
 
 int RegExpImpl::JsreCapture(Handle<JSRegExp> re) {
-  Object* value = re->data();
-  ASSERT(value->IsFixedArray());
-  return Smi::cast(FixedArray::cast(value)->get(CAPTURE_INDEX))->value();
+  FixedArray* value = FixedArray::cast(re->DataAt(JSRegExp::kJscreDataIndex));
+  return Smi::cast(value->get(CAPTURE_INDEX))->value();
 }
 
 
 ByteArray* RegExpImpl::JsreInternal(Handle<JSRegExp> re) {
-  Object* value = re->data();
-  ASSERT(value->IsFixedArray());
-  return ByteArray::cast(FixedArray::cast(value)->get(INTERNAL_INDEX));
+  FixedArray* value = FixedArray::cast(re->DataAt(JSRegExp::kJscreDataIndex));
+  return ByteArray::cast(value->get(INTERNAL_INDEX));
 }
 
 }}  // namespace v8::internal