Reviewers: antonm,
Message:
Anton,
This is a quick fix I needed while debugging window.open memory leak
reported by
Pavel (I don't know the bug number). In short, repeatedly opening/closing
gmail
window using window.open/close leaks memory. I don't think the result cache
is
the only thing to blame here, but fixing it helps with heap tracing. I'm
sure
there're better ways to fix it.
I'm also working on adding debug-mode checks for this.
Thanks,
Vitaly
Description:
Don't share function result caches between contexts.
A reference to the caches array was embedded directly into the builtin
code and this allowed sharing objects between contexts.
Unfortunately, clearing the cache on GC won't prevent sharing so we
either have to have per-context builtin code or load the cache
indirectly from the current context. This change implements the second
approach. The first approach may be interesting to consider in the
future for some perfomance critical functions, and the current
approach can still be improved by putting the caches directly into the
global context (or even global objects).
Please review this at http://codereview.chromium.org/1731002/show
Affected files:
M src/arm/codegen-arm.cc
M src/ia32/codegen-ia32.cc
M src/x64/codegen-x64.cc
Index: src/arm/codegen-arm.cc
diff --git a/src/arm/codegen-arm.cc b/src/arm/codegen-arm.cc
index
b2fe05f5a3acaffc3742669cd025865cdc42c897..909b6b944285bd6fe7752a48e22c35f5b214cf87
100644
--- a/src/arm/codegen-arm.cc
+++ b/src/arm/codegen-arm.cc
@@ -4172,18 +4172,21 @@ void
CodeGenerator::GenerateGetFromCache(ZoneList<Expression*>* args) {
frame_->EmitPush(r0);
return;
}
- Handle<FixedArray> cache_obj(
- FixedArray::cast(jsfunction_result_caches->get(cache_id)));
Load(args->at(1));
frame_->EmitPop(r2);
+ __ ldr(r1, MemOperand(cp, Context::SlotOffset(Context::GLOBAL_INDEX)));
+ __ ldr(r1, FieldMemOperand(r1, GlobalObject::kGlobalContextOffset));
+ __ ldr(r1, MemOperand(r1, Context::SlotOffset(
+ Context::JSFUNCTION_RESULT_CACHES_INDEX)));
+ __ ldr(r1, FieldMemOperand(r1, FixedArray::OffsetOfElementAt(cache_id)));
+
DeferredSearchCache* deferred = new DeferredSearchCache(r0, r1, r2);
const int kFingerOffset =
FixedArray::OffsetOfElementAt(JSFunctionResultCache::kFingerIndex);
ASSERT(kSmiTag == 0 && kSmiTagSize == 1);
- __ mov(r1, Operand(cache_obj));
__ ldr(r0, FieldMemOperand(r1, kFingerOffset));
// r0 now holds finger offset as a smi.
__ add(r3, r1, Operand(FixedArray::kHeaderSize - kHeapObjectTag));
Index: src/ia32/codegen-ia32.cc
diff --git a/src/ia32/codegen-ia32.cc b/src/ia32/codegen-ia32.cc
index
5ab7a531f325679e5285d045aadaa570625b56e4..340cd674f599485cfa4f2e92edf0f9026cfbf995
100644
--- a/src/ia32/codegen-ia32.cc
+++ b/src/ia32/codegen-ia32.cc
@@ -6551,17 +6551,24 @@ void
CodeGenerator::GenerateGetFromCache(ZoneList<Expression*>* args) {
frame_->Push(Factory::undefined_value());
return;
}
- Handle<FixedArray> cache_obj(
- FixedArray::cast(jsfunction_result_caches->get(cache_id)));
Load(args->at(1));
Result key = frame_->Pop();
key.ToRegister();
Result cache = allocator()->Allocate();
- __ mov(cache.reg(), cache_obj);
+ ASSERT(cache.is_valid());
+ __ mov(cache.reg(), Operand(esi,
Context::SlotOffset(Context::GLOBAL_INDEX)));
+ __ mov(cache.reg(),
+ FieldOperand(cache.reg(), GlobalObject::kGlobalContextOffset));
+ __ mov(cache.reg(),
+ Operand(cache.reg(),
+
Context::SlotOffset(Context::JSFUNCTION_RESULT_CACHES_INDEX)));
+ __ mov(cache.reg(),
+ FieldOperand(cache.reg(),
FixedArray::OffsetOfElementAt(cache_id)));
Result tmp = allocator()->Allocate();
+ ASSERT(tmp.is_valid());
DeferredSearchCache* deferred = new DeferredSearchCache(tmp.reg(),
cache.reg(),
Index: src/x64/codegen-x64.cc
diff --git a/src/x64/codegen-x64.cc b/src/x64/codegen-x64.cc
index
a6aa36d731f200074ada2760b67cf349235763f8..21ac7ba36c01c876da1e7f6a69c22ef4e379e607
100644
--- a/src/x64/codegen-x64.cc
+++ b/src/x64/codegen-x64.cc
@@ -4297,17 +4297,26 @@ void
CodeGenerator::GenerateGetFromCache(ZoneList<Expression*>* args) {
frame_->Push(Factory::undefined_value());
return;
}
- Handle<FixedArray> cache_obj(
- FixedArray::cast(jsfunction_result_caches->get(cache_id)));
Load(args->at(1));
Result key = frame_->Pop();
key.ToRegister();
Result cache = allocator()->Allocate();
- __ movq(cache.reg(), cache_obj, RelocInfo::EMBEDDED_OBJECT);
+ ASSERT(cache.is_valid());
+ __ movq(cache.reg(),
+ Operand(rsi, Context::SlotOffset(Context::GLOBAL_INDEX)));
+ __ movq(cache.reg(),
+ FieldOperand(cache.reg(), GlobalObject::kGlobalContextOffset));
+ __ movq(cache.reg(),
+ Operand(cache.reg(),
+ Context::SlotOffset(
+ Context::JSFUNCTION_RESULT_CACHES_INDEX)));
+ __ movq(cache.reg(),
+ FieldOperand(cache.reg(),
FixedArray::OffsetOfElementAt(cache_id)));
Result tmp = allocator()->Allocate();
+ ASSERT(tmp.is_valid());
DeferredSearchCache* deferred = new DeferredSearchCache(tmp.reg(),
cache.reg(),
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
