Changeset 28106 in webkit for trunk/JavaScriptCore/kjs/function.cpp

Timestamp:

Nov 28, 2007, 2:08:09 AM (17 years ago)

Author:

[email protected]

Message:

JavaScriptCore:

Reviewed by Darin and Geoff.

• Fixed "Stack overflow crash in JavaScript garbage collector mark pass" ​http://bugs.webkit.org/show_bug.cgi?id=12216

Implement mark stack. This version is not suitable for prime time because it makes a
huge allocation on every collect, and potentially makes marking of detached subtrees
slow. But it is an 0.4% SunSpider speedup even without much tweaking.

The basic approach is to replace mark() methods with
markChildren(MarkStack&) methods. Reachable references are pushed
onto a mark stack (which encapsulates ignoring already-marked
references).

Objects are no longer responsible for actually setting their own
mark bits, the collector does that. This means that for objects on
the number heap we don't have to call markChildren() at all since
we know there aren't any.

The mark phase of collect pushes roots onto the mark stack
and drains it as often as possible.

To make this approach viable requires a constant-size mark stack
and a slow fallback approach for when the stack size is exceeded,
plus optimizations to make the required stack small in common
cases. This should be doable.

• JavaScriptCore.exp: Export new symbols.
• JavaScriptCore.xcodeproj/project.pbxproj: Add new file.
• kjs/collector.cpp: (KJS::Collector::heapAllocate): (KJS::drainMarkStack): Helper for all of the below. (KJS::Collector::markStackObjectsConservatively): Use mark stack. (KJS::Collector::markCurrentThreadConservatively): ditto (KJS::Collector::markOtherThreadConservatively): ditto (KJS::Collector::markProtectedObjects): ditto (KJS::Collector::markMainThreadOnlyObjects): ditto (KJS::Collector::collect): ditto
• kjs/collector.h: (KJS::Collector::cellMayHaveRefs): Helper for MarkStack.

• kjs/MarkStack.h: Added. The actual mark stack implementation. (KJS::MarkStack::push): (KJS::MarkStack::pushAtom): (KJS::MarkStack::pop): (KJS::MarkStack::isEmpty): (KJS::MarkStack::reserveCapacity):

Changed mark() methods to markChildren() methods:

• kjs/ExecState.cpp: (KJS::ExecState::markChildren):
• kjs/ExecState.h:
• kjs/JSWrapperObject.cpp: (KJS::JSWrapperObject::markChildren):
• kjs/JSWrapperObject.h:
• kjs/array_instance.cpp: (KJS::ArrayInstance::markChildren):
• kjs/array_instance.h:
• kjs/bool_object.cpp: (BooleanInstance::markChildren):
• kjs/bool_object.h:
• kjs/error_object.cpp:
• kjs/error_object.h:
• kjs/function.cpp: (KJS::FunctionImp::markChildren): (KJS::Arguments::Arguments): (KJS::Arguments::markChildren): (KJS::ActivationImp::markChildren):
• kjs/function.h:
• kjs/internal.cpp: (KJS::GetterSetterImp::markChildren):
• kjs/interpreter.cpp: (KJS::Interpreter::markRoots):
• kjs/interpreter.h:
• kjs/list.cpp: (KJS::List::markProtectedListsSlowCase):
• kjs/list.h: (KJS::List::markProtectedLists):
• kjs/object.cpp: (KJS::JSObject::markChildren):
• kjs/object.h: (KJS::ScopeChain::markChildren):
• kjs/property_map.cpp: (KJS::PropertyMap::markChildren):
• kjs/property_map.h:
• kjs/scope_chain.h:
• kjs/string_object.cpp: (KJS::StringInstance::markChildren):
• kjs/string_object.h:

JavaScriptGlue:

Reviewed by Darin and Geoff.

Fixups for JavaScriptCore mark stack.

• JSObject.cpp: (JSUserObject::Mark):
• JSObject.h:
• JSValueWrapper.cpp: (JSValueWrapper::JSObjectMark):
• JSValueWrapper.h:
• UserObjectImp.cpp:
• UserObjectImp.h:

WebCore:

Reviewed by Darin and Geoff.

Implement mark stack. This version is not suitable for prime time because it makes a
huge allocation on every collect, and potentially makes marking of detached subtrees
slow. But it is a .2% - .4% speedup even without much tweaking.

I replaced mark() methods with markChildren() as usual. One
optimization that is lost is avoiding walking detached DOM
subtrees more than once to mark them; since marking is not
recursive there's no obvious way to bracket operation on the tree
any more.

