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

src/runtime.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 3219 matching lines @@
                                        regexp,
                                        replacement,
                                        last_match_info);
 }
 
 
 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;
     *found = true;
     Handle<String> first = isolate->factory()->NewSubString(subject, 0, index);
     Handle<String> cons1 = isolate->factory()->NewConsString(first, replace);
     Handle<String> second =
         isolate->factory()->NewSubString(subject, index + 1, subject->length());
     return isolate->factory()->NewConsString(cons1, second);
   }
 }
 
 
 RUNTIME_FUNCTION(MaybeObject*, Runtime_StringReplaceOneCharWithString) {
   ASSERT(args.length() == 3);
   HandleScope scope(isolate);
   CONVERT_ARG_CHECKED(String, subject, 0);
   CONVERT_ARG_CHECKED(String, search, 1);
   CONVERT_ARG_CHECKED(String, replace, 2);
+
+  // 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;
-
-  return *(Runtime::StringReplaceOneCharWithString(isolate,
-                                                   subject,
-                                                   search,
-                                                   replace,
-                                                   &found));
+  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;
 }
 
 
 // Perform string match of pattern on subject, starting at start index.
 // Caller must ensure that 0 <= start_index <= sub->length(),
 // and should check that pat->length() + start_index <= sub->length().
 int Runtime::StringMatch(Isolate* isolate,
                          Handle<String> sub,
                          Handle<String> pat,
                          int start_index) {
@@ skipping 10309 matching lines @@
   } else {
     // Handle last resort GC and make sure to allow future allocations
     // to grow the heap without causing GCs (if possible).
     isolate->counters()->gc_last_resort_from_js()->Increment();
     isolate->heap()->CollectAllGarbage(Heap::kNoGCFlags);
   }
 }
 
 
 } }  // namespace v8::internal