Filter out maps from different global context when collecting type feedback.
authorfschneider@chromium.org <fschneider@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Wed, 14 Dec 2011 14:01:54 +0000 (14:01 +0000)
committerfschneider@chromium.org <fschneider@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Wed, 14 Dec 2011 14:01:54 +0000 (14:01 +0000)
This avoid leaking memory because optimized code may hold on to maps
from different tabs otherwise.

BUG=v8:1823, 102895
Review URL: http://codereview.chromium.org/8892002

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

src/stub-cache.cc
src/stub-cache.h
src/type-info.cc
src/type-info.h
test/cctest/test-heap.cc

index 0d0105c..787e833 100644 (file)
@@ -877,7 +877,8 @@ void StubCache::Clear() {
 
 void StubCache::CollectMatchingMaps(SmallMapList* types,
                                     String* name,
-                                    Code::Flags flags) {
+                                    Code::Flags flags,
+                                    Handle<Context> global_context) {
   for (int i = 0; i < kPrimaryTableSize; i++) {
     if (primary_[i].key == name) {
       Map* map = primary_[i].value->FindFirstMap();
@@ -886,7 +887,8 @@ void StubCache::CollectMatchingMaps(SmallMapList* types,
       if (map == NULL) continue;
 
       int offset = PrimaryOffset(name, flags, map);
-      if (entry(primary_, offset) == &primary_[i]) {
+      if (entry(primary_, offset) == &primary_[i] &&
+          !TypeFeedbackOracle::CanRetainOtherContext(map, *global_context)) {
         types->Add(Handle<Map>(map));
       }
     }
@@ -909,7 +911,8 @@ void StubCache::CollectMatchingMaps(SmallMapList* types,
 
       // Lookup in secondary table and add matches.
       int offset = SecondaryOffset(name, flags, primary_offset);
-      if (entry(secondary_, offset) == &secondary_[i]) {
+      if (entry(secondary_, offset) == &secondary_[i] &&
+          !TypeFeedbackOracle::CanRetainOtherContext(map, *global_context)) {
         types->Add(Handle<Map>(map));
       }
     }
index f55a36d..720ad8b 100644 (file)
@@ -248,7 +248,8 @@ class StubCache {
   // Collect all maps that match the name and flags.
   void CollectMatchingMaps(SmallMapList* types,
                            String* name,
-                           Code::Flags flags);
+                           Code::Flags flags,
+                           Handle<Context> global_context);
 
   // Generate code for probing the stub cache table.
   // Arguments extra and extra2 may be used to pass additional scratch
index e722d14..af8a8ae 100644 (file)
@@ -438,11 +438,45 @@ void TypeFeedbackOracle::CollectReceiverTypes(unsigned ast_id,
       Handle<Code>::cast(object)->ic_state() == MEGAMORPHIC) {
     types->Reserve(4);
     ASSERT(object->IsCode());
-    isolate_->stub_cache()->CollectMatchingMaps(types, *name, flags);
+    isolate_->stub_cache()->CollectMatchingMaps(types,
+                                                *name,
+                                                flags,
+                                                global_context_);
   }
 }
 
 
+// Check if a map originates from a given global context. We use this
+// information to filter out maps from different context to avoid
+// retaining objects from different tabs in Chrome via optimized code.
+bool TypeFeedbackOracle::CanRetainOtherContext(Map* map,
+                                               Context* global_context) {
+  Object* constructor = map->constructor();
+  ASSERT(constructor != NULL);
+  while (!constructor->IsJSFunction()) {
+    // If the constructor is not null or a JSFunction, we have to
+    // conservatively assume that it may retain a global context.
+    if (!constructor->IsNull()) return true;
+
+    // If both, constructor and prototype are null, we conclude
+    // that no global context will be retained by this map.
+    if (map->prototype()->IsNull()) return false;
+
+    map = JSObject::cast(map->prototype())->map();
+    constructor = map->constructor();
+  }
+  JSFunction* function = JSFunction::cast(constructor);
+  return CanRetainOtherContext(function, global_context);
+}
+
+
+bool TypeFeedbackOracle::CanRetainOtherContext(JSFunction* function,
+                                               Context* global_context) {
+  return function->context()->global() != global_context->global()
+      && function->context()->global() != global_context->builtins();
+}
+
+
 static void AddMapIfMissing(Handle<Map> map, SmallMapList* list) {
   for (int i = 0; i < list->length(); ++i) {
     if (list->at(i).is_identical_to(map)) return;
@@ -539,7 +573,12 @@ void TypeFeedbackOracle::ProcessRelocInfos(ZoneList<RelocInfo>* infos) {
             SetInfo(ast_id, Smi::FromInt(target->check_type()));
           } else {
             Object* map = target->FindFirstMap();
-            SetInfo(ast_id, map == NULL ? static_cast<Object*>(target) : map);
+            if (map == NULL) {
+              SetInfo(ast_id, static_cast<Object*>(target));
+            } else if (!CanRetainOtherContext(Map::cast(map),
+                                              *global_context_)) {
+              SetInfo(ast_id, map);
+            }
           }
         } else if (target->ic_state() == MEGAMORPHIC) {
           SetInfo(ast_id, target);
@@ -565,7 +604,9 @@ void TypeFeedbackOracle::ProcessRelocInfos(ZoneList<RelocInfo>* infos) {
         if (target->major_key() == CodeStub::CallFunction &&
             target->has_function_cache()) {
           Object* value = CallFunctionStub::GetCachedValue(reloc_entry.pc());
-          if (value->IsJSFunction()) {
+          if (value->IsJSFunction() &&
+              !CanRetainOtherContext(JSFunction::cast(value),
+                                     *global_context_)) {
             SetInfo(ast_id, value);
           }
         }
index eba0987..1519fca 100644 (file)
@@ -256,6 +256,10 @@ class TypeFeedbackOracle BASE_EMBEDDED {
   void CollectKeyedReceiverTypes(unsigned ast_id,
                                  SmallMapList* types);
 
+  static bool CanRetainOtherContext(Map* map, Context* global_context);
+  static bool CanRetainOtherContext(JSFunction* function,
+                                    Context* global_context);
+
   CheckType GetCallCheckType(Call* expr);
   Handle<JSObject> GetPrototypeForPrimitiveCheck(CheckType check);
 
index 014eefb..0e09ee3 100644 (file)
@@ -1318,3 +1318,83 @@ TEST(IdleNotificationAdvancesIncrementalMarking) {
   intptr_t new_size = HEAP->SizeOfObjects();
   CHECK(no_idle_work || new_size < old_size);
 }
+
+
+static int NumberOfGlobalObjects() {
+  int count = 0;
+  HeapIterator iterator;
+  for (HeapObject* obj = iterator.next(); obj != NULL; obj = iterator.next()) {
+    if (obj->IsGlobalObject()) count++;
+  }
+  return count;
+}
+
+
+// Test that we don't embed maps from foreign contexts into
+// optimized code.
+TEST(LeakGlobalContextViaMap) {
+  v8::HandleScope outer_scope;
+  v8::Persistent<v8::Context> ctx1 = v8::Context::New();
+  v8::Persistent<v8::Context> ctx2 = v8::Context::New();
+  ctx1->Enter();
+
+  HEAP->CollectAllAvailableGarbage();
+  CHECK_EQ(4, NumberOfGlobalObjects());
+
+  {
+    v8::HandleScope inner_scope;
+    CompileRun("var v = {x: 42}");
+    v8::Local<v8::Value> v = ctx1->Global()->Get(v8_str("v"));
+    ctx2->Enter();
+    ctx2->Global()->Set(v8_str("o"), v);
+    v8::Local<v8::Value> res = CompileRun(
+        "function f() { return o.x; }"
+        "for (var i = 0; i < 1000000; ++i) f();"
+        "f();");
+    CHECK_EQ(42, res->Int32Value());
+    ctx2->Global()->Set(v8_str("o"), v8::Int32::New(0));
+    ctx2->Exit();
+    ctx1->Exit();
+    ctx1.Dispose();
+  }
+  HEAP->CollectAllAvailableGarbage();
+  CHECK_EQ(2, NumberOfGlobalObjects());
+  ctx2.Dispose();
+  HEAP->CollectAllAvailableGarbage();
+  CHECK_EQ(0, NumberOfGlobalObjects());
+}
+
+
+// Test that we don't embed functions from foreign contexts into
+// optimized code.
+TEST(LeakGlobalContextViaFunction) {
+  v8::HandleScope outer_scope;
+  v8::Persistent<v8::Context> ctx1 = v8::Context::New();
+  v8::Persistent<v8::Context> ctx2 = v8::Context::New();
+  ctx1->Enter();
+
+  HEAP->CollectAllAvailableGarbage();
+  CHECK_EQ(4, NumberOfGlobalObjects());
+
+  {
+    v8::HandleScope inner_scope;
+    CompileRun("var v = function() { return 42; }");
+    v8::Local<v8::Value> v = ctx1->Global()->Get(v8_str("v"));
+    ctx2->Enter();
+    ctx2->Global()->Set(v8_str("o"), v);
+    v8::Local<v8::Value> res = CompileRun(
+        "function f(x) { return x(); }"
+        "for (var i = 0; i < 1000000; ++i) f(o);"
+        "f(o);");
+    CHECK_EQ(42, res->Int32Value());
+    ctx2->Global()->Set(v8_str("o"), v8::Int32::New(0));
+    ctx2->Exit();
+    ctx1->Exit();
+    ctx1.Dispose();
+  }
+  HEAP->CollectAllAvailableGarbage();
+  CHECK_EQ(2, NumberOfGlobalObjects());
+  ctx2.Dispose();
+  HEAP->CollectAllAvailableGarbage();
+  CHECK_EQ(0, NumberOfGlobalObjects());
+}