Return number of allocations until next zeal GC

RESOLVED FIXED in mozilla37

Status

()

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: sfink, Assigned: sfink)

Tracking

unspecified
mozilla37
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
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.
(Assignee)

Comment 1

4 years ago
Created attachment 8546269 [details] [diff] [review]
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.
Attachment #8546269 - Flags: review?(terrence)
(Assignee)

Comment 2

4 years ago
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.
(Assignee)

Updated

4 years ago
Attachment #8546269 - Flags: feedback?(jruderman)
Comment on attachment 8546269 [details] [diff] [review]
Return number of allocations until next zeal GC

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

Neat!
Attachment #8546269 - Flags: review?(terrence) → review+
(Assignee)

Comment 4

4 years ago
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.
Attachment #8546269 - Flags: feedback?(gary)

Comment 5

4 years ago
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?

Updated

4 years ago
Attachment #8546269 - Flags: feedback?(jruderman)
(Assignee)

Comment 6

4 years ago
(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).
https://hg.mozilla.org/mozilla-central/rev/20ed57d8a4e7
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Comment on attachment 8546269 [details] [diff] [review]
Return number of allocations until next zeal GC

(deferred to Jesse's opinion here)
Attachment #8546269 - Flags: feedback?(gary)
You need to log in before you can comment on or make changes to this bug.