Most of the functionality is there to allow call frames to be reified in the case of inlining and uses of function.arguments, but a few pieces of glue are missing.
Created attachment 113230 [details] the patch This patch looks scary big, but most of that is the expectations file and a bunch of code motion to reduce duplication.
Looks to be performance-neutral. Benchmark report for SunSpider, V8, and Kraken. VMs tested: "TipOfTree" at /Volumes/Data/pizlo/quinary/OpenSource/WebKitBuild/Release/jsc "FixArguments" at /Volumes/Data/pizlo/tertiary/OpenSource/WebKitBuild/Release/jsc Collected 12 samples per benchmark/VM, with 4 VM invocations per benchmark. Emitted a call to gc() between sample measurements. Used 1 benchmark iteration per VM invocation for warm-up. Used the jsc-specific preciseTime() function to get microsecond-level timing. Reporting benchmark execution times with 95% confidence intervals in milliseconds. TipOfTree FixArguments SunSpider: 3d-cube 7.6499+-0.2593 7.6162+-0.1762 3d-morph 7.9063+-0.1820 ? 8.0282+-0.0711 ? might be 1.0154x slower 3d-raytrace 7.6961+-0.1969 ? 7.7956+-0.2230 ? might be 1.0129x slower access-binary-trees 1.6078+-0.0605 ? 1.7233+-0.0965 ? might be 1.0718x slower access-fannkuch 6.6464+-0.1293 6.5501+-0.1310 might be 1.0147x faster access-nbody 3.9135+-0.1152 3.7795+-0.1050 might be 1.0355x faster access-nsieve 2.7246+-0.0600 2.6066+-0.0895 might be 1.0452x faster bitops-3bit-bits-in-byte 1.3250+-0.0306 ? 1.3508+-0.0338 ? might be 1.0195x slower bitops-bits-in-byte 2.3868+-0.0435 ? 2.4308+-0.0609 ? might be 1.0184x slower bitops-bitwise-and 3.4625+-0.0592 ? 3.4809+-0.1118 ? bitops-nsieve-bits 5.5378+-0.1221 5.5309+-0.1361 controlflow-recursive 2.1887+-0.0393 2.1593+-0.0368 might be 1.0136x faster crypto-aes 7.9143+-0.2241 7.7961+-0.2043 might be 1.0152x faster crypto-md5 2.7922+-0.0625 ? 2.7973+-0.1063 ? crypto-sha1 2.5486+-0.0869 2.5365+-0.0548 date-format-tofte 10.8129+-0.4533 10.3446+-0.3227 might be 1.0453x faster date-format-xparb 9.0835+-0.2038 ? 9.5358+-0.4901 ? might be 1.0498x slower math-cordic 6.5703+-0.1055 ? 6.7803+-0.1592 ? might be 1.0320x slower math-partial-sums 7.7829+-0.1472 7.7043+-0.1182 might be 1.0102x faster math-spectral-norm 2.6944+-0.0389 ^ 2.5631+-0.0418 ^ definitely 1.0513x faster regexp-dna 11.9887+-0.2775 11.8429+-0.2067 might be 1.0123x faster string-base64 4.2709+-0.2253 ? 4.3151+-0.1974 ? might be 1.0103x slower string-fasta 6.5787+-0.1493 ? 6.6469+-0.2670 ? might be 1.0104x slower string-tagcloud 12.0964+-0.5036 ? 12.1212+-0.2836 ? string-unpack-code 21.3786+-0.5177 ? 21.5614+-0.5279 ? string-validate-input 5.4997+-0.1880 5.3993+-0.1570 might be 1.0186x faster <arithmetic> * 6.3484+-0.0278 6.3460+-0.0306 <geometric> 5.1016+-0.0212 5.0982+-0.0284 <harmonic> 4.0599+-0.0298 ? 4.0683+-0.0353 ? TipOfTree FixArguments V8: crypto 75.1757+-0.3731 ? 75.2230+-0.3923 ? deltablue 169.7191+-1.1452 ? 170.4428+-0.9564 ? earley-boyer 91.8773+-0.5745 ? 92.1749+-0.4571 ? raytrace 64.9346+-0.9143 64.6376+-0.7337 regexp 107.6506+-0.3287 ! 109.3592+-0.3937 ! definitely 1.0159x slower richards 128.6929+-0.4510 128.4158+-0.4081 splay 74.1930+-1.5509 ? 74.5736+-1.1579 ? <arithmetic> 101.7490+-0.2789 ? 102.1181+-0.2964 ? <geometric> * 96.5442+-0.2892 ? 96.8561+-0.3205 ? <harmonic> 92.0604+-0.3245 ? 92.3077+-0.3561 ? TipOfTree FixArguments Kraken: ai-astar 508.4903+-3.3297 ^ 499.9517+-1.4941 ^ definitely 1.0171x faster audio-beat-detection 194.7723+-2.8623 193.5630+-2.1211 audio-dft 282.1170+-2.5872 279.8534+-2.8109 audio-fft 126.9287+-0.9359 ? 127.5717+-0.9902 ? audio-oscillator 255.2655+-1.3368 ? 256.5465+-1.9383 ? imaging-darkroom 307.8138+-4.4949 307.2572+-4.4170 imaging-desaturate 231.2232+-0.7844 230.7775+-1.2691 imaging-gaussian-blur 563.5772+-2.1326 560.4199+-2.0540 json-parse-financial 57.7405+-0.3588 ! 58.8814+-0.2778 ! definitely 1.0198x slower json-stringify-tinderbox 70.7785+-0.5569 ^ 69.2321+-0.4219 ^ definitely 1.0223x faster stanford-crypto-aes 99.3010+-0.7644 98.6047+-1.5776 stanford-crypto-ccm 101.8039+-0.6532 ? 101.9548+-1.0385 ? stanford-crypto-pbkdf2 197.2721+-1.2126 ? 198.6930+-1.7125 ? stanford-crypto-sha256-iterative 82.6257+-0.4992 ^ 81.1939+-0.4549 ^ definitely 1.0176x faster <arithmetic> * 219.9793+-0.6961 ^ 218.8929+-0.3694 ^ definitely 1.0050x faster <geometric> 174.5806+-0.5142 173.9682+-0.3012 <harmonic> 139.5624+-0.3978 139.1772+-0.3166 TipOfTree FixArguments All benchmarks: <arithmetic> 84.1917+-0.1979 83.9218+-0.1213 <geometric> 22.6422+-0.0496 22.6210+-0.0747 <harmonic> 7.1424+-0.0509 ? 7.1566+-0.0606 ? TipOfTree FixArguments Geomean of preferred means: <scaled-result> 51.2766+-0.1098 51.2409+-0.1156
Attachment 113230 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1 Source/JavaScriptCore/runtime/Arguments.h:176: One line control clauses should not use braces. [whitespace/braces] [4] Total errors found: 1 in 9 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 113232 [details] the patch - fix style
Comment on attachment 113232 [details] the patch - fix style View in context: https://bugs.webkit.org/attachment.cgi?id=113232&action=review > Source/JavaScriptCore/runtime/Arguments.h:170 > + ASSERT(argc == numParameters + 1); What does ASSERT compile to if building without assertions? - does it compile to a do-nothing statement, or an empty one? If it compiles to an empty one, this could be a bug! - I think this should be: ASSERT(!callFrame->isInlineCallFrame() || (argc == numParameters + 1));
Comment on attachment 113232 [details] the patch - fix style r=me
Comment on attachment 113232 [details] the patch - fix style View in context: https://bugs.webkit.org/attachment.cgi?id=113232&action=review >> Source/JavaScriptCore/runtime/Arguments.h:170 >> + ASSERT(argc == numParameters + 1); > > What does ASSERT compile to if building without assertions? - does it compile to a do-nothing statement, or an empty one? > If it compiles to an empty one, this could be a bug! - I think this should be: > > ASSERT(!callFrame->isInlineCallFrame() || (argc == numParameters + 1)); The ASSERT macro does the right thing -- it's do { } while(0) IIRC
Landed in http://trac.webkit.org/changeset/99009