Revision: 10277
Author: [email protected]
Date: Mon Dec 19 04:39:52 2011
Log: Fix bug with filtering of foreign context maps in the type
feedback.
The first attempt did not properly handle keyed loads/stores and
did not check the constructors of the objects in the prototype
chain.
Added two more tests to handle the fixed cases.
BUG=v8:1823
TEST=LeakGlobalObjectViaMapKeyed,LeakGlobalContextViaMapProto
Review URL: http://codereview.chromium.org/8974009
http://code.google.com/p/v8/source/detail?r=10277
Modified:
/branches/bleeding_edge/src/type-info.cc
/branches/bleeding_edge/test/cctest/cctest.status
/branches/bleeding_edge/test/cctest/test-heap.cc
=======================================
--- /branches/bleeding_edge/src/type-info.cc Wed Dec 14 06:01:54 2011
+++ /branches/bleeding_edge/src/type-info.cc Mon Dec 19 04:39:52 2011
@@ -85,7 +85,8 @@
return code->is_keyed_load_stub() &&
code->ic_state() == MONOMORPHIC &&
Code::ExtractTypeFromFlags(code->flags()) == NORMAL &&
- code->FindFirstMap() != NULL;
+ code->FindFirstMap() != NULL &&
+ !CanRetainOtherContext(code->FindFirstMap(), *global_context_);
}
return false;
}
@@ -111,7 +112,9 @@
Handle<Code> code = Handle<Code>::cast(map_or_code);
return code->is_keyed_store_stub() &&
code->ic_state() == MONOMORPHIC &&
- Code::ExtractTypeFromFlags(code->flags()) == NORMAL;
+ Code::ExtractTypeFromFlags(code->flags()) == NORMAL &&
+ code->FindFirstMap() != NULL &&
+ !CanRetainOtherContext(code->FindFirstMap(), *global_context_);
}
return false;
}
@@ -144,7 +147,9 @@
Handle<Code> code = Handle<Code>::cast(map_or_code);
Map* first_map = code->FindFirstMap();
ASSERT(first_map != NULL);
- return Handle<Map>(first_map);
+ return CanRetainOtherContext(first_map, *global_context_)
+ ? Handle<Map>::null()
+ : Handle<Map>(first_map);
}
return Handle<Map>::cast(map_or_code);
}
@@ -155,7 +160,11 @@
Handle<Object> map_or_code = GetInfo(expr->id());
if (map_or_code->IsCode()) {
Handle<Code> code = Handle<Code>::cast(map_or_code);
- return Handle<Map>(code->FindFirstMap());
+ Map* first_map = code->FindFirstMap();
+ ASSERT(first_map != NULL);
+ return CanRetainOtherContext(first_map, *global_context_)
+ ? Handle<Map>::null()
+ : Handle<Map>(first_map);
}
return Handle<Map>::cast(map_or_code);
}
@@ -288,7 +297,11 @@
if (state != CompareIC::KNOWN_OBJECTS) {
return Handle<Map>::null();
}
- return Handle<Map>(code->FindFirstMap());
+ Map* first_map = code->FindFirstMap();
+ ASSERT(first_map != NULL);
+ return CanRetainOtherContext(first_map, *global_context_)
+ ? Handle<Map>::null()
+ : Handle<Map>(first_map);
}
@@ -451,20 +464,23 @@
// 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();
+ Object* constructor = NULL;
+ while (!map->prototype()->IsNull()) {
constructor = map->constructor();
- }
+ if (!constructor->IsNull()) {
+ // If the constructor is not null or a JSFunction, we have to
+ // conservatively assume that it may retain a global context.
+ if (!constructor->IsJSFunction()) return true;
+ // Check if the constructor directly references a foreign context.
+ if (CanRetainOtherContext(JSFunction::cast(constructor),
+ global_context)) {
+ return true;
+ }
+ }
+ map = HeapObject::cast(map->prototype())->map();
+ }
+ constructor = map->constructor();
+ if (constructor->IsNull()) return false;
JSFunction* function = JSFunction::cast(constructor);
return CanRetainOtherContext(function, global_context);
}
@@ -498,7 +514,10 @@
RelocInfo* info = it.rinfo();
Object* object = info->target_object();
if (object->IsMap()) {
- AddMapIfMissing(Handle<Map>(Map::cast(object)), types);
+ Map* map = Map::cast(object);
+ if (!CanRetainOtherContext(map, *global_context_)) {
+ AddMapIfMissing(Handle<Map>(map), types);
+ }
}
}
}
=======================================
--- /branches/bleeding_edge/test/cctest/cctest.status Thu Dec 15 02:08:25
2011
+++ /branches/bleeding_edge/test/cctest/cctest.status Mon Dec 19 04:39:52
2011
@@ -52,10 +52,6 @@
# We do not yet shrink weak maps after they have been emptied by the GC
test-weakmaps/Shrinking: FAIL
-# TODO(1823): Fails without snapshot. Temporarily disabled until fixed.
-test-heap/LeakGlobalContextViaMap: SKIP
-test-heap/LeakGlobalContextViaFunction: SKIP
-
##############################################################################
[ $arch == arm ]
=======================================
--- /branches/bleeding_edge/test/cctest/test-heap.cc Wed Dec 14 06:01:54
2011
+++ /branches/bleeding_edge/test/cctest/test-heap.cc Mon Dec 19 04:39:52
2011
@@ -1333,6 +1333,7 @@
// Test that we don't embed maps from foreign contexts into
// optimized code.
TEST(LeakGlobalContextViaMap) {
+ i::FLAG_allow_natives_syntax = true;
v8::HandleScope outer_scope;
v8::Persistent<v8::Context> ctx1 = v8::Context::New();
v8::Persistent<v8::Context> ctx2 = v8::Context::New();
@@ -1349,7 +1350,8 @@
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();"
+ "for (var i = 0; i < 10; ++i) f();"
+ "%OptimizeFunctionOnNextCall(f);"
"f();");
CHECK_EQ(42, res->Int32Value());
ctx2->Global()->Set(v8_str("o"), v8::Int32::New(0));
@@ -1368,6 +1370,7 @@
// Test that we don't embed functions from foreign contexts into
// optimized code.
TEST(LeakGlobalContextViaFunction) {
+ i::FLAG_allow_natives_syntax = true;
v8::HandleScope outer_scope;
v8::Persistent<v8::Context> ctx1 = v8::Context::New();
v8::Persistent<v8::Context> ctx2 = v8::Context::New();
@@ -1384,7 +1387,8 @@
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);"
+ "for (var i = 0; i < 10; ++i) f(o);"
+ "%OptimizeFunctionOnNextCall(f);"
"f(o);");
CHECK_EQ(42, res->Int32Value());
ctx2->Global()->Set(v8_str("o"), v8::Int32::New(0));
@@ -1398,3 +1402,77 @@
HEAP->CollectAllAvailableGarbage();
CHECK_EQ(0, NumberOfGlobalObjects());
}
+
+
+TEST(LeakGlobalContextViaMapKeyed) {
+ i::FLAG_allow_natives_syntax = true;
+ 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 = [42, 43]");
+ 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[0]; }"
+ "for (var i = 0; i < 10; ++i) f();"
+ "%OptimizeFunctionOnNextCall(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(LeakGlobalContextViaMapProto) {
+ i::FLAG_allow_natives_syntax = true;
+ 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 = { y: 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() {"
+ " var p = {x: 42};"
+ " p.__proto__ = o;"
+ " return p.x;"
+ "}"
+ "for (var i = 0; i < 10; ++i) f();"
+ "%OptimizeFunctionOnNextCall(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());
+}
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
