Bug 67694

Summary: Value profling and execution count profiling is performed even for code that cannot be optimized
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, fpizlo, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
work in progress
none
the patch
barraclough: review+
the patch - fix style and build, hopefully
none
the patch - fix conflict in ChangeLog
none
the patch - added a helpful assertion
none
the patch none

Description Filip Pizlo 2011-09-06 20:38:19 PDT
Value profiling and execution count profiling are necessary for invoking an optimizing compiler when tiered compilation is enabled.  But some code blocks cannot be compiled with an optimizing compiler.  Currently, we still incur the overhead of profiling even though these code blocks will never be optimized.
Comment 1 Filip Pizlo 2011-09-06 20:40:59 PDT
Created attachment 106539 [details]
work in progress
Comment 2 Filip Pizlo 2011-09-06 22:07:45 PDT
This is a pure win.  Here's the comparison between ToT with tiered compilation and this patch, with tiered compilation.



Benchmark report for V8.

VMs tested:
"TipOfTree" at /Volumes/Data/pizlo/quinary/OpenSource/WebKitBuild/Release/jsc
"LessProfile" at /Volumes/Data/pizlo/quartary/OpenSource/WebKitBuild/Release/jsc

Collected 12 samples per benchmark/VM, with 4 VM invocations per benchmark. Used 1
benchmark iteration per VM invocation for warm-up. Used the jsc-specific preciseTime()
function to get microsecond-level timing. Reporting benchmark execution times with 95%
confidence intervals in milliseconds.

                     TipOfTree              LessProfile                                   

crypto            81.1727+-0.3694    ?    81.2600+-0.5533       ?
deltablue        256.3750+-0.9625    ^   251.4166+-0.8416       ^ definitely 1.0197x faster
earley-boyer     101.1767+-0.6177    ^    95.7393+-0.5327       ^ definitely 1.0568x faster
raytrace          81.3929+-0.4155    ^    76.3499+-0.7089       ^ definitely 1.0661x faster
regexp           111.0145+-1.0576    ^   108.6231+-0.4444       ^ definitely 1.0220x faster
richards         215.4840+-0.9667    ?   215.7928+-0.9476       ?
splay            102.1599+-0.4318    ^   100.9486+-0.3325       ^ definitely 1.0120x faster

<arithmetic>     135.5394+-0.2250    ^   132.8758+-0.2813       ^ definitely 1.0200x faster
<geometric>      122.7006+-0.2233    ^   119.7584+-0.2910       ^ definitely 1.0246x faster
<harmonic>       113.2119+-0.2301    ^   110.1033+-0.3318       ^ definitely 1.0282x faster
Comment 3 Filip Pizlo 2011-09-06 22:11:17 PDT
Created attachment 106545 [details]
the patch

Still testing, but this looks like win.
Comment 4 WebKit Review Bot 2011-09-06 22:13:21 PDT
Attachment 106545 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1

Source/JavaScriptCore/jit/JIT.cpp:48:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/dfg/DFGCapabilities.cpp:48:  Non-label code inside switch statements should be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/bytecode/CodeBlock.cpp:35:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 3 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Gavin Barraclough 2011-09-06 23:44:05 PDT
Comment on attachment 106545 [details]
the patch

Seems reasonable.  We could probably save the extra pass if we wanted (track at the point opcodes are added to the byte code when unsupported ones are added), but I guess this is hardly an important cost.

Looks like you need to fix windows .defs file & also the alphabetical sorting issues (ascii ordering, so all caps are < all lowercase).
Comment 6 Filip Pizlo 2011-09-07 11:38:13 PDT
Created attachment 106608 [details]
the patch - fix style and build, hopefully

Will cq+ after bots are reasonably happy and I run a few more tests.
Comment 7 Filip Pizlo 2011-09-07 11:39:37 PDT
Created attachment 106610 [details]
the patch - fix conflict in ChangeLog
Comment 8 Filip Pizlo 2011-09-07 11:53:44 PDT
Created attachment 106615 [details]
the patch - added a helpful assertion

Still waiting for tests (both tiering enabled and disabled).
Comment 9 Filip Pizlo 2011-09-08 12:24:01 PDT
Created attachment 106772 [details]
the patch

Will watch the bots for a bit and commit.
Comment 10 Filip Pizlo 2011-09-08 13:22:32 PDT
Comment on attachment 106772 [details]
the patch

Looks like I've fixed Windows.  Committting.
Comment 11 WebKit Review Bot 2011-09-08 14:37:11 PDT
Comment on attachment 106772 [details]
the patch

Clearing flags on attachment: 106772

Committed r94802: <http://trac.webkit.org/changeset/94802>
Comment 12 WebKit Review Bot 2011-09-08 14:37:16 PDT
All reviewed patches have been landed.  Closing bug.