1119584 - Return number of allocations until next zeal GC

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect)

Description

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

On IRC, we were chatting about how to make zeal bugs more reducible. You sometimes have a large script that fails with zeal set to (eg) 14,100. Reducing is hard because anything you delete in the preamble is likely to move the zeal GC so that it no longer hits. Jesse suggested that if we could add something shortly before the crash point that sets the "allocation count until next zeal GC" trigger, then the preamble could be reduced freely.

Comment 1

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

Attachment: Return number of allocations until next zeal GC

This patch makes schedulegc() return the number of allocations before it will trigger the next zeal GC. So the idea is that you'd have a fuzz bug like:

  do_some_stuff();
  do_some_more_stuff();
  do_something_that_crashes_with_zeal_set();

then you'd modify it to be

  do_some_stuff();
  do_some_more_stuff();
  print(schedulegc());
  do_something_that_crashes_with_zeal_set();

getting back, say, 56. Then you would change it to:

  do_some_stuff();
  do_some_more_stuff();
  schedulegc(56);
  do_something_that_crashes_with_zeal_set();

and reduce it.

Comment 2

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

Note that it's a little tricky using this from an interactive shell, since schedulegc() will consume 2 allocations itself: one for the one-line script "schedulegc()", and one for the ScriptSource object identifying that it came from "(eval:3)" or whatever.

Comment 3

[Terrence Cole [:terrence]]

Comment on attachment 8546269 [details] [diff] [review]
Return number of allocations until next zeal GC

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

Neat!

Comment 4

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

Comment on attachment 8546269 [details] [diff] [review]
Return number of allocations until next zeal GC

Come to think of it, I'm not sure if the fuzzers are going to be too happy with this. It makes schedulegc() return a somewhat nondeterministic value (as in, it may change if the JS engine changes to add additional internal allocations.) Marking you guys with feedback? to see if you'd rather do it some other way.

Comment 5

[Jesse Ruderman]

I'm not sure what your concern is regarding nondeterminism --

 -- Spurious differential-testing reports, due to number of allocations depending on JIT mode? I'll just stick |void| before all calls, so the output of schedulegc() won't be part of the differential-testing output.

 -- Occasional changes in the small number of allocations that schedulegc() does internally? I assume we'll be doing these reductions manually, taking the output from schedulegc() and transforming the testcase to call schedulegc(output + SMALL_OFFSET). If we have to use SMALL_OFFSET = 2 for the first year, and then a change lands and we have to use SMALL_OFFSET = -1 instead, that's not a big deal.

With this patch, it sounds like schedulegc(n) will return n?

How does schedulegc interact with gczeal? Like, if I call gczeal(4, 100) and then at some point later call schedulegc(56), does that just move the same counter that gczeal is using?

Comment 6

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

(In reply to Jesse Ruderman from comment #5)
> I'm not sure what your concern is regarding nondeterminism --
> 
>  -- Spurious differential-testing reports, due to number of allocations
> depending on JIT mode? I'll just stick |void| before all calls, so the
> output of schedulegc() won't be part of the differential-testing output.

Yes, it was the output of schedulegc() that I was worried about. If you never use its return value for differential testing, then you're fine.

>  -- Occasional changes in the small number of allocations that schedulegc()
> does internally? I assume we'll be doing these reductions manually, taking
> the output from schedulegc() and transforming the testcase to call
> schedulegc(output + SMALL_OFFSET). If we have to use SMALL_OFFSET = 2 for
> the first year, and then a change lands and we have to use SMALL_OFFSET = -1
> instead, that's not a big deal.

schedulegc() won't do any GCs internally, unless you run it via Debugger or eval or whatever. So SMALL_OFFSET = 0 should be fine. Hopefully print() or whatever you use to record its value won't GC either.

> With this patch, it sounds like schedulegc(n) will return n?

Yes. Though I would only guarantee that for the zeal modes that have a meaningful N in the first place. (And I expect you would be calling schedulegc() normally, not schedulegc(n).)

> How does schedulegc interact with gczeal? Like, if I call gczeal(4, 100) and
> then at some point later call schedulegc(56), does that just move the same
> counter that gczeal is using?

Yes.

There is one wrinkle I'll mention, but I don't think it'll matter.

  gczeal(14, 100);
  print(schedulegc());

is not guaranteed to print 100. If you are already past the 100th allocation, then it will print 0 (and do the GC before the very next allocation).

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/20ed57d8a4e7

Comment 8

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Comment on attachment 8546269 [details] [diff] [review]
Return number of allocations until next zeal GC

(deferred to Jesse's opinion here)