Closed Bug 296772 Opened 15 years ago Closed 15 years ago
Consider adding a WAY
_TOO _MUCH _GC ifdef
I'd like to have an ifdef I can set that would force GC whenever JS_MaybeGC is called. Brendan suggested that this be named WAY_TOO_MUCH_GC and that it also force GC on every allocation attempt. In addition to those, I think the macro should also make nsJSContext::ScriptEvaluated always call JS_MaybeGC. Can that file include jsgc.h? Or would the macro need to go in jsapi.h?
Touching jsapi.h (or jspubtd.h) will cause WAY_TOO_MUCH_RECOMPILING ;-). Just include jsgc.h in nsJSEnvironment.cpp #ifdef NS_DEBUG? Or unconditionally, I'm cool with either way. /be
It's fine with me if it's fine with jst.
bz: are you developing this patch? If so, want to take the bug? /be
Brendan, I don't think I know nearly enough about the JS engine to do the "GC on every allocation attempt" part. I also have no tree until I get back into town on June 30... So I'm not developing anything right now. ;)
Looking for testing buddying as well as review. /be
Assignee: general → brendan
OS: other → All
Priority: -- → P2
Hardware: PC → All
Target Milestone: --- → mozilla1.8beta3
Comment on attachment 185612 [details] [diff] [review] desired change, plus my patch for bug 262948 r=shaver
Attachment #185612 - Flags: review?(shaver) → review+
Comment on attachment 185612 [details] [diff] [review] desired change, plus my patch for bug 262948 Index: js/src/jsgc.h [...] +#undef GC_MARK_DEBUG + #ifdef GC_MARK_DEBUG Is that really what we want here? sr=jst with that explained or fixed.
Attachment #185612 - Flags: superreview?(jst) → superreview+
(In reply to comment #7) > (From update of attachment 185612 [details] [diff] [review] ) > Index: js/src/jsgc.h > [...] > +#undef GC_MARK_DEBUG > + > #ifdef GC_MARK_DEBUG > > Is that really what we want here? Oops, that was deadwood. Plus, I added #ifdef NS_DEBUG #include "jsgc.h" // for WAY_TOO_MUCH_GC, if defined for GC debugging #endif to nsJSEnvironment.cpp after the first two groups of #includes, as promised. Fixed on trunk. /be
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Verified. Thanks, Brendan!
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.