Bug 88385 - Global object variable accesses should not require an extra load
Summary: Global object variable accesses should not require an extra load
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:
Blocks:
 
Reported: 2012-06-05 18:40 PDT by Filip Pizlo
Modified: 2012-06-06 17:50 PDT (History)
7 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2012-06-05 18:40:01 PDT
Patch forthcoming.
Comment 1 Filip Pizlo 2012-06-05 18:42:58 PDT
Created attachment 145910 [details]
it begins!
Comment 2 Filip Pizlo 2012-06-05 19:00:00 PDT
Created attachment 145912 [details]
more!
Comment 3 Filip Pizlo 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.
Comment 4 Filip Pizlo 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.
Comment 5 Filip Pizlo 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.
Comment 6 Filip Pizlo 2012-06-06 11:52:32 PDT
Created attachment 146079 [details]
the patch
Comment 7 WebKit Review Bot 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.
Comment 8 Geoffrey Garen 2012-06-06 12:06:58 PDT
Comment on attachment 146079 [details]
the patch

r=us
Comment 9 Build Bot 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
Comment 10 Early Warning System Bot 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
Comment 11 Filip Pizlo 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.
Comment 12 Filip Pizlo 2012-06-06 13:05:08 PDT
Created attachment 146092 [details]
the latest, not yet for landing
Comment 13 Filip Pizlo 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.
Comment 14 WebKit Review Bot 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.
Comment 15 Build Bot 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
Comment 16 Filip Pizlo 2012-06-06 15:22:34 PDT
Created attachment 146124 [details]
with mac build fixes
Comment 17 Build Bot 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
Comment 18 Filip Pizlo 2012-06-06 16:50:59 PDT
Created attachment 146144 [details]
trying to make windows green
Comment 19 Filip Pizlo 2012-06-06 17:50:17 PDT
Landed in http://trac.webkit.org/changeset/119655