Bug 143357 - Enhance ability to dump JSC Options
Summary: Enhance ability to dump JSC Options
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-04-02 20:04 PDT by Mark Lam
Modified: 2015-04-02 20:47 PDT (History)
7 users (show)

See Also:


Attachments
the patch. (35.31 KB, patch)
2015-04-02 20:22 PDT, Mark Lam
benjamin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Lam 2015-04-02 20:04:08 PDT
Some enhancements to how the JSC options work:

1. Add a JSC_showOptions option which take values: 0 = None, 1 = Overridden only, 2 = All, 3 = Verbose.
   The default is 0 (None).  This dumps nothing.
   With the Overridden setting, at VM initialization time, we will dump all option values that have been changed from their default.
   With the All setting, at VM initialization time, we will dump all option values.
   With the Verbose setting, at VM initialization time, we will dump all option values along with their descriptions (if available).

2. We now store a copy of the default option values.
   We later use this for comparison to tell if an option has been overridden, and print the default value for reference.
   As a result, we no longer need the didOverride flag since we can compute whether the option is overridden at any time.

3. Added description strings to some options to be printed when JSC_showOptions=3 (Verbose).  This will come in handy later when we want to rename some of the options to more sane names that are easier to remember.  For example, we can change Options::dfgFunctionWhitelistFile() to Options::dfgWhiteList(), and Options::slowPathAllocsBetweenGCs() to Options::forcedGcRate().  With the availability of the description, we can afford to use shorter and less descriptive option names, but they will be easier to remember and use for day to day debugging work.

   In this patch, I did not change the names of any of the options yet.  I only added description strings for options that I know about, and where I think the option name isn't already descriptive enough.

4. Also deleted some unused code.
Comment 1 Mark Lam 2015-04-02 20:22:52 PDT
Created attachment 250033 [details]
the patch.
Comment 2 Benjamin Poulain 2015-04-02 20:34:57 PDT
Comment on attachment 250033 [details]
the patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=250033&action=review

> Source/JavaScriptCore/runtime/Options.cpp:397
> +        fprintf(stream, "%s", m_entry.boolVal?"true":"false");

Missing spaces around the part of the operator.

> Source/JavaScriptCore/runtime/Options.h:275
> +    v(unsigned, slowPathAllocsBetweenGCs, 0, "debugging option to trigger a GC for every count of this number of slow path allocs") \

The description is a bit hard to process.
Comment 3 Mark Lam 2015-04-02 20:45:32 PDT
(In reply to comment #2)
> Comment on attachment 250033 [details]
> the patch.
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=250033&action=review
> 
> > Source/JavaScriptCore/runtime/Options.cpp:397
> > +        fprintf(stream, "%s", m_entry.boolVal?"true":"false");
> 
> Missing spaces around the part of the operator.

Fixed locally.

> > Source/JavaScriptCore/runtime/Options.h:275
> > +    v(unsigned, slowPathAllocsBetweenGCs, 0, "debugging option to trigger a GC for every count of this number of slow path allocs") \
> 
> The description is a bit hard to process.

I changed this string to "force a GC on every Nth slow path alloc, where N is specified by this option".
Comment 4 Mark Lam 2015-04-02 20:47:57 PDT
Thanks for the review.  Landed in r182304: <http://trac.webkit.org/r182304>.