390847 - Large increase in private byte usage after the landing of bug 385393

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Adam Guthrie]

Looking at the talos stats, there was a large increase in the number of private bytes allocated after the patch for bug 385393 was checked in. 

Graph: http://graphs.mozilla.org/graph.html#spst=range&spstart=0&spend=1186162910&bpst=cursor&bpstart=0&bpend=1186162910&m1tid=8&m1bl=0&m1avg=0

Bonsai: http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=PhoenixTinderbox&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-08-01+20%3A00&maxdate=2007-08-01+22%3A00&cvsroot=%2Fcvsroot

Comment 1

[Robert Sayre]

Working Set numbers showed a similar increase

Comment 2

[Igor Bukanov]

I suspect this related to that decreased number of GC runs, see bug 385393 comment 107. Less GC means that more garbage can be accumulated between runs.

Comment 4

[Brendan Eich [:brendan]]

The cause of reduced GC rate is very likely the unit-length string memo-ization change that went in along with the fast native bulk of the patch for bug 385393. The fast native changes should not have affected memory use, at least not in ways that feed back on the GC schedule.

As I said to Jesse, the way JS_MaybeGC tries to assess when to GC is like trying to tell time of day by measuring bathroom visit distribution. We should try to do better, using process data segment size, perhaps.

/be

Comment 5

[Brendan Eich [:brendan]]

Attachment: patch to try on that talos setup

Could this be applied to a test build and run against talos?

/be

Comment 6

[Robert Sayre]

(In reply to comment #5)
> Created an attachment (id=275859) [details]
> patch to try on that talos setup
> 
> Could this be applied to a test build and run against talos?

This and bug 395828 came up in the perf meeting today. The js_NewStringCopyN call has already been changed to the version in this patch. Any objections to checking a version of this file with the unit string cache turned off? The test perf boxes are busy in maintenance or baseline work.

Comment 7

[Robert Sayre]

fixed by bug 395828.

Comment 8

[Jesse Ruderman]

Does that mean this bug wasn't due to a decrease in GC frequency, but to a leak that we failed to diagnose at the time?

Comment 9

[Brendan Eich [:brendan]]

Sadly, yes. And a leak that depended on relative address order of certain malloc'ed allocations.

/be

Comment 10

[Jesse Ruderman]

Ok.  Is there a bug on potentially tweaking the GC rate anyway?  Or on changing the strategy so it's not "like trying to tell time of day by measuring bathroom visit distribution"?