Bug 98606

Summary:

JSC should infer when indexed storage contains only integers or doubles

Product:

WebKit

Reporter:

Filip Pizlo <fpizlo>

Component:

JavaScriptCore

Assignee:

Filip Pizlo <fpizlo>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

barraclough, dino, ggaren, gustavo, gyuyoung.kim, mark.lam, mhahnenberg, msaboff, oliver, ossy, philn, rakuco, webkit.review.bot, xan.lopez

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

99263, 99371, 97288, 98997, 99260, 99261, 99262, 99269, 99287, 99557, 99905, 100052, 100311, 100328, 100599, 100620, 100827, 101174, 101704, 101706, 101740, 101746, 101832

Bug Blocks:

100870, 101145, 101718

Attachments:

Description	Flags
work in progress	none
moar	none
more things	none
even more things	none
more profiling	none
done with 64-bit LLInt and baseline JIT	none
did more things	none
bigger is better!	none
more!	none
I think the 64-bit bit code is complete	none
it compiles	none
it runs things	none
it speeds up real programs	none
rebased and fighting regressions	none
it seems to be passing some tests	none
more tests pass	none
almost done with 32-bit	none
it appears to totally work	webkit-ews: commit-queue-
build systems should be in order	none
rebase	buildbot: commit-queue-
fixes and rebasing	buildbot: commit-queue-
bigger speed-up	buildbot: commit-queue-
rebase and address Windows fail	buildbot: commit-queue-
more Windows fixes	none
rebase	none
rebase	none
rebase	oliver: review+, buildbot: commit-queue-
patch for landing	none
patch for landing	buildbot: commit-queue-
fix more windows	none

Filip Pizlo

Reported 2012-10-06 15:55:26 PDT

This is another step to full-blown array type inference.

Attachments
work in progress (35.46 KB, patch) 2012-10-16 17:59 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
moar (59.26 KB, patch) 2012-10-16 18:50 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
more things (75.20 KB, patch) 2012-10-18 15:52 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
even more things (106.12 KB, patch) 2012-10-19 12:46 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
more profiling (122.91 KB, patch) 2012-10-19 23:25 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
done with 64-bit LLInt and baseline JIT (141.77 KB, patch) 2012-10-20 13:02 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
did more things (150.85 KB, patch) 2012-10-22 16:49 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
bigger is better! (170.41 KB, patch) 2012-10-24 14:43 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
more! (193.73 KB, patch) 2012-10-24 21:23 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
I think the 64-bit bit code is complete (226.55 KB, patch) 2012-10-25 11:46 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
it compiles (255.10 KB, patch) 2012-10-26 01:08 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
it runs things (269.05 KB, patch) 2012-10-26 18:20 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
it speeds up real programs (281.48 KB, patch) 2012-10-27 17:49 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
rebased and fighting regressions (266.66 KB, patch) 2012-10-28 00:57 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
it seems to be passing some tests (272.25 KB, patch) 2012-10-29 18:15 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
more tests pass (267.65 KB, patch) 2012-10-30 12:52 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
almost done with 32-bit (307.51 KB, patch) 2012-10-30 16:44 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
it appears to totally work (337.70 KB, patch) 2012-10-31 19:40 PDT, Filip Pizlo	webkit-ews: commit-queue-	Details Formatted Diff Diff
build systems should be in order (341.41 KB, patch) 2012-11-01 14:43 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
rebase (340.76 KB, patch) 2012-11-01 15:04 PDT, Filip Pizlo	buildbot: commit-queue-	Details Formatted Diff Diff
fixes and rebasing (341.25 KB, patch) 2012-11-02 16:35 PDT, Filip Pizlo	buildbot: commit-queue-	Details Formatted Diff Diff
bigger speed-up (341.36 KB, patch) 2012-11-04 17:26 PST, Filip Pizlo	buildbot: commit-queue-	Details Formatted Diff Diff
rebase and address Windows fail (341.27 KB, patch) 2012-11-05 10:46 PST, Filip Pizlo	buildbot: commit-queue-	Details Formatted Diff Diff
more Windows fixes (342.28 KB, patch) 2012-11-05 17:55 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
rebase (342.35 KB, patch) 2012-11-05 20:49 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
rebase (342.85 KB, patch) 2012-11-05 23:10 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
rebase (341.06 KB, patch) 2012-11-06 11:32 PST, Filip Pizlo	oliver: review+ buildbot: commit-queue-	Details Formatted Diff Diff
patch for landing (342.04 KB, patch) 2012-11-06 17:16 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
patch for landing (345.52 KB, patch) 2012-11-06 17:49 PST, Filip Pizlo	buildbot: commit-queue-	Details Formatted Diff Diff
fix more windows (346.91 KB, patch) 2012-11-07 01:03 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
Show Obsolete (29) View All Add attachment proposed patch, testcase, etc.

Filip Pizlo

Comment 1 2012-10-16 17:59:40 PDT

Created attachment 169066 [details] work in progress

Filip Pizlo

Comment 2 2012-10-16 18:50:35 PDT

Created attachment 169069 [details] moar

Filip Pizlo

Comment 3 2012-10-18 15:52:06 PDT

Created attachment 169493 [details] more things I think I have a story for array allocation inference.

Filip Pizlo

Comment 4 2012-10-19 12:46:53 PDT

Created attachment 169678 [details] even more things

Filip Pizlo

Comment 5 2012-10-19 23:25:05 PDT

Created attachment 169757 [details] more profiling I think that the array allocation profiling story is now done. All LLInt and baseline JIT array allocation sites should be doing the right thing, and the GC should be helping out. (I'm speaking as if the code is doing things. It's not. It doesn't compile yet.)

Filip Pizlo

Comment 6 2012-10-20 13:02:58 PDT

Created attachment 169771 [details] done with 64-bit LLInt and baseline JIT Now I just need to write the DFG part! And the 32-bit parts. And get the other platforms building.

Filip Pizlo

Comment 7 2012-10-22 16:49:59 PDT

Created attachment 170026 [details] did more things Started doing the DFG stuff.

Filip Pizlo

Comment 8 2012-10-24 14:43:36 PDT

Created attachment 170475 [details] bigger is better!

Filip Pizlo

Comment 9 2012-10-24 21:23:47 PDT

Created attachment 170545 [details] more! Lots of meaty DFG stuff done.

Filip Pizlo

Comment 10 2012-10-25 11:46:52 PDT

Created attachment 170699 [details] I think the 64-bit bit code is complete Still haven't tried compiling it.

Filip Pizlo

Comment 11 2012-10-26 01:08:16 PDT

Created attachment 170842 [details] it compiles It compiles.

Filip Pizlo

Comment 12 2012-10-26 18:20:48 PDT

Created attachment 171066 [details] it runs things Looks like 54% speed-up on doubly programs and a 14% speed-up on inty programs.

Filip Pizlo

Comment 13 2012-10-27 17:49:19 PDT

Created attachment 171111 [details] it speeds up real programs Still working on some regressions, though.

Filip Pizlo

Comment 14 2012-10-28 00:57:51 PDT

Created attachment 171125 [details] rebased and fighting regressions There is still some weirdness in the profiling. Figuring it out now.

Filip Pizlo

Comment 15 2012-10-29 18:15:07 PDT

Created attachment 171351 [details] it seems to be passing some tests So far: 1% regression on SunSpider, >2% progression on V8v7, >2% progression on Kraken.

Filip Pizlo

Comment 16 2012-10-30 12:52:18 PDT

Created attachment 171507 [details] more tests pass

Filip Pizlo

Comment 17 2012-10-30 16:44:38 PDT

Created attachment 171547 [details] almost done with 32-bit Still a bit more work to do though.

Filip Pizlo

Comment 18 2012-10-31 19:40:44 PDT

Created attachment 171759 [details] it appears to totally work Will mark r? as soon as I finish a few more tests.

WebKit Review Bot

Comment 19 2012-10-31 19:44:18 PDT

Attachment 171759 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/API/JSObjectRef.cpp'..." exit_code: 1 Source/JavaScriptCore/runtime/JSObject.h:956: The parameter name "globalData" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/bytecode/Instruction.h:50: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1852: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1872: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1876: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1881: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1888: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1890: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1896: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1899: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1906: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2067: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2071: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2080: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2082: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2089: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2092: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2095: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2100: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:48: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:49: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:56: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:57: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:58: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 24 in 81 files If any of these errors are false positives, please file a bug against check-webkit-style.

Early Warning System Bot

Comment 20 2012-10-31 19:49:32 PDT

Comment on attachment 171759 [details] it appears to totally work Attachment 171759 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/14693349

Early Warning System Bot

Comment 21 2012-10-31 19:50:39 PDT

Comment on attachment 171759 [details] it appears to totally work Attachment 171759 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/14697131

Build Bot

Comment 22 2012-10-31 20:06:46 PDT

Comment on attachment 171759 [details] it appears to totally work Attachment 171759 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/14663402

EFL EWS Bot

Comment 23 2012-10-31 20:57:37 PDT

Comment on attachment 171759 [details] it appears to totally work Attachment 171759 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/14678366

Filip Pizlo

Comment 24 2012-11-01 12:03:04 PDT

Comment on attachment 171759 [details] it appears to totally work This is now passing 32-bit and 64-bit tests for me. It appears there are some style errors (I will fix the real ones) and I still need to update build files for Qt, Gtk, Win, and EFL. But, the patch is in good shape other than that and so it's ready for review!

Filip Pizlo

Comment 25 2012-11-01 14:43:01 PDT

Created attachment 171934 [details] build systems should be in order Fixed build systems. Random 32-bit fixes.

Filip Pizlo

Comment 26 2012-11-01 15:04:32 PDT

Created attachment 171941 [details] rebase

WebKit Review Bot

Comment 27 2012-11-01 15:16:21 PDT

Attachment 171941 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/API/JSObjectRef.cpp'..." exit_code: 1 Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2067: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2071: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2080: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2082: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2089: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2092: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2095: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2100: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/runtime/JSObject.h:956: The parameter name "globalData" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/bytecode/Instruction.h:50: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1852: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1872: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1876: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1881: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1888: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1890: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1896: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1899: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1906: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:48: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:49: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:56: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:57: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:58: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 24 in 84 files If any of these errors are false positives, please file a bug against check-webkit-style.

Build Bot

Comment 28 2012-11-01 17:32:15 PDT

Comment on attachment 171941 [details] rebase Attachment 171941 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/14678663

Yuqiang Xian

Comment 29 2012-11-01 19:02:12 PDT

Comment on attachment 171941 [details] rebase View in context: https://bugs.webkit.org/attachment.cgi?id=171941&action=review Thanks for taking care! > Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:3689 > + m_jit.move(TrustedImm64(bitwise_cast<intptr_t>(0.0 / 0.0)), scratchGPR); Can we use bitwise_cast<int64_t>? x32 would be happy for this. :) > Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:85 > + m_jit.move(TrustedImm64(bitwise_cast<intptr_t>(0.0 / 0.0)), scratchGPR); Can we use bitwise_cast<int64_t>? x32 would be happy for this. :)

Filip Pizlo

Comment 30 2012-11-02 00:28:04 PDT

(In reply to comment #29) > (From update of attachment 171941 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=171941&action=review > > Thanks for taking care! > > > Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:3689 > > + m_jit.move(TrustedImm64(bitwise_cast<intptr_t>(0.0 / 0.0)), scratchGPR); > > Can we use bitwise_cast<int64_t>? x32 would be happy for this. :) > > > Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:85 > > + m_jit.move(TrustedImm64(bitwise_cast<intptr_t>(0.0 / 0.0)), scratchGPR); > > Can we use bitwise_cast<int64_t>? x32 would be happy for this. :) Yup, I can make these changes.

Filip Pizlo

Comment 31 2012-11-02 16:35:18 PDT

Created attachment 172173 [details] fixes and rebasing

WebKit Review Bot

Comment 32 2012-11-02 16:39:00 PDT

Attachment 172173 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/API/JSObjectRef.cpp'..." exit_code: 1 Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2067: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2071: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2080: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2082: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2089: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2092: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2095: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2100: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/runtime/JSObject.h:956: The parameter name "globalData" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/bytecode/Instruction.h:50: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1852: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1872: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1876: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1881: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1888: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1890: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1896: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1899: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1906: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:48: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:49: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:56: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:57: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:58: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 24 in 84 files If any of these errors are false positives, please file a bug against check-webkit-style.

Build Bot

Comment 33 2012-11-02 17:41:32 PDT

Comment on attachment 172173 [details] fixes and rebasing Attachment 172173 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/14719072

Filip Pizlo

Comment 34 2012-11-04 17:26:16 PST

Created attachment 172249 [details] bigger speed-up Now up to 4.8% faster on V8v7.

WebKit Review Bot

Comment 35 2012-11-04 17:30:24 PST

Attachment 172249 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/API/JSObjectRef.cpp'..." exit_code: 1 Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2067: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2071: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2080: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2082: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2089: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2092: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2095: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2100: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/runtime/JSObject.h:956: The parameter name "globalData" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/bytecode/Instruction.h:50: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1852: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1872: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1876: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1881: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1888: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1890: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1896: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1899: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1906: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:48: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:49: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:56: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:57: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:58: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 24 in 84 files If any of these errors are false positives, please file a bug against check-webkit-style.

Build Bot

Comment 36 2012-11-04 17:53:52 PST

Comment on attachment 172249 [details] bigger speed-up Attachment 172249 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/14744016

Filip Pizlo

Comment 37 2012-11-04 19:47:57 PST

(In reply to comment #36) > (From update of attachment 172249 [details]) > Attachment 172249 [details] did not pass win-ews (win): > Output: http://queues.webkit.org/results/14744016 Looks like VC++ is too pedantic to compile 0.0/0.0. Weird. I will fix by switching our references to NaN to a #define. See https://bugs.webkit.org/show_bug.cgi?id=101174

Filip Pizlo

Comment 38 2012-11-05 10:46:20 PST

Created attachment 172362 [details] rebase and address Windows fail

WebKit Review Bot

Comment 39 2012-11-05 10:54:02 PST

Attachment 172362 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/API/JSObjectRef.cpp'..." exit_code: 1 Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2067: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2071: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2080: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2082: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2089: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2092: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2095: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2100: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/runtime/JSObject.h:956: The parameter name "globalData" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/bytecode/Instruction.h:50: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1852: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1872: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1876: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1881: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1888: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1890: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1896: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1899: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1906: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:48: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:49: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:56: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:57: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:58: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 24 in 84 files If any of these errors are false positives, please file a bug against check-webkit-style.

Build Bot

Comment 40 2012-11-05 11:17:59 PST

Comment on attachment 172362 [details] rebase and address Windows fail Attachment 172362 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/14744204

Filip Pizlo

Comment 41 2012-11-05 17:55:42 PST

Created attachment 172458 [details] more Windows fixes

WebKit Review Bot

Comment 42 2012-11-05 18:04:19 PST

Attachment 172458 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/API/JSObjectRef.cpp'..." exit_code: 1 Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2067: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2071: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2080: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2082: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2089: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2092: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2095: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2100: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/runtime/JSObject.h:955: The parameter name "globalData" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/bytecode/Instruction.h:50: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1852: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1872: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1876: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1881: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1888: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1890: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1896: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1899: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1906: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:48: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:49: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:56: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:57: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:58: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 24 in 85 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 43 2012-11-05 20:49:01 PST

Created attachment 172479 [details] rebase

Filip Pizlo

Comment 44 2012-11-05 23:10:49 PST

Created attachment 172493 [details] rebase

WebKit Review Bot

Comment 45 2012-11-05 23:13:38 PST

Attachment 172493 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/API/JSObjectRef.cpp'..." exit_code: 1 Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2067: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2071: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2080: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2082: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2089: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2092: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2095: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2100: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/bytecode/Instruction.h:50: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/bytecode/ArrayProfile.h:48: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:49: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:56: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:57: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:58: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/runtime/JSObject.h:955: The parameter name "globalData" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1852: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1872: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1876: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1881: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1888: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1890: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1896: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1899: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1906: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Total errors found: 24 in 86 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 46 2012-11-06 11:32:46 PST

Created attachment 172625 [details] rebase

WebKit Review Bot

Comment 47 2012-11-06 11:36:32 PST

Attachment 172625 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/API/JSObjectRef.cpp'..." exit_code: 1 Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2067: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2071: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2080: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2082: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2089: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2092: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2095: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2100: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/bytecode/Instruction.h:50: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/bytecode/ArrayProfile.h:48: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:49: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:56: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:57: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:58: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/runtime/JSObject.h:955: The parameter name "globalData" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1852: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1872: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1876: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1881: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1888: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1890: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1896: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1899: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1906: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Total errors found: 24 in 86 files If any of these errors are false positives, please file a bug against check-webkit-style.

Build Bot

Comment 48 2012-11-06 12:02:32 PST

Comment on attachment 172625 [details] rebase Attachment 172625 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/14744479

Oliver Hunt

Comment 49 2012-11-06 16:59:30 PST

Comment on attachment 172625 [details] rebase View in context: https://bugs.webkit.org/attachment.cgi?id=172625&action=review r=me, one concern i have (although not introduced by this patch) is that there are a number of places where we are multiplying an attacker controlled value and using that to control an allocation size. I don't think op_new_array_buffer really needs a profile as we should be able to just record that when we originally generate the instruction. Also couldn't we use this information to improve the GC? If we know the indexing type is int or double then there is no need to mark the array storage. Anyhoo, r=me with the exception of windows bustage this seems fine to land. > Source/JavaScriptCore/jit/JITExceptions.cpp:45 > ExceptionHandler genericThrow(JSGlobalData* globalData, ExecState* callFrame, JSValue exceptionValue, unsigned vPCIndex) > { > ASSERT(exceptionValue); > - > + > globalData->exception = JSValue(); > HandlerInfo* handler = globalData->interpreter->throwException(callFrame, exceptionValue, vPCIndex); // This may update callFrame & exceptionValue! > globalData->exception = exceptionValue; Reverteration here

Filip Pizlo

Comment 50 2012-11-06 17:01:25 PST

(In reply to comment #49) > (From update of attachment 172625 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=172625&action=review > > r=me, one concern i have (although not introduced by this patch) is that there are a number of places where we are multiplying an attacker controlled value and using that to control an allocation size. > > I don't think op_new_array_buffer really needs a profile as we should be able to just record that when we originally generate the instruction. It does need the profile. Consider: Some thingy: var a = [1,2,3]; Some other thingy: a[4] = 0.5; Array allocation profiling will hint to us that we should allocate 'a' as an array of doubles, if the other thingy happens often enough. > > Also couldn't we use this information to improve the GC? If we know the indexing type is int or double then there is no need to mark the array storage. This patch already does that! :-) > > Anyhoo, r=me with the exception of windows bustage this seems fine to land. > > > Source/JavaScriptCore/jit/JITExceptions.cpp:45 > > ExceptionHandler genericThrow(JSGlobalData* globalData, ExecState* callFrame, JSValue exceptionValue, unsigned vPCIndex) > > { > > ASSERT(exceptionValue); > > - > > + > > globalData->exception = JSValue(); > > HandlerInfo* handler = globalData->interpreter->throwException(callFrame, exceptionValue, vPCIndex); // This may update callFrame & exceptionValue! > > globalData->exception = exceptionValue; > > Reverteration here Ackitude.

Filip Pizlo

Comment 51 2012-11-06 17:16:25 PST

Created attachment 172680 [details] patch for landing Fixing windows

Filip Pizlo

Comment 52 2012-11-06 17:49:50 PST

Created attachment 172686 [details] patch for landing Rebased past Oliver's monster change.

WebKit Review Bot

Comment 53 2012-11-06 17:53:54 PST

Attachment 172686 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/API/JSObjectRef.cpp'..." exit_code: 1 Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2067: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2071: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2080: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2082: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2089: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2092: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2095: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2100: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/bytecode/Instruction.h:50: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/bytecode/ArrayProfile.h:48: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:49: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:56: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:57: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:58: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/runtime/JSObject.h:955: The parameter name "globalData" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1852: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1872: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1876: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1881: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1888: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1890: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1896: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1899: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1906: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Total errors found: 24 in 88 files If any of these errors are false positives, please file a bug against check-webkit-style.

Build Bot

Comment 54 2012-11-06 18:24:53 PST

Comment on attachment 172686 [details] patch for landing Attachment 172686 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/14744593

Filip Pizlo

Comment 55 2012-11-07 01:03:01 PST

Created attachment 172731 [details] fix more windows

WebKit Review Bot

Comment 56 2012-11-07 01:06:42 PST

Attachment 172731 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/API/JSObjectRef.cpp'..." exit_code: 1 Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2067: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2071: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2080: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2082: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2089: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2092: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2095: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2100: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/bytecode/Instruction.h:50: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/bytecode/ArrayProfile.h:48: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:49: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:56: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:57: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/bytecode/ArrayProfile.h:58: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/runtime/JSObject.h:955: The parameter name "globalData" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1852: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1872: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1876: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1881: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1888: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1890: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1896: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1899: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1906: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Total errors found: 24 in 89 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 57 2012-11-08 14:29:09 PST

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

Csaba Osztrogonác

Comment 58 2012-11-08 22:05:04 PST

(In reply to comment #57) > Landed in http://trac.webkit.org/changeset/133953 It broke the MIPS build: https://bugs.webkit.org/show_bug.cgi?id=101704 and the ARM_TRADITIONAL build: https://bugs.webkit.org/show_bug.cgi?id=101706

Csaba Osztrogonác

Comment 59 2012-11-13 23:52:16 PST

(In reply to comment #57) > Landed in http://trac.webkit.org/changeset/133953 It broke the build on ARM and MIPS - https://bugs.webkit.org/show_bug.cgi?id=101740 Could you review the proposed patches?

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