Closed Bug 633543 Opened 13 years ago Closed 12 years ago

clean up JIT-related flags

Categories

(Core :: JavaScript Engine, defect)

Product:
Core
Component:
JavaScript Engine
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED WONTFIX

People

(Reporter: shaver, Assigned: shaver)

Details

We have 3 flags for controlling the JIT, and a 4th coming in bug 631951.  I propose rationalizing them behind a --jit flag:

--jit=tracer,method,profile
--jit=default (the default, which is above; useful in scripts)

--jit=tracer,methodall,profile (force whole-method JIT always, after bug 631951)

JS_JIT_MODE="tracer,method"

This would also let us change the default to be the config we ship in FF, namely tracer,method,profile.
This is one of those trickier-than-it-first-seems things.

Let's start by enumerating the valid combinations (where 'i'==interpreter, 'm'==methodJIT, 't'==traceJIT, 'p'==profiling).

  imtp
  ----
  x...
  .x..
  xx..
  x.x. (browser default for chrome)
  .xx.
  xxx.
  .xxx (browser default for content before bug 631951 lands)
  xxxx (browser default for content after bug 631951 lands)

So we need an "interp" value, which suggests that --jit isn't a good name.  Maybe --mode?  --exec?  Also, "methodall" isn't right;  it should just be "method" (which contrasts with "interp,method").

We currently have two ways of combining the method JIT and the trace JIT -- with or without profiling.  Can we remove "without profiling"?  (You could just use interp+traceJIT if you want to run the traceJIT as often as possible.)  That would simplify things.  (In comparison, we only have one way to combine the interpreter and the method JIT.)  If we did that, all six combinations of interpreter/methodJIT/traceJIT would make sense:  i, m, im, it, mt, imt.  That feels right to me.

Also, I personally type these options a lot.  Would eg. 'js --mode=imt' style be better?  Maybe we could do both.  Even ps-style 'js imt' or 'js -imt'?  Maybe that's too much.  (And -i and -t are already in use, though they could be renamed.)  Yes, I could create aliases, but I already have a heap of them for debug32/opt32/debug64/opt64 builds in 10 different repositories, and I'd rather not multiply the number required by 6 or 8.

TL;DR:  My suggestions:
- Remove the "method JIT + trace JIT without profiling" option.
- Use a single letter to specify each mode: 'i', 'm', 't'.
- Write the six possibilities like this:  -i, -m, -im, -it, -mt, -imt, and rename the old -i and -t options to something else.
> If we did that, all six combinations of
> interpreter/methodJIT/traceJIT would make sense:  i, m, im, it, mt, imt.  That
> feels right to me.
 
Oh crap, there are seven possibilities, but traceJIT by itself doesn't make sense.
Still, seems better than if 'p' is in there too.
Please don't remove the "method JIT + trace JIT without profiling" option. I use it all the time to debug the profiler.
(In reply to comment #1)
> Also, I personally type these options a lot.  Would eg. 'js --mode=imt' style
> be better?  Maybe we could do both.  Even ps-style 'js imt' or 'js -imt'? 

ps-style is bad form, and doesn't really gel with something which can take an arbitrary filename as an option. 'js -imt' is a standard shorthand for 'js -i -m -t', and therefore nice and unixy. (Though we don't currently support shorthands, because we hand rolled our command-line parser.)
The hand-rolled parser was from 1995. GNU getopt was not an option for several reasons. But it is easy to hack on if people want shorthand options.

