Closed Bug 908903 Opened 11 years ago Closed 11 years ago

Add a shell function for switching compiler options at runtime.

Categories

(Core :: JavaScript Engine, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla26

People

(Reporter: nbp, Assigned: nbp)

References

Details

Attachments

(1 file, 1 obsolete file)

+++ This bug was initially created as a clone of Bug #902690 +++ So far I have been unable to reproduce any bug with the patch provided as part of Bug 902690. So I am opening this bug to extract the part of the patch which deals with the extra JS Shell function and use it as a generic toggle for some Compiler options, which can be switched at runtime. The first two that I am looking into right now are the baseline & ion use count. As followup bugs/patches/ we could also add other options such as the Call & OSI point checks. This way we can prevent adding many command line flags to the test suite.
Another option might be to do bug 880330 and add the JIT flags to the options argument to `newGlobal`.
(In reply to Till Schneidereit [:till] from comment #1) > Another option might be to do bug 880330 and add the JIT flags to the > options argument to `newGlobal`. Compiler options are not attached to any runtime / context or any. So it does not make any sense to have it part of the newGlobal function. On the other hand, we can handle JSContext specific options with this interface. Note that the JS_OPTION are different than the compiler option that I mention in comment 0. The Compiler options are contained in jit/Ion.h (see IonOptions). I already made a JSAPI function to use these for fuzzing, such as Jesse can test ion-eager flags in the browser, and I am just reusing the same API to add this JS Shell function.
This patch introduces a new function to the JS shell. This function is available in both debug and optimized build. This function can be used as follow: - Switch all future compilation to be --baseline-eager & --ion-eager compilations setCompilerOption("baseline.usecount.trigger", 0); setCompilerOption("ion.usecount.trigger", 0); - Trigger Baseline / Ion compilations after 2 / 5 iterations. setCompilerOption("baseline.usecount.trigger", 2); setCompilerOption("ion.usecount.trigger", 5); - Reset default triggers (ignores the command line): setCompilerOption("baseline.usecount.trigger", -1); // reset to 10 setCompilerOption("ion.usecount.trigger", -1); // reset to 1000
Attachment #794918 - Flags: review?(jdemooij)
Comment on attachment 794918 [details] [diff] [review] Add generic shell function to toggle global compiler options. Review of attachment 794918 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jsapi.h @@ +5040,1 @@ > typedef enum JSCompilerOption { Can we rename this to JSJitCompilerOption while you're here? The API also uses "compiler" for bytecode compilation, see for instance the CompileOptions class. ::: js/src/shell/js.cpp @@ +3291,5 @@ > > #endif // JS_THREADSAFE > > +static bool > +SetCompilerOption(JSContext *cx, unsigned argc, jsval *vp) If we move this to builtin/TestingFunctions.cpp we can also use this function in the browser and (Jesse's) browser fuzzers can use it. We should also add some jit-tests, so that the fuzzers know about this function. @@ +3296,5 @@ > +{ > + CallArgs args = CallArgsFromVp(argc, vp); > + > + if (args.length() != 2) > + return false; Call ReportUsageError before returning false ("return false" without a pending exception will be treated as OOM) @@ +3299,5 @@ > + if (args.length() != 2) > + return false; > + > + if (!args[0].isString() || !args[1].isNumber()) > + return false; Same here. Also, s/isNumber/isInt32, or the args[1].toInt32() below will assert if it's a double.
Attachment #794918 - Flags: review?(jdemooij)
Delta: - Apply nits. - Rename JS_SetGlobalCompilerOption to JS_SetGlobalJitCompilerOption.
Attachment #794918 - Attachment is obsolete: true
Attachment #795619 - Flags: review?(jdemooij)
Comment on attachment 795619 [details] [diff] [review] Add generic shell function to toggle global jit compiler options. Review of attachment 795619 [details] [diff] [review]: ----------------------------------------------------------------- Nice!
Attachment #795619 - Flags: review?(jdemooij) → review+
I think this should be called setCompilerOption (without "Jit") because it's likely to be expanded to non-JIT things in the future. For example, it might grow a flag for turning on extra parser checks, or it might subsume gcparam.
Severity: normal → enhancement
Blocks: 909997
https://hg.mozilla.org/integration/mozilla-inbound/rev/aaf17ea43b59 With this patch the use case is like describe in comment 3, except that the function is named "setJitCompilerOption". The 2 options should remain the same, as documented in comment 3. (In reply to Jesse Ruderman from comment #7) > I think this should be called setCompilerOption (without "Jit") because it's > likely to be expanded to non-JIT things in the future. For example, it > might grow a flag for turning on extra parser checks, or it might subsume > gcparam. I jandem agree, I can make a follow-up patch to rename these.
Flags: needinfo?(jdemooij)
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
(In reply to Jesse Ruderman from comment #7) > I think this should be called setCompilerOption (without "Jit") because it's > likely to be expanded to non-JIT things in the future. For example, it > might grow a flag for turning on extra parser checks, or it might subsume > gcparam. That's not unreasonable, but does "setCompilerOption" make a lot of sense for setting GC parameters etc? What does "Compiler" refer to in that case? Maybe setJSOption but that sounds really generic..
Flags: needinfo?(jdemooij)
(In reply to Jan de Mooij [:jandem] from comment #11) > (In reply to Jesse Ruderman from comment #7) > > I think this should be called setCompilerOption (without "Jit") because it's > > likely to be expanded to non-JIT things in the future. For example, it > > might grow a flag for turning on extra parser checks, or it might subsume > > gcparam. > > That's not unreasonable, but does "setCompilerOption" make a lot of sense > for setting GC parameters etc? What does "Compiler" refer to in that case? > Maybe setJSOption but that sounds really generic.. The goal was to make it generic in the first place, this is also why I included a JSContext * as argument of this function, in case we might want to change or check against non-global options.
Depends on: 944153
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: