Bug 88385

Summary:

Global object variable accesses should not require an extra load

Product:

WebKit

Reporter:

Filip Pizlo <fpizlo>

Component:

JavaScriptCore

Assignee:

Filip Pizlo <fpizlo>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

barraclough, ggaren, gustavo, philn, rakuco, webkit.review.bot, xan.lopez

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
it begins!	none
more!	none
the code is written	none
it's starting to do things	none
the patch	ggaren: review+, buildbot: commit-queue-
the latest, not yet for landing	none
almost ready	buildbot: commit-queue-
with mac build fixes	buildbot: commit-queue-
trying to make windows green	none

Filip Pizlo

Reported 2012-06-05 18:40:01 PDT

Patch forthcoming.

Attachments
it begins! (30.82 KB, patch) 2012-06-05 18:42 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
more! (43.06 KB, patch) 2012-06-05 19:00 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
the code is written (73.56 KB, patch) 2012-06-05 21:49 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
it's starting to do things (80.27 KB, patch) 2012-06-05 23:56 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (83.82 KB, patch) 2012-06-06 11:52 PDT, Filip Pizlo	ggaren: review+ buildbot: commit-queue-	Details Formatted Diff Diff
the latest, not yet for landing (86.96 KB, patch) 2012-06-06 13:05 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
almost ready (88.96 KB, patch) 2012-06-06 14:46 PDT, Filip Pizlo	buildbot: commit-queue-	Details Formatted Diff Diff
with mac build fixes (89.25 KB, patch) 2012-06-06 15:22 PDT, Filip Pizlo	buildbot: commit-queue-	Details Formatted Diff Diff
trying to make windows green (91.10 KB, patch) 2012-06-06 16:50 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
Show Obsolete (8) View All Add attachment proposed patch, testcase, etc.

Filip Pizlo

Comment 1 2012-06-05 18:42:58 PDT

Created attachment 145910 [details] it begins!

Filip Pizlo

Comment 2 2012-06-05 19:00:00 PDT

Created attachment 145912 [details] more!

Filip Pizlo

Comment 3 2012-06-05 19:00:57 PDT

Oops, didn't mean to put either patch up for EWS. The first one doesn't have the build foo for any non-Mac platforms. The second one, if it compiles, it will be a miracle.

Filip Pizlo

Comment 4 2012-06-05 21:49:32 PDT

Created attachment 145931 [details] the code is written Now I actually ought to compile it. And test it.

Filip Pizlo

Comment 5 2012-06-05 23:56:34 PDT

Created attachment 145947 [details] it's starting to do things Got it to compile and do the right thing for a stupidly simple test. Now the real testing starts.

Filip Pizlo

Comment 6 2012-06-06 11:52:32 PDT

Created attachment 146079 [details] the patch

WebKit Review Bot

Comment 7 2012-06-06 11:55:27 PDT

Attachment 146079 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1 Source/JavaScriptCore/runtime/JSSegmentedVariableObject.h:36: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/runtime/JSSegmentedVariableObject.h:39: "wtf/UnusedParam.h" already included at Source/JavaScriptCore/runtime/JSSegmentedVariableObject.h:36 [build/include] [4] Source/JavaScriptCore/jit/JITPropertyAccess.cpp:38: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 3 in 33 files If any of these errors are false positives, please file a bug against check-webkit-style.

Geoffrey Garen

Comment 8 2012-06-06 12:06:58 PDT

Comment on attachment 146079 [details] the patch r=us

Build Bot

Comment 9 2012-06-06 12:32:49 PDT

Comment on attachment 146079 [details] the patch Attachment 146079 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/12897884

Early Warning System Bot

Comment 10 2012-06-06 12:42:48 PDT

Comment on attachment 146079 [details] the patch Attachment 146079 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/12900877

Filip Pizlo

Comment 11 2012-06-06 13:04:14 PDT

Based on the buildbot feedback it appears that I have some minor issues to fix: - 32-bit build foo. - DOM calls into symbolTableStuff. Working on these now, but will first post an updated patch with some other fixes. This now appears to be a 1% across the board win.

Filip Pizlo

Comment 12 2012-06-06 13:05:08 PDT

Created attachment 146092 [details] the latest, not yet for landing

Filip Pizlo

Comment 13 2012-06-06 14:46:02 PDT

Created attachment 146115 [details] almost ready Win EWS might scream because of exports but other than that it should be good.

WebKit Review Bot

Comment 14 2012-06-06 14:50:52 PDT

Attachment 146115 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1 Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp:38: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/runtime/JSSegmentedVariableObject.h:36: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/runtime/JSSegmentedVariableObject.h:39: "wtf/UnusedParam.h" already included at Source/JavaScriptCore/runtime/JSSegmentedVariableObject.h:36 [build/include] [4] Source/JavaScriptCore/jit/JITPropertyAccess.cpp:38: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 4 in 35 files If any of these errors are false positives, please file a bug against check-webkit-style.

Build Bot

Comment 15 2012-06-06 15:13:05 PDT

Comment on attachment 146115 [details] almost ready Attachment 146115 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/12897929

Filip Pizlo

Comment 16 2012-06-06 15:22:34 PDT

Created attachment 146124 [details] with mac build fixes

Build Bot

Comment 17 2012-06-06 16:47:30 PDT

Comment on attachment 146124 [details] with mac build fixes Attachment 146124 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/12897968

Filip Pizlo

Comment 18 2012-06-06 16:50:59 PDT

Created attachment 146144 [details] trying to make windows green

Filip Pizlo

Comment 19 2012-06-06 17:50:17 PDT

Landed in http://trac.webkit.org/changeset/119655

Note You need to log in before you can comment on or make changes to this bug.