Bug 89802

Summary:

Value profiling should use tier-up threshold randomization to get more coverage

Product:

WebKit

Reporter:

Filip Pizlo <fpizlo>

Component:

JavaScriptCore

Assignee:

Filip Pizlo <fpizlo>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ossy, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

89953

Bug Blocks:

89940

Attachments:

Description	Flags
the patch	buildbot: commit-queue-
the patch	barraclough: review+, buildbot: commit-queue-
patch for landing, hopefully	buildbot: commit-queue-
moar fixes	none
and now, with a ChangeLog!	none

Filip Pizlo

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

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
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.

Filip Pizlo

Comment 1 2012-06-22 22:00:53 PDT

Created attachment 149162 [details] the patch

WebKit Review Bot

Comment 2 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.

Build Bot

Comment 3 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

Filip Pizlo

Comment 4 2012-06-22 22:38:03 PDT

Created attachment 149163 [details] the patch Fix style, attempt to fix Windows.

Build Bot

Comment 5 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

Filip Pizlo

Comment 6 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.