Fix 3 places where we were doing retry after GC without using the macros that

were defined for that purpose.
Review URL: http://codereview.chromium.org/8734

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@650 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/builtins.cc b/src/builtins.cc
index 9309c1c..fdff2d2 100644 (file)
--- a/src/builtins.cc
+++ b/src/builtins.cc
@@ -642,13 +642,12 @@ void Builtins::Setup(bool create_heap_objects) {
       CodeDesc desc;
       masm.GetCode(&desc);
       Code::Flags flags =  functions[i].flags;
-      Object* code = Heap::CreateCode(desc, NULL, flags);
-      if (code->IsFailure()) {
-        if (code->IsRetryAfterGC()) {
-          CHECK(Heap::CollectGarbage(Failure::cast(code)->requested(),
-                                     Failure::cast(code)->allocation_space()));
-          code = Heap::CreateCode(desc, NULL, flags);
-        }
+      Object* code;
+      {
+        // During startup it's OK to always allocate and defer GC to later.
+        // This simplifies things because we don't need to retry.
+        AlwaysAllocateScope __scope__;
+        code = Heap::CreateCode(desc, NULL, flags);
         if (code->IsFailure()) {
           v8::internal::V8::FatalProcessOutOfMemory("CreateCode");
         }

diff --git a/src/factory.cc b/src/factory.cc
index 0adf258..4b0faef 100644 (file)
--- a/src/factory.cc
+++ b/src/factory.cc
@@ -452,6 +452,7 @@ Handle<JSFunction> Factory::NewFunctionWithPrototype(Handle<String> name,
   return function;
 }
 
+
 Handle<Code> Factory::NewCode(const CodeDesc& desc, ScopeInfo<>* sinfo,
                               Code::Flags flags) {
   CALL_HEAP_FUNCTION(Heap::CreateCode(desc, sinfo, flags), Code);
@@ -463,42 +464,26 @@ Handle<Code> Factory::CopyCode(Handle<Code> code) {
 }
 
 
-#define CALL_GC(RETRY)                                                     \
-  do {                                                                     \
-    if (!Heap::CollectGarbage(Failure::cast(RETRY)->requested(),           \
-                              Failure::cast(RETRY)->allocation_space())) { \
-      /* TODO(1181417): Fix this. */                                       \
-      V8::FatalProcessOutOfMemory("Factory CALL_GC");                      \
-    }                                                                      \
-  } while (false)
+static inline Object* DoCopyInsert(DescriptorArray* array,
+                                   String* key,
+                                   Object* value,
+                                   PropertyAttributes attributes) {
+  CallbacksDescriptor desc(key, value, attributes);
+  Object* obj = array->CopyInsert(&desc, REMOVE_TRANSITIONS);
+  return obj;
+}
 
 
-// Allocate the new array. We cannot use the CALL_HEAP_FUNCTION macro here,
-// because the stack-allocated CallbacksDescriptor instance is not GC safe.
+// Allocate the new array.
 Handle<DescriptorArray> Factory::CopyAppendProxyDescriptor(
     Handle<DescriptorArray> array,
     Handle<String> key,
     Handle<Object> value,
     PropertyAttributes attributes) {
-  GC_GREEDY_CHECK();
-  CallbacksDescriptor desc(*key, *value, attributes);
-  Object* obj = array->CopyInsert(&desc, REMOVE_TRANSITIONS);
-  if (obj->IsFailure()) {
-    if (obj->IsRetryAfterGC()) {
-      CALL_GC(obj);
-      CallbacksDescriptor desc(*key, *value, attributes);
-      obj = array->CopyInsert(&desc, REMOVE_TRANSITIONS);
-    }
-    if (obj->IsFailure()) {
-      // TODO(1181417): Fix this.
-      V8::FatalProcessOutOfMemory("CopyAppendProxyDescriptor");
-    }
-  }
-  return Handle<DescriptorArray>(DescriptorArray::cast(obj));
+  CALL_HEAP_FUNCTION(DoCopyInsert(*array, *key, *value, attributes),
+                     DescriptorArray);
 }
 
-#undef CALL_GC
-
 
 Handle<String> Factory::SymbolFromString(Handle<String> value) {
   CALL_HEAP_FUNCTION(Heap::LookupSymbol(*value), String);

diff --git a/src/jsregexp.cc b/src/jsregexp.cc
index 02b53e0..115ff27 100644 (file)
--- a/src/jsregexp.cc
+++ b/src/jsregexp.cc
@@ -307,15 +307,55 @@ Handle<Object> RegExpImpl::AtomExecGlobal(Handle<JSRegExp> re,
 }
 
 
-Handle<Object> RegExpImpl::JsreCompile(Handle<JSRegExp> re,
-                                       Handle<String> pattern,
-                                       JSRegExp::Flags flags) {
+static inline Object* DoCompile(String* pattern,
+                                JSRegExp::Flags flags,
+                                unsigned* number_of_captures,
+                                const char** error_message,
+                                JscreRegExp** code) {
   JSRegExpIgnoreCaseOption case_option = flags.is_ignore_case()
     ? JSRegExpIgnoreCase
     : JSRegExpDoNotIgnoreCase;
   JSRegExpMultilineOption multiline_option = flags.is_multiline()
     ? JSRegExpMultiline
     : JSRegExpSingleLine;
+  *error_message = NULL;
+  malloc_failure = Failure::Exception();
+  *code = jsRegExpCompile(pattern->GetTwoByteData(),
+                          pattern->length(),
+                          case_option,
+                          multiline_option,
+                          number_of_captures,
+                          error_message,
+                          &JSREMalloc,
+                          &JSREFree);
+  if (code == NULL && (malloc_failure->IsRetryAfterGC() ||
+                       malloc_failure->IsOutOfMemoryFailure())) {
+    return malloc_failure;
+  } else {
+    // It doesn't matter which object we return here, we just need to return
+    // a non-failure to indicate to the GC-retry code that there was no
+    // allocation failure.
+    return pattern;
+  }
+}
+
+
+void CompileWithRetryAfterGC(Handle<String> pattern,
+                             JSRegExp::Flags flags,
+                             unsigned* number_of_captures,
+                             const char** error_message,
+                             JscreRegExp** code) {
+  CALL_HEAP_FUNCTION_VOID(DoCompile(*pattern,
+                                    flags,
+                                    number_of_captures,
+                                    error_message,
+                                    code));
+}
+
+
+Handle<Object> RegExpImpl::JsreCompile(Handle<JSRegExp> re,
+                                       Handle<String> pattern,
+                                       JSRegExp::Flags flags) {
 
   Handle<String> two_byte_pattern = StringToTwoByte(pattern);
 
@@ -325,52 +365,33 @@ Handle<Object> RegExpImpl::JsreCompile(Handle<JSRegExp> re,
   JscreRegExp* code = NULL;
   FlattenString(pattern);
 
-  bool first_time = true;
+  CompileWithRetryAfterGC(two_byte_pattern,
+                          flags,
+                          &number_of_captures,
+                          &error_message,
+                          &code);
 
-  while (true) {
-    malloc_failure = Failure::Exception();
-    code = jsRegExpCompile(two_byte_pattern->GetTwoByteData(),
-                           pattern->length(), case_option,
-                           multiline_option, &number_of_captures,
-                           &error_message, &JSREMalloc, &JSREFree);
-    if (code == NULL) {
-      if (first_time && malloc_failure->IsRetryAfterGC()) {
-        first_time = false;
-        if (!Heap::CollectGarbage(malloc_failure->requested(),
-                                  malloc_failure->allocation_space())) {
-          // TODO(1181417): Fix this.
-          V8::FatalProcessOutOfMemory("RegExpImpl::JsreCompile");
-        }
-        continue;
-      }
-      if (malloc_failure->IsRetryAfterGC() ||
-          malloc_failure->IsOutOfMemoryFailure()) {
-        // TODO(1181417): Fix this.
-        V8::FatalProcessOutOfMemory("RegExpImpl::JsreCompile");
-      } else {
-        // Throw an exception.
-        Handle<JSArray> array = Factory::NewJSArray(2);
-        SetElement(array, 0, pattern);
-        SetElement(array, 1, Factory::NewStringFromUtf8(CStrVector(
-            (error_message == NULL) ? "Unknown regexp error" : error_message)));
-        Handle<Object> regexp_err =
-            Factory::NewSyntaxError("malformed_regexp", array);
-        return Handle<Object>(Top::Throw(*regexp_err));
-      }
-    }
+  if (code == NULL) {
+    // Throw an exception.
+    Handle<JSArray> array = Factory::NewJSArray(2);
+    SetElement(array, 0, pattern);
+    SetElement(array, 1, Factory::NewStringFromUtf8(CStrVector(
+        (error_message == NULL) ? "Unknown regexp error" : error_message)));
+    Handle<Object> regexp_err =
+        Factory::NewSyntaxError("malformed_regexp", array);
+    return Handle<Object>(Top::Throw(*regexp_err));
+  }
 
-    ASSERT(code != NULL);
-    // Convert the return address to a ByteArray pointer.
-    Handle<ByteArray> internal(
-        ByteArray::FromDataStartAddress(reinterpret_cast<Address>(code)));
+  // Convert the return address to a ByteArray pointer.
+  Handle<ByteArray> internal(
+      ByteArray::FromDataStartAddress(reinterpret_cast<Address>(code)));
 
-    Handle<FixedArray> value = Factory::NewFixedArray(2);
-    value->set(CAPTURE_INDEX, Smi::FromInt(number_of_captures));
-    value->set(INTERNAL_INDEX, *internal);
-    Factory::SetRegExpData(re, JSRegExp::JSCRE, pattern, flags, value);
+  Handle<FixedArray> value = Factory::NewFixedArray(2);
+  value->set(CAPTURE_INDEX, Smi::FromInt(number_of_captures));
+  value->set(INTERNAL_INDEX, *internal);
+  Factory::SetRegExpData(re, JSRegExp::JSCRE, pattern, flags, value);
 
-    return re;
-  }
+  return re;
 }