Bug 150570 – Add logging to warn about under-estimated FTL inline cache sizes.

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Mark Lam]

This makes it a little more convenient to check if something has changed and if we need to re-calibrate.

[Mark Lam]

Created attachment 264062 [details]
the patch.

[Geoffrey Garen]

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?

[Mark Lam]

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.

[Mark Lam]

Created attachment 264077 [details]
Patch for landing.

Also added comments in the ChangeLog to describe how to use these options.

[Mark Lam]

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".

[Mark Lam]

Landed in r191602: <http://trac.webkit.org/r191602>.