Closed
Bug 660734
Opened 13 years ago
Closed 13 years ago
Strings are not counted by GC heuristics (high memory use with regexps on large strings)
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: jarben, Assigned: luke)
References
Details
(Keywords: memory-footprint, testcase, Whiteboard: [MemShrink:P2][fixed-in-tracemonkey])
Attachments
(3 files, 1 obsolete file)
(deleted),
text/html
|
Details | |
(deleted),
patch
|
igor
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
igor
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0.1) Gecko/20100101 Firefox/4.0.1
Build Identifier: 4.0.1
Try attached example and see memory usage:
FF4 uses 109MB - 560MB
FF3 uses 73MB - 196MB
Chrome 11 uses 55MB - 60MB
The other (and major) problem is that it leaks when embedded as in iframe but I haven't been able to reproduce a simple test case outside of our product, will try and report it as another bug.
Interesting that Chrome can do the same operation with 5MB, maybe would be worth to overview this as it might improve performance in general.
Reproducible: Always
Reporter | ||
Comment 1•13 years ago
|
||
Reporter | ||
Updated•13 years ago
|
Attachment #536215 -
Attachment is obsolete: true
Reporter | ||
Comment 2•13 years ago
|
||
Reporter | ||
Updated•13 years ago
|
Attachment #536218 -
Attachment mime type: text/plain → text/html
Reporter | ||
Updated•13 years ago
|
Attachment #536215 -
Attachment mime type: text/plain → text/html
Comment 3•13 years ago
|
||
I am seeing pretty high memory usage, it seems to go from roughly 180mb to 630 before garbage collection kicks it and it goes back down. But the number it goes to is always higher than the number is started at the last cycle (eg, 180-630 GC 140-630GC)
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0.1) Gecko/20100101 Firefox/4.0.1
Comment 4•13 years ago
|
||
Luke, could this have anything to do with ropes?
Assignee | ||
Comment 5•13 years ago
|
||
The rope optimization only applies when we can do "flat" matches (e.g., str.replace('a', 'b')), not when we are given a regexp as in the example.
Basically, the testcase just takes a huge string and does 6 regexp replace's on it (each producing a copy), and does that from setTimeout every 10ms. Each string becomes garbage, so it should all get collected on each GC (which about:memory and ps seems to show is the case). I watched total usage for several minutes and saw no increase in the peaks or valleys of the sawtooth curve. I believe the reason we get so high compared to Chrome is b/c we wait so long for a GC whereas Chrome (with its generational gc) is going to collect much more often and thus collect all the garbage being created. Our own GC work should help this.
Reporter | ||
Comment 6•13 years ago
|
||
Luke, any thoughts why FF3 memory behaviour is better? Is there anything we could do, would WeakMap help in this case?
Comment 7•13 years ago
|
||
3.6 is a lot more aggressive about running gc than 4.0 is. That means more time is taken running GC, though, which hurts throughput.
Updated•13 years ago
|
Updated•13 years ago
|
Whiteboard: [MemShrink:P2]
Comment 8•13 years ago
|
||
I also noticed this on my very low end netbook where it causes serious memory pressure problems as I told Luke yesterday.
I looked at a shark profile and noticed that most memory is allocated via our vectors and vector reallocs don't account for our GC triggers. So we could probably run into OOM without running the GC.
Luke does this sound reasonable?
Assignee | ||
Comment 9•13 years ago
|
||
Could you point me to the particular Vectors that are doing this? So, recently, igor changed ContextAllocPolicy to not do gc-malloc-bytes accounting since, in theory, if you can use ContextAllocPolicy, you must be short-lived (else you can get a dangling JSContext, as you know :), and hence you are "temporary" and thats not what gcMallocBytes is measuring. However, here you have a case where a Vector is long-lived enough to cause real GC pressure. So it seems we just really need a new AllocPolicy. There have been a couple of recent examples that our AllocPolicy situation is weird and needs to be refactored; I'll file a bug once you point me to the offending Vector so I can check out the code.
Assignee | ||
Comment 10•13 years ago
|
||
Aha, when ContextAllocPolicy stopped accounting gcMallocBytes, StringBuffer's internal vector stopped accounting for its allcoations. Since the StringBuffer's buffer is ultimately extracted to be the long-lived buffer of a string, this is totally a hole in the GC heuristic! No wonder the GC sawtooth gets to high. The fix is simply to give StringBuffer's Vector a policy that does do accounting.
Comment 11•13 years ago
|
||
Luke, I'll assign this to you, hope this is ok.
Assignee: general → luke
Version: 2.0 Branch → unspecified
Assignee | ||
Comment 12•13 years ago
|
||
Sure, I can take it.
What I'd like to do is throw out all these crazy "Context" "System" "Runtime" names and make a new set whose names correspond the pairs:
{ Reporting, NonReporting } x { Accounting, NonAccounting }
(In the process auditing our existing policy use.)
Assignee | ||
Comment 13•13 years ago
|
||
I need to put back the old ContextAllocPolicy in the next patch (the one that calls cx->malloc_ instead of js_malloc+report), so rename the thing currently (mis)named as ContextAllocPolicy. I'd rather not debate the merits of the name "TempAllocPolicy" since all these *AllocPolicy names need to be changed after we settle on the underlying allocator names in bug 647103.
Attachment #539926 -
Flags: review?(igor)
Assignee | ||
Comment 14•13 years ago
|
||
Incidentally, even before Igor's changes, StringBuffer had accounting problems since it called realloc() without passing oldBytes (which means realloc() wouldn't do accounting). Fortunately, we just added an oldBytes param to AllocPolicy (like, yesterday).
Attachment #539928 -
Flags: review?(igor)
Assignee | ||
Comment 15•13 years ago
|
||
With these two patches (applied to TM tip) I see memory usage drop from FF4 levels to FF3 levels (as reported by comment 0).
Comment 16•13 years ago
|
||
(In reply to comment #15)
> With these two patches (applied to TM tip) I see memory usage drop from FF4
> levels to FF3 levels (as reported by comment 0).
Great! Thanks, Luke.
Comment 17•13 years ago
|
||
Comment on attachment 539928 [details] [diff] [review]
add old ContextAllocPolicy, use in StringBuffer
Review of attachment 539928 [details] [diff] [review]:
-----------------------------------------------------------------
Attachment #539928 -
Flags: review?(igor) → review+
Assignee | ||
Comment 18•13 years ago
|
||
Igor: there is one other patch needing your r+ (purely syntactic).
Comment 19•13 years ago
|
||
Comment on attachment 539926 [details] [diff] [review]
s/ContextAllocPolicy/TempAllocPolicy/
Review of attachment 539926 [details] [diff] [review]:
-----------------------------------------------------------------
Attachment #539926 -
Flags: review?(igor) → review+
Assignee | ||
Comment 20•13 years ago
|
||
http://hg.mozilla.org/tracemonkey/rev/47b578958aa4
http://hg.mozilla.org/tracemonkey/rev/31e3b521775b
Whiteboard: [MemShrink:P2] → [MemShrink:P2][fixed-in-tracemonkey]
Comment 21•13 years ago
|
||
My netbook says DANKE! :)
Comment 22•13 years ago
|
||
cdleary-bot mozilla-central merge info:
http://hg.mozilla.org/mozilla-central/rev/47b578958aa4
http://hg.mozilla.org/mozilla-central/rev/31e3b521775b
Updated•13 years ago
|
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Updated•13 years ago
|
Summary: Firefox 4: Huge memory usage when performing regular expressions on large strings → Strings are not counted by GC heuristics (high memory use with regexps on large strings)
You need to log in
before you can comment on or make changes to this bug.
Description
•