Closed
Bug 487191
Opened 16 years ago
Closed 16 years ago
Lk increase due to tracemonkey merge on April 7th
Categories
(Core :: JavaScript Engine, defect, P1)
Core
JavaScript Engine
Tracking
()
RESOLVED
DUPLICATE
of bug 487967
mozilla1.9.1b4
People
(Reporter: standard8, Assigned: brendan)
Details
(Keywords: memory-leak, regression)
Attachments
(1 file, 1 obsolete file)
70.94 KB,
text/plain
|
Details |
The recent tracemonkey merge on April 7th caused Lk regressions on all Firefox tier-1 platforms. The checkins:
http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2009-04-07+00%3A07&enddate=2009-04-07+00%3A08
Linux: Approx 3kB increase.
Mac: Hidden due to current size of leak.
Windows: Approx 4kB increase.
Linux Log: http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1239088257.1239089374.2927.gz
Leak diff extract attached.
Updated•16 years ago
|
Flags: blocking1.9.1?
Comment 1•16 years ago
|
||
Yep, some of the new stuff is leaking.
valgrind --leak-check=full ./js -j -e "gSkipSlowTests=true" -f trace-test.js
http://pastebin.mozilla.org/641166
Updated•16 years ago
|
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P1
Assignee | ||
Updated•16 years ago
|
Assignee: general → brendan
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.9.1b4
Assignee | ||
Comment 2•16 years ago
|
||
I ran with this cmd line in gdb:
r -e 'dumpHeap("/tmp/before.heap")' -j -e "gSkipSlowTests=true" -f trace-test.js -f cleanup.js -e 'gc(), dumpHeap("/tmp/after.heap")'
cleanup.js is just:
for (i in this)
delete this[i];
delete i;
The only patch is to jscntxt.cpp:
diff --git a/js/src/jscntxt.cpp b/js/src/jscntxt.cpp
--- a/js/src/jscntxt.cpp
+++ b/js/src/jscntxt.cpp
@@ -666,17 +666,20 @@ js_DestroyContext(JSContext *cx, JSDestr
* request to end. We'll let it run below, just before we do the truly
* final GC and then free atom state.
*/
while (cx->requestDepth != 0)
JS_EndRequest(cx);
#endif
if (last) {
+ memset(rt->builtinFunctions, 0, sizeof rt->builtinFunctions);
+
js_GC(cx, GC_LAST_CONTEXT);
+{ FILE *fp = fopen("/tmp/final.heap", "w"); if (fp) { JS_DumpHeap(cx, fp, NULL, 0, NULL, (size_t)-1, NULL); fclose(fp); } }
DUMP_EVAL_CACHE_METER(cx);
DUMP_FUNCTION_METER(cx);
/*
* Free the script filename table if it exists and is empty. Do this
* after the last GC to avoid finalizers tripping on free memory.
*/
if (rt->scriptFilenameTable &&
Without the memset to clear builtin functions, I see final.heap showing some leaks. With that memset, final.heap is a zero-length file.
Did some recent checkin fix this bug, or am I not doing the right thing to reproduce?
/be
Assignee | ||
Comment 3•16 years ago
|
||
Attachment #372166 -
Flags: review?(jorendorff)
Assignee | ||
Comment 4•16 years ago
|
||
I asked about recent checkins cuz the final.heap contents were only about 8 to 10 lines without the memset. Not enough to account for the valgrind log.
/be
Assignee | ||
Comment 5•16 years ago
|
||
Wow, dbaron nailed this at fifty paces. Dup'ing forward since he has the patch and diagnosis. Thanks, David!
/be
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → DUPLICATE
Assignee | ||
Comment 6•16 years ago
|
||
Comment on attachment 372166 [details] [diff] [review]
clearn rt->builtinFunctions before last GC
Over in bug 487968 now.
/be
Attachment #372166 -
Attachment is obsolete: true
Attachment #372166 -
Flags: review?(jorendorff)
You need to log in
before you can comment on or make changes to this bug.
Description
•