Bug 89802 - Value profiling should use tier-up threshold randomization to get more coverage
Summary: Value profiling should use tier-up threshold randomization to get more coverage
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords:
Depends on: 89953
Blocks: 89940
  Show dependency treegraph
 
Reported: 2012-06-22 21:57 PDT by Filip Pizlo
Modified: 2012-06-26 00:42 PDT (History)
2 users (show)

See Also:


Attachments
the patch (22.39 KB, patch)
2012-06-22 22:00 PDT, Filip Pizlo
buildbot: commit-queue-
Details | Formatted Diff | Diff
the patch (22.46 KB, patch)
2012-06-22 22:38 PDT, Filip Pizlo
barraclough: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
patch for landing, hopefully (22.47 KB, patch)
2012-06-25 15:56 PDT, Filip Pizlo
buildbot: commit-queue-
Details | Formatted Diff | Diff
moar fixes (20.71 KB, patch)
2012-06-25 16:29 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
and now, with a ChangeLog! (23.23 KB, patch)
2012-06-25 16:30 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2012-06-22 21:57:19 PDT
Currently we often end up optimizing code with incomplete value profiling coverage - as in, the typical predicted type will be based on only one recorded value.  We're pretty good at making reasonable guesses even with incomplete information, but the fact that the code relies on this is kind of sad.  It would be better if we could reliably get multiple recorded values.

We can do this, if we LUB (least-upper-bound, or ValueProfile::computeUpdatedPrediction()) all value profiles a few times between when profiling starts and when OSR is triggered.  One way to make that happen is to make the execution counter have some random value that is smaller than the required threshold for OSR.  This will cause each function to take an OSR slow path without actually triggering optimization, which will in turn give us an opportunity to LUB all value profiles.
Comment 1 Filip Pizlo 2012-06-22 22:00:53 PDT
Created attachment 149162 [details]
the patch
Comment 2 WebKit Review Bot 2012-06-22 22:04:23 PDT
Attachment 149162 [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/runtime/JSGlobalObject.h:178:  The parameter name "globalData" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/JSGlobalObject.h:178:  The parameter name "structure" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/JSGlobalObject.h:178:  The parameter name "globalObjectMethodTable" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 3 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Build Bot 2012-06-22 22:24:12 PDT
Comment on attachment 149162 [details]
the patch

Attachment 149162 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/13025585
Comment 4 Filip Pizlo 2012-06-22 22:38:03 PDT
Created attachment 149163 [details]
the patch

Fix style, attempt to fix Windows.
Comment 5 Build Bot 2012-06-22 23:00:43 PDT
Comment on attachment 149163 [details]
the patch

Attachment 149163 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/13027636
Comment 6 Filip Pizlo 2012-06-25 15:56:56 PDT
Created attachment 149377 [details]
patch for landing, hopefully

Already reviewed by Gavin.  Now I'm just trying to get Win to build.
Comment 7 Build Bot 2012-06-25 16:27:23 PDT
Comment on attachment 149377 [details]
patch for landing, hopefully

Attachment 149377 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/13103207
Comment 8 Filip Pizlo 2012-06-25 16:29:29 PDT
Created attachment 149387 [details]
moar fixes
Comment 9 Filip Pizlo 2012-06-25 16:30:28 PDT
Created attachment 149388 [details]
and now, with a ChangeLog!
Comment 10 Filip Pizlo 2012-06-25 19:14:52 PDT
Landed in http://trac.webkit.org/changeset/121215
Comment 11 Csaba Osztrogonác 2012-06-26 00:42:59 PDT
(In reply to comment #10)
> Landed in http://trac.webkit.org/changeset/121215

The new test introduced in it fails on 32 bit platforms. (GTK, Qt)
I filed a new bug report for it: https://bugs.webkit.org/show_bug.cgi?id=89953