Bug 150570 - Add logging to warn about under-estimated FTL inline cache sizes.
Summary: Add logging to warn about under-estimated FTL inline cache sizes.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-10-26 12:31 PDT by Mark Lam
Modified: 2015-10-26 14:47 PDT (History)
0 users

See Also:


Attachments
the patch. (1.59 KB, patch)
2015-10-26 12:35 PDT, Mark Lam
ggaren: review+
ggaren: commit-queue-
Details | Formatted Diff | Diff
Patch for landing. (3.79 KB, patch)
2015-10-26 14:34 PDT, Mark Lam
no flags 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-10-26 12:31:40 PDT
This makes it a little more convenient to check if something has changed and if we need to re-calibrate.
Comment 1 Mark Lam 2015-10-26 12:35:11 PDT
Created attachment 264062 [details]
the patch.
Comment 2 Geoffrey Garen 2015-10-26 12:55:50 PDT
Comment on attachment 264062 [details]
the patch.

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

> Source/JavaScriptCore/ftl/FTLCompile.cpp:177
> +    if (verboseCompilationEnabled()) {
> +        static size_t maxSize = 0;
> +        if (maxSize < actualCodeSize)
> +            maxSize = actualCodeSize;
> +        dataLogF("ALERT: Under-estimated FTL Inline Cache Size for %s: estimated %zu, actual %zu, max %zu\n", codeDescription, sizeOfInlineCode, actualCodeSize, maxSize);
> +    }

I'd like to be able to use this tool to figure out if we have undersized ICs by running benchmarks. But verbose compilation and dumping to stdout would fall over for benchmarks.

Can we log and then crash, with a special option dedicated to this?
Comment 3 Mark Lam 2015-10-26 13:03:42 PDT
Comment on attachment 264062 [details]
the patch.

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

>> Source/JavaScriptCore/ftl/FTLCompile.cpp:177
>> +    }
> 
> I'd like to be able to use this tool to figure out if we have undersized ICs by running benchmarks. But verbose compilation and dumping to stdout would fall over for benchmarks.
> 
> Can we log and then crash, with a special option dedicated to this?

I'll add 2 options:
1. assertICSizes - will crash if IC size is inadequate.  Useful for the automated regression test case (e.g. when running benchmarks).
2. dumpFailedICSizing - will dump to stdout and continue.  Useful for finding the max size of new ICs being added.
Comment 4 Mark Lam 2015-10-26 14:34:59 PDT
Created attachment 264077 [details]
Patch for landing.

Also added comments in the ChangeLog to describe how to use these options.
Comment 5 Mark Lam 2015-10-26 14:45:52 PDT
Comment on attachment 264077 [details]
Patch for landing.

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

> Source/JavaScriptCore/ChangeLog:10
> +        1. JSC_dumpFailedICSizing - dumps an error message if the FTL encounters an
> +           IC size estimates that are less than the actual needed code size.

"encounters an IC size estimates" ==> "encounters IC size estimates".  Will fix before landing.

> Source/JavaScriptCore/ChangeLog:13
> +           This option is useful for when we add a new IC and want to compute estimate
> +           sizes for the IC.  To do this:

"compute estimate sizes for" ==> "compute an estimated size for".  Will fix before landing.

> Source/JavaScriptCore/ChangeLog:17
> +           2. Implement a test suite with scenarios that exercises all the code paths in

"exercises" ==> "exercise".
Comment 6 Mark Lam 2015-10-26 14:47:16 PDT
Landed in r191602: <http://trac.webkit.org/r191602>.