462826 - memory leaks from tracemonkey merge Mon Oct 27 20:18:42 2008 -0700

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Karl Tomlinson (:karlt)]

http://graphs.mozilla.org/graph.html#show=395139,395141,395170,1431037&sel=1225153291,1225173524

http://graphs.mozilla.org/graph.html#type=series&show=2207650

http://tinderbox.mozilla.org/showbuilds.cgi?tree=Firefox&maxdate=1225165355&legend=0&norules=1

http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=Oct+27+2008+18%3A00&enddate=Oct+27+2008+21%3A30

Comment 1

[Robert Sayre]

I'm worried that Karl's second link shows heap growth over time.

Comment 2

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: fix for a memory leak [Checkin: Comment 3 & 11]

Not sure if there's more, this came up from valgrinding.

Comment 3

[David Anderson [:dvander] - inactive, e-mail if emergency]

Pushed fix as changeset: http://hg.mozilla.org/tracemonkey/rev/e24a2e8fc744

Comment 4

[Karl Tomlinson (:karlt)]

Looks like that's probably the leak, thank you:

http://graphs-stage.mozilla.org/graph.html#type=series&show=430167

(Still a slight increase over time but I suspect that was there before.)

Comment 5

[Serge Gautherie (:sgautherie)]

(In reply to comment #4)
> Looks like that's probably the leak, thank you:

I don't know what this patch fixed,

> (Still a slight increase over time but I suspect that was there before.)

but major bug 462937 isn't fixed yet...

Comment 6

[Karl Tomlinson (:karlt)]

Serge, you'll need to use a tracemonkey build or apply the patch yourself to hg-central.

Setting Target Milestone in an attempt to request that this is fixed for Beta2.
Is another tracemonkey merge planned?

Comment 7

[Andreas Gal :gal]

We are waiting for m-c to go green so we can merge.

Comment 8

[Serge Gautherie (:sgautherie)]

(In reply to comment #6)
> Serge, you'll need to use a tracemonkey build

Oh ! I hadn't noticed that :-<

> Is another tracemonkey merge planned?

I can't understand this was allowed to stay in the tree for days, without even a warning (either manual or automatic), afaict :-(

(Looking forward for the merge !)

Comment 9

[David Anderson [:dvander] - inactive, e-mail if emergency]

I can't push to m-c -- if the merge doesn't happen soon enough someone can cherry pick the patch.

Comment 10

[Serge Gautherie (:sgautherie)]

Comment on attachment 346091 [details] [diff] [review]
fix for a memory leak
[Checkin: Comment 3 & 11]

"approval1.9.1b2=?":
per comment 9; fixes (major) "leak"; is already in tracemonkey repo, should be no risk.

Comment 11

[Serge Gautherie (:sgautherie)]

Comment on attachment 346091 [details] [diff] [review]
fix for a memory leak
[Checkin: Comment 3 & 11]

Merged to trunk as part of
[e-mail]  Sat Nov 08 01:06:43 2008 -0800
(http://hg.mozilla.org/mozilla-central/rev/17747605490c)

Comment 12

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b2pre) Gecko/20081108 Minefield/3.1b2pre] (home, optim default) (W2Ksp4)

V.Fixed, per bug 462937.