1101602 - Add command line option to the shell to set GC zeal

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect)

Comment 1

[Jon Coppeard (:jonco)]

Attachment: bug1101602-shell-zeal-option

Here's a patch to add a -z / --gc-zeal option to the shell.

I improved the parsing of the option string a little, although it's still not perfect.

Comment 2

[Steve Fink [:sfink] [:s:]]

Comment on attachment 8525387 [details] [diff] [review]
bug1101602-shell-zeal-option

Review of attachment 8525387 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jsgc.cpp
@@ +1316,4 @@
>              frequency = atoi(p + 1);
>      }
>  
>      if (zeal < 0 || zeal > ZealLimit || frequency < 0) {

So the original would allow --gc-zeal=snargle. This one does not. But couldn't you just change the test to zeal <= 0 and then not use the isdigit check? Your new one allows "14bleaghcckkth". I don't understand why it was < 0 in the first place.

@@ +1376,5 @@
>  #endif
>  
>  #ifdef JS_GC_ZEAL
> +    const char *zealSpec = getenv("JS_GC_ZEAL");
> +    if (zealSpec && !parseAndSetZeal(zealSpec))

Hm, I don't know how we usually do these, but I like to use

  if (zealSpec && zealSpec[0] && !parseAndSetZeal(zealSpec))

because people sometimes clear a value by setting it to the empty string (instead of unsetting it).

Comment 3

[Jon Coppeard (:jonco)]

(In reply to Steve Fink [:sfink, :s:] from comment #2)

> So the original would allow --gc-zeal=snargle. This one does not. But
> couldn't you just change the test to zeal <= 0 and then not use the isdigit
> check?

Well zero is a valid value for zeal and I want to allow that.  But that makes sense for the frequency.

> Hm, I don't know how we usually do these, but I like to use
> 
>   if (zealSpec && zealSpec[0] && !parseAndSetZeal(zealSpec))
> 
> because people sometimes clear a value by setting it to the empty string
> (instead of unsetting it).

Good idea.

Comment 4

[Jon Coppeard (:jonco)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/03c6a758c9e8

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/03c6a758c9e8