Recursion limit for one-char string replace and retire String::kMinNonFlatLength.

src/runtime.cc

Index: src/runtime.cc
diff --git a/src/runtime.cc b/src/runtime.cc
index c778c32231f7cc87710727a40d974e1edc460134..39e8ca3fafbaf40381bdab12d8a4de2d24f1123e 100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -3237,26 +3237,34 @@ Handle<String> Runtime::StringReplaceOneCharWithString(Isolate* isolate,
                                                        Handle<String> subject,
                                                        Handle<String> search,
                                                        Handle<String> replace,
-                                                       bool* found) {
+                                                       bool* found,
+                                                       int recursion_limit) {
+  if (recursion_limit == 0) return Handle<String>::null();
   if (subject->IsConsString()) {
     ConsString* cons = ConsString::cast(*subject);
     Handle<String> first = Handle<String>(cons->first());
     Handle<String> second = Handle<String>(cons->second());
-    Handle<String> new_first = StringReplaceOneCharWithString(isolate,
-                                                              first,
-                                                              search,
-                                                              replace,
-                                                              found);
-    if (*found) {
-      return isolate->factory()->NewConsString(new_first, second);
-    } else {
-      Handle<String> new_second = StringReplaceOneCharWithString(isolate,
-                                                                 second,
-                                                                 search,
-                                                                 replace,
-                                                                 found);
-      return isolate->factory()->NewConsString(first, new_second);
-    }
+    Handle<String> new_first =
+        StringReplaceOneCharWithString(isolate,
+                                       first,
+                                       search,
+                                       replace,
+                                       found,
+                                       recursion_limit - 1);
+    if (*found) return isolate->factory()->NewConsString(new_first, second);
+    if (new_first.is_null()) return new_first;
+
+    Handle<String> new_second =
+        StringReplaceOneCharWithString(isolate,
+                                       second,
+                                       search,
+                                       replace,
+                                       found,
+                                       recursion_limit - 1);
+    if (*found) return isolate->factory()->NewConsString(first, new_second);
+    if (new_second.is_null()) return new_second;
+
+    return subject;
   } else {
     int index = StringMatch(isolate, subject, search, 0);
     if (index == -1) return subject;
@@ -3276,13 +3284,24 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_StringReplaceOneCharWithString) {
   CONVERT_ARG_CHECKED(String, subject, 0);
   CONVERT_ARG_CHECKED(String, search, 1);
   CONVERT_ARG_CHECKED(String, replace, 2);
-  bool found = false;
 
-  return *(Runtime::StringReplaceOneCharWithString(isolate,
-                                                   subject,
-                                                   search,
-                                                   replace,
-                                                   &found));
+  // If the cons string tree is too deep, we simply abort the recursion and
+  // retry with a flattened subject string.
+  const int kRecursionLimit = 0x1000;

[Yang, 2012/01/17 13:07:34]

Recursion limit is chosen so that regexpdna still benefits from this fast path.

+  bool found = false;
+  for (int i = 0; i < 2; i++) {

[Erik Corry, 2012/01/17 13:50:24]

I think this would be clearer without the loop, just duplicating the call.

[Yang, 2012/01/17 14:29:04]

Done.

+    Handle<String> result =
+        Runtime::StringReplaceOneCharWithString(isolate,
+                                                subject,
+                                                search,
+                                                replace,
+                                                &found,
+                                                kRecursionLimit);
+    if (!result.is_null()) return *result;

[Erik Corry, 2012/01/17 13:51:21]

Should you have a test for found == false here and then return the original
string?

[Yang, 2012/01/17 14:29:04]

Allocation is only ever done if found==true. In all other cases, either a
null-handle or the original subject string is returned, so there is no need for
an additional check.

+    subject = FlattenGetString(subject);
+  }
+  UNREACHABLE();
+  return NULL;
 }