• bindings/js/JSDocumentCustom.cpp: (WebCore::JSDocument::markChildren):
• bindings/js/JSNodeCustom.cpp: (WebCore::JSNode::markChildren):
• bindings/js/JSNodeFilterCondition.cpp:
• bindings/js/JSNodeFilterCondition.h:
• bindings/js/JSNodeFilterCustom.cpp: (WebCore::JSNodeFilter::markChildren):
• bindings/js/JSNodeIteratorCustom.cpp: (WebCore::JSNodeIterator::markChildren):
• bindings/js/JSTreeWalkerCustom.cpp: (WebCore::JSTreeWalker::markChildren):
• bindings/js/JSXMLHttpRequest.cpp: (KJS::JSXMLHttpRequest::markChildren):
• bindings/js/JSXMLHttpRequest.h:
• bindings/js/kjs_binding.cpp: (KJS::ScriptInterpreter::markDOMNodesForDocument):
• bindings/js/kjs_binding.h:
• bindings/js/kjs_events.cpp: (WebCore::JSUnprotectedEventListener::markChildren):
• bindings/js/kjs_events.h:
• bindings/js/kjs_window.cpp: (KJS::Window::markChildren):
• bindings/js/kjs_window.h:
• bindings/scripts/CodeGeneratorJS.pm:
• dom/Node.cpp: (WebCore::Node::Node):
• dom/Node.h:
• dom/NodeFilter.h:
• dom/NodeFilterCondition.h:

LayoutTests:

Not reviewed.

• Test cases for "Stack overflow crash in JavaScript garbage collector mark pass" ​http://bugs.webkit.org/show_bug.cgi?id=12216

I have fixed this with the mark stack work.

• fast/js/gc-breadth-2-expected.txt: Added.
• fast/js/gc-breadth-2.html: Added.
• fast/js/gc-breadth-expected.txt: Added.
• fast/js/gc-breadth.html: Added.
• fast/js/gc-depth-expected.txt: Added.
• fast/js/gc-depth.html: Added.
• fast/js/resources/gc-breadth-2.js: Added.
• fast/js/resources/gc-breadth.js: Added.
• fast/js/resources/gc-depth.js: Added.

File:

• trunk/JavaScriptCore/kjs/function.cpp (modified) (4 diffs)

trunk/JavaScriptCore/kjs/function.cpp

@@ -62 +62 @@
 }
 
-void FunctionImp::mark()
-{
-    InternalFunctionImp::mark();
-    _scope.mark();
+void FunctionImp::markChildren(MarkStack& stack)
+{
+    InternalFunctionImp::markChildren(stack);
+    _scope.markChildren(stack);
 }
 
@@ -332 +332 @@
 // ECMA 10.1.8
 Arguments::Arguments(ExecState* exec, FunctionImp* func, const List& args, ActivationImp* act)
-: JSObject(exec->lexicalInterpreter()->builtinObjectPrototype()), 
-_activationObject(act),
-indexToNameMap(func, args)
+  : JSObject(exec->lexicalInterpreter()->builtinObjectPrototype()) 
+  , _activationObject(act)
+  , indexToNameMap(func, args)
 {
   putDirect(exec->propertyNames().callee, func, DontEnum);
@@ -348 +348 @@
 }
 
-void Arguments::mark() 
-{
-  JSObject::mark();
-  if (_activationObject && !_activationObject->marked())
-    _activationObject->mark();
+void Arguments::markChildren(MarkStack& stack) 
+{
+  JSObject::markChildren(stack);
+  stack.push(_activationObject);
 }
 
@@ -485 +484 @@
 }
 
-void ActivationImp::mark()
-{
-    JSObject::mark();
+void ActivationImp::markChildren(MarkStack& stack)
+{
+    JSObject::markChildren(stack);
 
     size_t size = d->localStorage.size();
-    for (size_t i = 0; i < size; ++i) {
-        JSValue* value = d->localStorage[i].value;
-        if (!value->marked())
-            value->mark();
-    }
+    for (size_t i = 0; i < size; ++i)
+        stack.push(d->localStorage[i].value);
     
-    ASSERT(d->function);
-    if (!d->function->marked())
-        d->function->mark();
-
-    if (d->argumentsObject && !d->argumentsObject->marked())
-        d->argumentsObject->mark();
+    stack.push(d->function);
+    if (d->argumentsObject)
+      stack.push(d->argumentsObject);
 }