Closed Bug 296772 Opened 15 years ago Closed 15 years ago

Consider adding a WAY_TOO_MUCH_GC ifdef

Categories

(Core :: JavaScript Engine, defect, P2)

defect

Tracking

()

VERIFIED FIXED
mozilla1.8beta3

People

(Reporter: bzbarsky, Assigned: brendan)

Details

(Keywords: js1.5)

Attachments

(1 file)

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
Attachment #185612 - Flags: superreview?(jst)
Attachment #185612 - Flags: review?(shaver)
Assignee: general → brendan
Flags: blocking1.8b3+
Keywords: js1.5
OS: other → All
Priority: -- → P2
Hardware: PC → All
Target Milestone: --- → mozilla1.8beta3
Status: NEW → ASSIGNED
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] [edit])
> 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
Flags: testcase-
You need to log in before you can comment on or make changes to this bug.