Closed Bug 791759 Opened 7 years ago Closed 7 years ago

Fix jit-tests default jitflags

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla20

People

(Reporter: jandem, Assigned: till)

References

Details

Attachments

(1 file)

jit-test.py with no arguments runs each test once with -m and once with -m -n. This no longer makes sense though, with the IonMonkey merge JM, TI and Ion are now enabled by default so we run each test twice with the same configuration.

We should clean-up the jitflags stuff and I propose by default we run each test with (1) --no-jm and (2) --no-ion.

TBPL can test the other flags like --no-ti and --ion-eager (and note that TBPL already uses the right flags).
(In reply to Jan de Mooij [:jandem] from comment #0)
> 
> We should clean-up the jitflags stuff and I propose by default we run each
> test with (1) --no-jm and (2) --no-ion.

Or rather (1) --no-jm and (2) no flags.
Do we even need --no-jm? But yeah either way, this sounds good to me.
What if we remove the --jitflags argument entirely and by default run the shell with no arguments. If you want to run with other arguments you can either

(1) Use --args: jit_test.py --args="--no-ti -a -d"
(2) Use --ion to run with --no-jm/--ion-eager
(3) Use --tbpl to run with all tinderbox flags

dmandelin, terrence, does this sound reasonable?
(In reply to Jan de Mooij [:jandem] from comment #3)
> What if we remove the --jitflags argument entirely and by default run the
> shell with no arguments. If you want to run with other arguments you can
> either
> 
> (1) Use --args: jit_test.py --args="--no-ti -a -d"
> (2) Use --ion to run with --no-jm/--ion-eager
> (3) Use --tbpl to run with all tinderbox flags
> 
> dmandelin, terrence, does this sound reasonable?

I would say:

 - jit_test.py by default should do the thing that is most useful to devs in day-to-day work
 - it needs whatever other options devs end up finding useful
 - Tinderbox needs to run whatever is appropriate for catching bugs in continuous integration and shipping a high-quality product

But it's up to you, the JIT people, to decide what that actually means.

(Feedback on option names: --args, --tbpl are nice and clear; --ion is ambiguous, --ion-eager or --ion-only would be better)
(In reply to David Anderson [:dvander] from comment #2)
> Do we even need --no-jm? But yeah either way, this sounds good to me.

I found --no-jm useful for cases where we need to pre-heat the script before compiling, it act as some-kind of ion eager with higher threshold.
I just stumbled across this. As we currently run each test twice with effectively the same configuration, I think we should fix this in the short term by just removing the default flags.

I also added some documentation about flags.
Attachment #683222 - Flags: review?(dmandelin)
Comment on attachment 683222 [details] [diff] [review]
Remove default flags

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

Makes sense to me.
Attachment #683222 - Flags: review?(dmandelin) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/b71748955cf3
Assignee: general → tschneidereit
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/b71748955cf3
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
Duplicate of this bug: 808049
You need to log in before you can comment on or make changes to this bug.