296772 - Consider adding a WAY_TOO_MUCH_GC ifdef

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P2)

Description

[Boris Zbarsky [:bzbarsky]]

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?

Comment 1

[Brendan Eich [:brendan]]

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

Comment 2

[Boris Zbarsky [:bzbarsky]]

It's fine with me if it's fine with jst.

Comment 3

[Brendan Eich [:brendan]]

bz: are you developing this patch?  If so, want to take the bug?

/be

Comment 4

[Boris Zbarsky [:bzbarsky]]

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.  ;)

Comment 5

[Brendan Eich [:brendan]]

Attachment: desired change, plus my patch for bug 262948

Looking for testing buddying as well as review.

/be

Comment 6

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 185612 [details] [diff] [review]
desired change, plus my patch for bug 262948

r=shaver

Comment 7

[Johnny Stenback (:jst)]

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.

Comment 8

[Brendan Eich [:brendan]]

(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

Comment 9

[Boris Zbarsky [:bzbarsky]]

Verified. Thanks, Brendan!