- Revision
- 141999
- Author
- commit-qu...@webkit.org
- Date
- 2013-02-06 07:50:56 -0800 (Wed, 06 Feb 2013)
Log Message
Unreviewed, rolling out r141983.
http://trac.webkit.org/changeset/141983
https://bugs.webkit.org/show_bug.cgi?id=109055
lots of new crashes in handlescope (Requested by gavinp on
#webkit).
Patch by Sheriff Bot <webkit.review....@gmail.com> on 2013-02-06
* bindings/v8/DOMDataStore.h:
(WebCore::DOMDataStore::setWrapperInObject):
* bindings/v8/DOMWrapperWorld.h:
(DOMWrapperWorld):
(WebCore::DOMWrapperWorld::isolated):
* bindings/v8/V8Binding.h:
(WebCore):
(WebCore::worldForEnteredContextIfIsolated):
* bindings/v8/V8DOMWindowShell.cpp:
(WebCore::V8DOMWindowShell::initializeIfNeeded):
* bindings/v8/V8GCController.cpp:
(WebCore::WrapperVisitor::WrapperVisitor):
(WebCore):
(WebCore::gcTree):
(WebCore::V8GCController::didCreateWrapperForNode):
(WebCore::V8GCController::gcPrologue):
(WebCore::V8GCController::minorGCPrologue):
(WebCore::V8GCController::majorGCPrologue):
* bindings/v8/V8GCController.h:
(V8GCController):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (141998 => 141999)
--- trunk/Source/WebCore/ChangeLog 2013-02-06 14:43:55 UTC (rev 141998)
+++ trunk/Source/WebCore/ChangeLog 2013-02-06 15:50:56 UTC (rev 141999)
@@ -1,3 +1,33 @@
+2013-02-06 Sheriff Bot <webkit.review....@gmail.com>
+
+ Unreviewed, rolling out r141983.
+ http://trac.webkit.org/changeset/141983
+ https://bugs.webkit.org/show_bug.cgi?id=109055
+
+ lots of new crashes in handlescope (Requested by gavinp on
+ #webkit).
+
+ * bindings/v8/DOMDataStore.h:
+ (WebCore::DOMDataStore::setWrapperInObject):
+ * bindings/v8/DOMWrapperWorld.h:
+ (DOMWrapperWorld):
+ (WebCore::DOMWrapperWorld::isolated):
+ * bindings/v8/V8Binding.h:
+ (WebCore):
+ (WebCore::worldForEnteredContextIfIsolated):
+ * bindings/v8/V8DOMWindowShell.cpp:
+ (WebCore::V8DOMWindowShell::initializeIfNeeded):
+ * bindings/v8/V8GCController.cpp:
+ (WebCore::WrapperVisitor::WrapperVisitor):
+ (WebCore):
+ (WebCore::gcTree):
+ (WebCore::V8GCController::didCreateWrapperForNode):
+ (WebCore::V8GCController::gcPrologue):
+ (WebCore::V8GCController::minorGCPrologue):
+ (WebCore::V8GCController::majorGCPrologue):
+ * bindings/v8/V8GCController.h:
+ (V8GCController):
+
2013-02-06 Andreas Kling <akl...@apple.com>
Optimize GlyphPage for case where all glyphs are available in the same font.
Modified: trunk/Source/WebCore/bindings/v8/DOMDataStore.h (141998 => 141999)
--- trunk/Source/WebCore/bindings/v8/DOMDataStore.h 2013-02-06 14:43:55 UTC (rev 141998)
+++ trunk/Source/WebCore/bindings/v8/DOMDataStore.h 2013-02-06 15:50:56 UTC (rev 141999)
@@ -164,6 +164,13 @@
object->setWrapper(wrapper);
wrapper.MakeWeak(isolate, object, weakCallback);
}
+ static void setWrapperInObject(Node* object, v8::Persistent<v8::Object> wrapper, v8::Isolate* isolate)
+ {
+ ASSERT(object->wrapper().IsEmpty());
+ object->setWrapper(wrapper);
+ V8GCController::didCreateWrapperForNode(object);
+ wrapper.MakeWeak(isolate, static_cast<ScriptWrappable*>(object), weakCallback);
+ }
static void weakCallback(v8::Isolate*, v8::Persistent<v8::Value>, void* context);
Modified: trunk/Source/WebCore/bindings/v8/DOMWrapperWorld.h (141998 => 141999)
--- trunk/Source/WebCore/bindings/v8/DOMWrapperWorld.h 2013-02-06 14:43:55 UTC (rev 141998)
+++ trunk/Source/WebCore/bindings/v8/DOMWrapperWorld.h 2013-02-06 15:50:56 UTC (rev 141999)
@@ -63,7 +63,6 @@
#ifndef NDEBUG
static void assertContextHasCorrectPrototype(v8::Handle<v8::Context>);
#endif
- // FIXME: Rename this method to getWorld().
static DOMWrapperWorld* isolated(v8::Handle<v8::Context> context)
{
#ifndef NDEBUG
@@ -71,10 +70,6 @@
#endif
return static_cast<DOMWrapperWorld*>(context->GetAlignedPointerFromEmbedderData(v8ContextIsolatedWorld));
}
- static DOMWrapperWorld* getWorldWithoutContextCheck(v8::Handle<v8::Context> context)
- {
- return static_cast<DOMWrapperWorld*>(context->GetAlignedPointerFromEmbedderData(v8ContextIsolatedWorld));
- }
// Associates an isolated world (see above for description) with a security
// origin. XMLHttpRequest instances used in that world will be considered
Modified: trunk/Source/WebCore/bindings/v8/V8Binding.h (141998 => 141999)
--- trunk/Source/WebCore/bindings/v8/V8Binding.h 2013-02-06 14:43:55 UTC (rev 141998)
+++ trunk/Source/WebCore/bindings/v8/V8Binding.h 2013-02-06 15:50:56 UTC (rev 141999)
@@ -437,31 +437,13 @@
// a context, if the window is currently being displayed in the Frame.
Frame* toFrameIfNotDetached(v8::Handle<v8::Context>);
- // FIXME: Rename this method to worldForEnteredContext().
inline DOMWrapperWorld* worldForEnteredContextIfIsolated()
{
- v8::Handle<v8::Context> context = v8::Context::GetEntered();
- if (context.IsEmpty())
+ if (!v8::Context::InContext())
return 0;
- return DOMWrapperWorld::isolated(context);
+ return DOMWrapperWorld::isolated(v8::Context::GetEntered());
}
- // This is a slightly different version of worldForEnteredContext().
- // The difference is just that worldForEnteredContextWithoutContextCheck()
- // does not call assertContextHasCorrectPrototype() (which is enabled on
- // Debug builds only). Because assertContextHasCorrectPrototype() crashes
- // if it is called when a current context is not completely initialized,
- // you have to use worldForEnteredContextWithoutContextCheck() if you need
- // to get a DOMWrapperWorld while a current context is being initialized.
- // See https://bugs.webkit.org/show_bug.cgi?id=108579#c15 for more details.
- inline DOMWrapperWorld* worldForEnteredContextWithoutContextCheck()
- {
- v8::Handle<v8::Context> context = v8::Context::GetEntered();
- if (context.IsEmpty())
- return 0;
- return DOMWrapperWorld::getWorldWithoutContextCheck(context);
- }
-
// If the current context causes out of memory, _javascript_ setting
// is disabled and it returns true.
bool handleOutOfMemory();
Modified: trunk/Source/WebCore/bindings/v8/V8DOMWindowShell.cpp (141998 => 141999)
--- trunk/Source/WebCore/bindings/v8/V8DOMWindowShell.cpp 2013-02-06 14:43:55 UTC (rev 141998)
+++ trunk/Source/WebCore/bindings/v8/V8DOMWindowShell.cpp 2013-02-06 15:50:56 UTC (rev 141999)
@@ -208,13 +208,13 @@
if (m_context.isEmpty())
return false;
- m_world->setIsolatedWorldField(m_context.get());
-
bool isMainWorld = m_world->isMainWorld();
v8::Local<v8::Context> context = v8::Local<v8::Context>::New(m_context.get());
v8::Context::Scope contextScope(context);
+ m_world->setIsolatedWorldField(m_context.get());
+
if (m_global.isEmpty()) {
m_global.set(context->Global());
if (m_global.isEmpty()) {
Modified: trunk/Source/WebCore/bindings/v8/V8GCController.cpp (141998 => 141999)
--- trunk/Source/WebCore/bindings/v8/V8GCController.cpp 2013-02-06 14:43:55 UTC (rev 141998)
+++ trunk/Source/WebCore/bindings/v8/V8GCController.cpp 2013-02-06 15:50:56 UTC (rev 141999)
@@ -178,116 +178,18 @@
return node;
}
-static void gcTree(Node* startNode)
-{
- Vector<v8::Persistent<v8::Value>, initialNodeVectorSize> newSpaceWrappers;
-
- // We traverse a DOM tree in the DFS order starting from startNode.
- // The traversal order does not matter for correctness but does matter for performance.
- Node* node = startNode;
- // To make each minor GC time bounded, we might need to give up
- // traversing at some point for a large DOM tree. That being said,
- // I could not observe the need even in pathological test cases.
- do {
- ASSERT(node);
- if (!node->wrapper().IsEmpty()) {
- // FIXME: Remove the special handling for image elements.
- // The same special handling is in V8GCController::opaqueRootForGC().
- // Maybe should image elements be active DOM nodes?
- // See https://code.google.com/p/chromium/issues/detail?id=164882
- if (!node->isV8CollectableDuringMinorGC() || (node->hasTagName(HTMLNames::imgTag) && static_cast<HTMLImageElement*>(node)->hasPendingActivity())) {
- // This node is not in the new space of V8. This indicates that
- // the minor GC cannot anyway judge reachability of this DOM tree.
- // Thus we give up traversing the DOM tree.
- return;
- }
- node->setV8CollectableDuringMinorGC(false);
- newSpaceWrappers.append(node->wrapper());
- }
- if (node->firstChild()) {
- node = node->firstChild();
- continue;
- }
- while (!node->nextSibling()) {
- if (!node->parentNode())
- break;
- node = node->parentNode();
- }
- if (node->parentNode())
- node = node->nextSibling();
- } while (node != startNode);
-
- // We completed the DOM tree traversal. All wrappers in the DOM tree are
- // stored in newSpaceWrappers and are expected to exist in the new space of V8.
- // We report those wrappers to V8 as an object group.
- for (size_t i = 0; i < newSpaceWrappers.size(); i++)
- newSpaceWrappers[i].MarkPartiallyDependent();
- if (newSpaceWrappers.size() > 0)
- v8::V8::AddObjectGroup(&newSpaceWrappers[0], newSpaceWrappers.size());
-}
-
-// Regarding a minor GC algorithm for DOM nodes, see this document:
-// https://docs.google.com/a/google.com/presentation/d/1uifwVYGNYTZDoGLyCb7sXa7g49mWNMW2gaWvMN5NLk8/edit#slide=id.p
-class MinorGCWrapperVisitor : public v8::PersistentHandleVisitor {
+class WrapperVisitor : public v8::PersistentHandleVisitor {
public:
- explicit MinorGCWrapperVisitor(v8::Isolate* isolate)
+ explicit WrapperVisitor(v8::Isolate* isolate)
: m_isolate(isolate)
{
- UNUSED_PARAM(m_isolate);
}
virtual void VisitPersistentHandle(v8::Persistent<v8::Value> value, uint16_t classId) OVERRIDE
{
- // A minor DOM GC can collect only Nodes.
- if (classId != v8DOMNodeClassId)
- return;
-
- // To make minor GC cycle time bounded, we limit the number of wrappers handled
- // by each minor GC cycle to 10000. This value was selected so that the minor
- // GC cycle time is bounded to 20 ms in a case where the new space size
- // is 16 MB and it is full of wrappers (which is almost the worst case).
- // Practically speaking, as far as I crawled real web applications,
- // the number of wrappers handled by each minor GC cycle is at most 3000.
- // So this limit is mainly for pathological micro benchmarks.
- const unsigned wrappersHandledByEachMinorGC = 10000;
- if (m_nodesInNewSpace.size() >= wrappersHandledByEachMinorGC)
- return;
-
ASSERT(value->IsObject());
v8::Persistent<v8::Object> wrapper = v8::Persistent<v8::Object>::Cast(value);
- ASSERT(V8DOMWrapper::maybeDOMWrapper(value));
- ASSERT(V8Node::HasInstance(wrapper, m_isolate));
- Node* node = V8Node::toNative(wrapper);
- m_nodesInNewSpace.append(node);
- node->setV8CollectableDuringMinorGC(true);
- }
- void notifyFinished()
- {
- for (size_t i = 0; i < m_nodesInNewSpace.size(); i++) {
- ASSERT(!m_nodesInNewSpace[i]->wrapper().IsEmpty());
- if (m_nodesInNewSpace[i]->isV8CollectableDuringMinorGC()) // This branch is just for performance.
- gcTree(m_nodesInNewSpace[i]);
- }
- }
-
-private:
- Vector<Node*> m_nodesInNewSpace;
- v8::Isolate* m_isolate;
-};
-
-class MajorGCWrapperVisitor : public v8::PersistentHandleVisitor {
-public:
- explicit MajorGCWrapperVisitor(v8::Isolate* isolate)
- : m_isolate(isolate)
- {
- }
-
- virtual void VisitPersistentHandle(v8::Persistent<v8::Value> value, uint16_t classId) OVERRIDE
- {
- ASSERT(value->IsObject());
- v8::Persistent<v8::Object> wrapper = v8::Persistent<v8::Object>::Cast(value);
-
if (classId != v8DOMNodeClassId && classId != v8DOMObjectClassId)
return;
@@ -346,28 +248,105 @@
v8::Isolate* m_isolate;
};
+// Regarding a minor GC algorithm for DOM nodes, see this document:
+// https://docs.google.com/a/google.com/presentation/d/1uifwVYGNYTZDoGLyCb7sXa7g49mWNMW2gaWvMN5NLk8/edit#slide=id.p
+//
+// m_edenNodes stores nodes that have wrappers that have been created since the last minor/major GC.
+Vector<Node*>* V8GCController::m_edenNodes = 0;
+
+static void gcTree(Node* startNode)
+{
+ Vector<v8::Persistent<v8::Value>, initialNodeVectorSize> newSpaceWrappers;
+
+ // We traverse a DOM tree in the DFS order starting from startNode.
+ // The traversal order does not matter for correctness but does matter for performance.
+ Node* node = startNode;
+ // To make each minor GC time bounded, we might need to give up
+ // traversing at some point for a large DOM tree. That being said,
+ // I could not observe the need even in pathological test cases.
+ do {
+ ASSERT(node);
+ if (!node->wrapper().IsEmpty()) {
+ // FIXME: Remove the special handling for image elements.
+ // The same special handling is in V8GCController::opaqueRootForGC().
+ // Maybe should image elements be active DOM nodes?
+ // See https://code.google.com/p/chromium/issues/detail?id=164882
+ if (!node->isV8CollectableDuringMinorGC() || (node->hasTagName(HTMLNames::imgTag) && static_cast<HTMLImageElement*>(node)->hasPendingActivity())) {
+ // The fact that we encounter a node that is not in the Eden space
+ // implies that its wrapper might be in the old space of V8.
+ // This indicates that the minor GC cannot anyway judge reachability
+ // of this DOM tree. Thus we give up traversing the DOM tree.
+ return;
+ }
+ // A once traversed node is removed from the Eden space.
+ node->setV8CollectableDuringMinorGC(false);
+ newSpaceWrappers.append(node->wrapper());
+ }
+ if (node->firstChild()) {
+ node = node->firstChild();
+ continue;
+ }
+ while (!node->nextSibling()) {
+ if (!node->parentNode())
+ break;
+ node = node->parentNode();
+ }
+ if (node->parentNode())
+ node = node->nextSibling();
+ } while (node != startNode);
+
+ // We completed the DOM tree traversal. All wrappers in the DOM tree are
+ // stored in newSpaceWrappers and are expected to exist in the new space of V8.
+ // We report those wrappers to V8 as an object group.
+ for (size_t i = 0; i < newSpaceWrappers.size(); i++)
+ newSpaceWrappers[i].MarkPartiallyDependent();
+ if (newSpaceWrappers.size() > 0)
+ v8::V8::AddObjectGroup(&newSpaceWrappers[0], newSpaceWrappers.size());
+}
+
+void V8GCController::didCreateWrapperForNode(Node* node)
+{
+ // To make minor GC cycle time bounded, we limit the number of wrappers handled
+ // by each minor GC cycle to 10000. This value was selected so that the minor
+ // GC cycle time is bounded to 20 ms in a case where the new space size
+ // is 16 MB and it is full of wrappers (which is almost the worst case).
+ // Practically speaking, as far as I crawled real web applications,
+ // the number of wrappers handled by each minor GC cycle is at most 3000.
+ // So this limit is mainly for pathological micro benchmarks.
+ const unsigned wrappersHandledByEachMinorGC = 10000;
+ ASSERT(!node->wrapper().IsEmpty());
+ if (!m_edenNodes)
+ m_edenNodes = adoptPtr(new Vector<Node*>).leakPtr();
+ if (m_edenNodes->size() <= wrappersHandledByEachMinorGC) {
+ // A node of a newly created wrapper is put into the Eden space.
+ m_edenNodes->append(node);
+ node->setV8CollectableDuringMinorGC(true);
+ }
+}
+
void V8GCController::gcPrologue(v8::GCType type, v8::GCCallbackFlags flags)
{
if (type == v8::kGCTypeScavenge)
minorGCPrologue();
else if (type == v8::kGCTypeMarkSweepCompact)
majorGCPrologue();
+
+ if (isMainThreadOrGCThread() && m_edenNodes) {
+ // The Eden space is cleared at every minor/major GC.
+ m_edenNodes->clear();
+ }
}
void V8GCController::minorGCPrologue()
{
TRACE_EVENT_BEGIN0("v8", "GC");
- v8::Isolate* isolate = v8::Isolate::GetCurrent();
-
- // A minor GC can handle the main world only.
- DOMWrapperWorld* world = worldForEnteredContextWithoutContextCheck();
- if (world && world->isMainWorld()) {
- v8::HandleScope scope;
-
- MinorGCWrapperVisitor visitor(isolate);
- v8::V8::VisitHandlesForPartialDependence(isolate, &visitor);
- visitor.notifyFinished();
+ if (isMainThreadOrGCThread() && m_edenNodes) {
+ for (size_t i = 0; i < m_edenNodes->size(); i++) {
+ ASSERT(!m_edenNodes->at(i)->wrapper().IsEmpty());
+ if (m_edenNodes->at(i)->isV8CollectableDuringMinorGC()) // This branch is just for performance.
+ gcTree(m_edenNodes->at(i));
+ }
}
}
@@ -379,7 +358,7 @@
v8::Isolate* isolate = v8::Isolate::GetCurrent();
v8::HandleScope scope;
- MajorGCWrapperVisitor visitor(isolate);
+ WrapperVisitor visitor(isolate);
v8::V8::VisitHandlesWithClassIds(&visitor);
visitor.notifyFinished();
Modified: trunk/Source/WebCore/bindings/v8/V8GCController.h (141998 => 141999)
--- trunk/Source/WebCore/bindings/v8/V8GCController.h 2013-02-06 14:43:55 UTC (rev 141998)
+++ trunk/Source/WebCore/bindings/v8/V8GCController.h 2013-02-06 15:50:56 UTC (rev 141999)
@@ -52,6 +52,7 @@
static void collectGarbage();
static Node* opaqueRootForGC(Node*, v8::Isolate*);
+ static void didCreateWrapperForNode(Node*);
private:
static Vector<Node*>* m_edenNodes;
