Closed Bug 1101602 Opened 5 years ago Closed 5 years ago

Add command line option to the shell to set GC zeal

Categories

(Core :: JavaScript: GC, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: jonco, Assigned: jonco)

References

Details

Attachments

(1 file)

No description provided.
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.
Attachment #8525387 - Flags: review?(sphink)
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).
Attachment #8525387 - Flags: review?(sphink) → review+
(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.
https://hg.mozilla.org/mozilla-central/rev/03c6a758c9e8
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.