/be
(In reply to comment #5)
> But it is easy to hack on if people want shorthand options.

It's also buggy. See eg bug 582353. Command-line parsers are notoriously difficult get right, especially when half the time they're asked to do the impossible and nobody can agree on standards. (The GNU standard is totally reasonable, except it turns out to be impossible to train people to only do |--long=value| and not |--long value|.)
(In reply to comment #3)
> Please don't remove the "method JIT + trace JIT without profiling" option. I
> use it all the time to debug the profiler.

Ok, fair enough.  Maybe allow -mtp and -imtp as well, then?

(In reply to comment #4)
> 
> ps-style is bad form

True!
We don't need to specify the interpreter, IMO.  There is no way to disable its use at present, when all else fails.  If we want -J none, we can have it.

All combinations of t?, [mM]?, p? are valid, where M means always-mjit and m means mjit-in-its-default-config.

-J tmp would work, but it looks like a temp dir.

-J default would be what people would use most of the time, or just omit it.  It's that we default to interp-only in the shell that leads to having to type it all the time.  We could permit a - or "no" prefix to disable, so "-J noprofile" would work.
(In reply to comment #9)
> We don't need to specify the interpreter, IMO.  There is no way to disable its
> use at present

"-mt" doesn't use the interpreter, except for the single iteration when recording traces, which I'm not counting because it doesn't make sense to.

> -J default would be what people would use most of the time

I can only speak for myself, but I wouldn't use that most of the time.
> All combinations of t?, [mM]?, p? are valid, where M means always-mjit and m
> means mjit-in-its-default-config.

tp, mp and Mp are all invalid.  Please see comment 1!
The best way to move this forward is with suggestions for fully specified designs, rather than partially specified designs.  Here is mine and Shaver's (AIUI, apologies if I've got it wrong), for the eight valid modes:

  mode
  imtp        njn         shaver
  ----        ---         ------
  x...        js -i       js -J none
  .x..        js -m       js -J M
  xx..        js -im      js -J m
  x.x.        js -it      js -J t
  .xx.        js -mt      js -J Mt
  xxx.        js -imt     js -J mt
  .xxx        js -mtp     js -J Mtp
  xxxx        js -imtp    js -J mtp

For my design the letters can be in any order, eg "js -mti" is the valid and the same as "js -imt".

(In reply to comment #10)
> > We don't need to specify the interpreter, IMO.  There is no way to disable its
> > use at present
> 
> "-mt" doesn't use the interpreter, except for the single iteration when
> recording traces, which I'm not counting because it doesn't make sense to.

Oh, and -m (using my syntax) doesn't use the interpreter at all.
While we're doing this, we should clean up the browser flags as well -- we should end up with two non-boolean flags, one for chrome and one for content.

And we also need to make sure the shell builtin options() still works with the new forms.
I don't like the idea of expressing this in terms of subsets of the "imtp" options. That's just not the way the system works anymore. Invalid combinations are a symptom of that problem. More fundamentally, that design is really a mishmash of two separate sets of switches: "enable/disable jit X" and "enable jit X when".

I think the simplest design would be to express complete 'policy' or 'strategy' choices about how the different execution modes are combined. It could be done via an option

 exec=POLICY

I think we need at least these policies, with suggested names in the interest of having a complete proposal:

 - (default) default (today's -m -j -p)
 - (interp) interpreter only
 - (mjit) mjit only
 - (maxtjit) trace maximally (similar to today's -j: the idea is to 
     start tracing as soon and aggressively as possible to maximally test
     the tracer)

If desired, we could get finer control by allowing customization of strategy parameters, e.g., HOTLOOP for the maxtjit case.

I'm not convinced we can really make an orthogonal set of switches for controlling jits, but if we wanted something like that, I think the closest we could get would be "activate jit X when" switches. There would be a set for the methodjit (first call, loop edge N, never), and a set for the tracer (loop edge N, via profiling, never).
dmandelin, if you have a design, can you please flesh it out fully (like comment 12)?  As I said before, partially-specified designs aren't very helpful here, because the devil is really in the details with this one.

If you think the eight combinations I suggested aren't the right way of thinking about it, that's fine, just please specify the syntax and meaning of all the possible combinations your design would provide.  Thanks!
As long as the browser can tweak the policy with individual bits, the shell should be able to as well.
comment 12 doesn't correctly represent my proposal, no doubt due to a lack of clarity on my part. I'm not going to write more in this bug until after we've shipped, though.
Finally have the time to get back to this discussion. Before any complete proposals, first some general thoughts:

1. If you really like single-char flags, then they pretty much have to have a boolean sense.

2. If you want the flags to reflect the design/code, then IMO the most direct reflection is to have 2 flags "activate mjit when _" and "activate tjit when _".

2a. "activate xjit when _" doesn't look boolean, but of course you can encode it in a boolean way.

2b. The only place these options aren't orthogonal is that "activate mjit never" and "activate tjit when mjit profiling says so" don't make sense together. This is a reflection of a non-orthogonality in the code.

3. I tend to favor making options clear over making them short, but that's personal preference only and not a completely consistent one.

OK, now some proposals.

A. activate-xjit-when

  -m, --mjit ( always | counter | never )
  -t, --tjit ( hotloop | profile | never )

  default is -m counter -t profile
  -m never -t profile is invalid
  if -m never is given but -t is not given, then -t hotloop is selected

  Note that we can expand the syntax to counter=N, hotloop=N, and
  profile=tuning_params for even more power

The argument can be shortened to a prefix, one character in this case. Also allow '0' for never. Examples:

  default:     -mc -tp
  interp only: -m0 -t0

B. activate xjit-when with boolean encoding

  -m   |mjit always|
  -M   |mjit never|
  -t   |tjit always|
  -T   |tjit never|

  default is mjit counter, tjit profile

This allows for maximum brevity, but I think the letters are kind of confusing. Maybe different letters would help?

Conversion table from comment 12:

  mode
  imtp        njn         shaver                18A       18B
  ----        ---         ------            -------   -------
  x...        js -i       js -J none        -m0 -t0        MT
  .x..        js -m       js -J M           -ma -t0        mT
  xx..        js -im      js -J m           ??             ??
  x.x.        js -it      js -J t           -m0 -th        Mt
  .xx.        js -mt      js -J Mt          -ma -th        mt
  xxx.        js -imt     js -J mt          ??             ??
  .xxx        js -mtp     js -J Mtp         -mc -th        
  xxxx        js -imtp    js -J mtp         ??             ??

?? because I'm not sure what the "im" combination is supposed to do.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.