Closed
Bug 381199
Opened 18 years ago
Closed 18 years ago
revisit cycle collector aging strategy
Categories
(Core :: XPCOM, defect)
Core
XPCOM
Tracking
()
RESOLVED
FIXED
People
(Reporter: dbaron, Assigned: dbaron)
References
Details
(Keywords: memory-footprint, perf)
Attachments
(1 file)
531 bytes,
patch
|
peterv
:
review+
peterv
:
superreview+
|
Details | Diff | Splinter Review |
Once we've switched the cycle collector to using the tracing API, I want to revisit the performance/footprint tradeoff in the cycle collector's aging strategy (mScanDelay). I suspect that, given the current frequency of cycle collections, we're sacrificing too much footprint for negligible performance benefit. In fact, it wouldn't surprise me if there were no performance benefit to aging at all (and in fact, it's possible there's a performance loss, since it would increase JS GC times).
(Also consider the interaction with the patch to suspect all native wrappers from bug 368869.)
Flags: blocking1.9+
Assignee | ||
Comment 1•18 years ago
|
||
I haven't had the chance to do measurement, but my gut feeling is that this is what we want given the current frequency of cycle collections, so I'd like to do this; maybe I'll have a chance to measure later.
Attachment #268108 -
Flags: superreview?(peterv)
Attachment #268108 -
Flags: review?(peterv)
Comment 2•18 years ago
|
||
Comment on attachment 268108 [details] [diff] [review]
change mScanDelay default to 0
Ok, let's try it.
Attachment #268108 -
Flags: superreview?(peterv)
Attachment #268108 -
Flags: superreview+
Attachment #268108 -
Flags: review?(peterv)
Attachment #268108 -
Flags: review+
Assignee | ||
Comment 3•18 years ago
|
||
Checked in to trunk.
Comment 4•18 years ago
|
||
This may have caused a huge regression in leaks:
Rlk up to 816KB
Lk up to 3.71MB
Assignee | ||
Comment 5•18 years ago
|
||
Backed out due to Lk, RLk, and MH increase.
Maybe bug 368869 would help? Or maybe the collector was Fault-ing?
Assignee | ||
Comment 6•18 years ago
|
||
I relanded this after relanding bug 368869, and the leak regression was still present.
Assignee | ||
Comment 7•18 years ago
|
||
Relanded again (and it stuck this time).
Comment 8•18 years ago
|
||
Is there still something to do here or could this be closed?
Assignee | ||
Comment 9•18 years ago
|
||
Might be nice to actually measure performance and memory use characteristics. Probably the first thing to measure would be 0 vs. 1 for performance -- if there's no difference, it's probably not worth bothering to measure further.
Comment 10•18 years ago
|
||
How would one measure the performance here. Running tp/tdhtml or
using COLLECT_TIME_DEBUG to get some data and trying to interpret it
in some reasonable way, or what?
Btw, bug 373462/bug 385322 will change ccollecting scheduling. Not
sure if the current patch there is the best possible, but something like
that (or mapping ccollector somehow to mem allocs) is needed.
Comment 11•18 years ago
|
||
(In reply to comment #9)
> Might be nice to actually measure performance and memory use characteristics.
> Probably the first thing to measure would be 0 vs. 1 for performance -- if
> there's no difference, it's probably not worth bothering to measure further.
>
Marking this fixed. Please open new bugs for measurement issues.
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•