Bug 143422

Summary: Return Optional<uint32_t> from PropertyName::asIndex
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: JavaScriptCoreAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, darin, fpizlo, ggaren
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 140426    
Attachments:
Description Flags
Patch
none
Patch darin: review+

Yusuke Suzuki
Reported 2015-04-05 13:10:00 PDT
Spawn from https://bugs.webkit.org/show_bug.cgi?id=140426. To re-land https://bugs.webkit.org/show_bug.cgi?id=140426, we first extract simple refactoring patch from the target issue. In this patch, change return value of asIndex from uint32_t to Optional<uint32_t>.
Attachments
Patch (43.36 KB, patch)
2015-04-05 13:15 PDT, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (43.22 KB, patch)
2015-04-05 13:24 PDT, Yusuke Suzuki 		darin: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Yusuke Suzuki
Comment 1 2015-04-05 13:15:32 PDT
Created attachment 250163 [details] Patch
Darin Adler
Comment 2 2015-04-05 13:17:00 PDT
Comment on attachment 250163 [details] Patch Seems cleaner than using a magic value, but there is some performance risk. Did you run the performance tests?
Yusuke Suzuki
Comment 3 2015-04-05 13:22:15 PDT
(In reply to comment #2) > Comment on attachment 250163 [details] > Patch > > Seems cleaner than using a magic value, but there is some performance risk. > Did you run the performance tests? OK! I'll do performance tests.
Yusuke Suzuki
Comment 4 2015-04-05 13:24:34 PDT
Created attachment 250164 [details] Patch
Yusuke Suzuki
Comment 5 2015-04-05 13:25:02 PDT
I've updated the patch to pass the binding tests.
Yusuke Suzuki
Comment 6 2015-04-05 14:10:04 PDT
Took the run-jsc-benchmarks result. Benchmark report for SunSpider, LongSpider, V8Spider, Octane, and JSRegress on yusuke (MacBookPro8,2). VMs tested: "Baseline" at /Users/yusuke/dev/WebKit/WebKitBuild/Release/jsc "Mine" at /Users/yusuke/dev/WebKit/WebKitBuild/index/Release/jsc Collected 4 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. Baseline Mine SunSpider: 3d-cube 5.4728+-0.8000 5.4165+-0.4810 might be 1.0104x faster 3d-morph 5.9995+-0.0589 ? 6.0229+-0.1378 ? 3d-raytrace 7.4490+-0.8991 6.8873+-0.2474 might be 1.0816x faster access-binary-trees 2.4670+-0.0556 ? 2.4680+-0.1416 ? access-fannkuch 6.2453+-0.1753 ? 6.4904+-0.9798 ? might be 1.0392x slower access-nbody 3.7012+-2.1074 3.2291+-0.6342 might be 1.1462x faster access-nsieve 3.5394+-0.1589 ? 3.5939+-0.2110 ? might be 1.0154x slower bitops-3bit-bits-in-byte 1.9302+-0.0859 ? 1.9472+-0.0797 ? bitops-bits-in-byte 3.5703+-0.0539 ? 3.5969+-0.0792 ? bitops-bitwise-and 2.3936+-0.1824 2.3665+-0.2073 might be 1.0114x faster bitops-nsieve-bits 4.1790+-0.0881 4.0652+-0.1343 might be 1.0280x faster controlflow-recursive 2.4288+-0.0964 2.3182+-0.0486 might be 1.0477x faster crypto-aes 4.5078+-0.1778 ? 4.6695+-0.1497 ? might be 1.0359x slower crypto-md5 2.7092+-0.0480 2.6791+-0.0847 might be 1.0112x faster crypto-sha1 2.8290+-0.2371 2.8149+-0.2483 date-format-tofte 9.7230+-0.2114 9.6030+-0.0532 might be 1.0125x faster date-format-xparb 5.7005+-0.2249 5.5720+-0.1840 might be 1.0231x faster math-cordic 3.4178+-0.0576 3.3848+-0.0491 math-partial-sums 6.0042+-1.1125 5.8710+-1.0158 might be 1.0227x faster math-spectral-norm 2.3990+-0.0434 ? 2.4067+-0.1073 ? regexp-dna 8.9366+-4.4004 7.4221+-0.0368 might be 1.2041x faster string-base64 5.8749+-2.8253 ? 6.1243+-2.2366 ? might be 1.0425x slower string-fasta 7.2150+-0.2122 7.2086+-0.1902 string-tagcloud 10.4259+-0.2290 ? 10.5206+-0.3712 ? string-unpack-code 22.4555+-3.5757 ? 26.9509+-5.7439 ? might be 1.2002x slower string-validate-input 5.2610+-0.1823 ? 5.3085+-0.0764 ? <arithmetic> 5.6475+-0.3005 ? 5.7284+-0.1260 ? might be 1.0143x slower Baseline Mine LongSpider: 3d-cube 963.8527+-37.0709 ? 982.1213+-16.0707 ? might be 1.0190x slower 3d-morph 1590.6920+-7.0415 ? 1599.4299+-13.7053 ? 3d-raytrace 825.1250+-5.9263 ? 828.6057+-12.3930 ? access-binary-trees 1143.8883+-19.2819 ? 1146.9945+-18.6927 ? access-fannkuch 344.4578+-14.9871 ? 349.3720+-3.3791 ? might be 1.0143x slower access-nbody 668.8215+-10.6692 ? 670.9675+-7.8058 ? access-nsieve 954.6959+-32.0667 ? 962.6248+-24.1956 ? bitops-3bit-bits-in-byte 51.8857+-0.5068 51.8080+-0.2039 bitops-bits-in-byte 98.5856+-3.9911 ? 100.6520+-8.8912 ? might be 1.0210x slower bitops-nsieve-bits 816.9312+-13.5383 813.5010+-17.2267 controlflow-recursive 532.2280+-10.9280 524.4557+-5.1789 might be 1.0148x faster crypto-aes 770.8790+-21.0214 ? 784.4233+-12.3460 ? might be 1.0176x slower crypto-md5 632.2375+-20.4835 ? 645.8077+-20.2416 ? might be 1.0215x slower crypto-sha1 745.1247+-17.4889 743.0477+-21.1577 date-format-tofte 814.8959+-20.0143 799.8657+-30.3600 might be 1.0188x faster date-format-xparb 809.2614+-35.1035 805.5229+-52.8847 math-cordic 656.7354+-2.6095 ? 659.4233+-4.2510 ? math-partial-sums 588.7678+-0.7551 ? 589.5973+-6.7478 ? math-spectral-norm 912.9001+-23.5951 908.9229+-12.1131 string-base64 427.0251+-20.1482 422.0098+-10.0096 might be 1.0119x faster string-fasta 489.6420+-9.4650 ? 492.7524+-10.2720 ? string-tagcloud 240.0890+-6.5187 238.2360+-11.0486 <geometric> 560.9707+-2.7008 ? 562.4229+-1.3472 ? might be 1.0026x slower Baseline Mine V8Spider: crypto 62.6360+-1.9466 ? 62.8113+-1.4833 ? deltablue 103.7214+-6.2591 ? 109.5897+-32.4267 ? might be 1.0566x slower earley-boyer 45.8506+-1.6876 45.3932+-1.3407 might be 1.0101x faster raytrace 42.0208+-2.2243 ? 42.2401+-3.9443 ? regexp 82.6570+-31.6249 74.7061+-6.1523 might be 1.1064x faster richards 85.4338+-5.3182 ? 85.8752+-1.9087 ? splay 42.5330+-2.3785 41.3793+-4.4077 might be 1.0279x faster <geometric> 62.4185+-3.5209 61.8101+-1.4409 might be 1.0098x faster Baseline Mine Octane: encrypt 0.26024+-0.00511 0.25863+-0.00134 decrypt 4.53915+-0.01522 ? 4.62799+-0.12266 ? might be 1.0196x slower deltablue x2 0.22581+-0.00155 0.22391+-0.00670 earley 0.65923+-0.01228 ? 0.66978+-0.00849 ? might be 1.0160x slower boyer 7.35405+-0.12621 7.33505+-0.10590 navier-stokes x2 5.42004+-0.06721 5.41545+-0.03138 raytrace x2 1.45171+-0.07619 ? 1.45455+-0.06387 ? richards x2 0.12153+-0.00142 0.12076+-0.00226 splay x2 0.42045+-0.01134 ? 0.43039+-0.00912 ? might be 1.0236x slower regexp x2 35.44749+-0.54879 ? 35.87830+-0.56797 ? might be 1.0122x slower pdfjs x2 50.47327+-2.43581 49.57198+-1.02629 might be 1.0182x faster mandreel x2 59.67650+-0.68640 ? 59.82963+-0.79771 ? gbemu x2 47.10523+-4.57073 44.47309+-4.61054 might be 1.0592x faster closure 0.59604+-0.00607 0.59562+-0.00783 jquery 7.55609+-0.05813 7.55298+-0.12013 box2d x2 13.39275+-0.06457 13.31189+-0.10888 zlib x2 419.00498+-28.66742 ? 431.68833+-8.43290 ? might be 1.0303x slower typescript x2 889.50519+-37.60563 881.11877+-19.18283 <geometric> 7.42702+-0.10647 7.41530+-0.03014 might be 1.0016x faster Baseline Mine JSRegress: abs-boolean 3.1712+-0.1103 3.1113+-0.1917 might be 1.0193x faster adapt-to-double-divide 17.2542+-0.3004 17.2184+-0.2685 aliased-arguments-getbyval 1.3577+-0.3699 1.2651+-0.0359 might be 1.0732x faster allocate-big-object 2.7606+-0.1186 ? 2.7935+-0.0625 ? might be 1.0119x slower arguments-named-and-reflective 12.8412+-0.2642 ? 14.1688+-4.4550 ? might be 1.1034x slower arguments-out-of-bounds 15.8792+-0.7138 ? 16.2370+-1.9135 ? might be 1.0225x slower arguments-strict-mode 11.8253+-0.4943 ? 12.7440+-2.4614 ? might be 1.0777x slower arguments 11.2299+-2.8388 ? 13.3657+-4.8875 ? might be 1.1902x slower arity-mismatch-inlining 0.9833+-0.0349 ? 1.0437+-0.2602 ? might be 1.0615x slower array-access-polymorphic-structure 8.6103+-4.0435 8.3442+-3.6547 might be 1.0319x faster array-nonarray-polymorhpic-access 41.1108+-2.5298 ? 41.5039+-0.5055 ? array-prototype-every 105.9562+-6.5638 102.4488+-6.1615 might be 1.0342x faster array-prototype-forEach 95.7275+-3.6263 ? 99.3962+-3.5015 ? might be 1.0383x slower array-prototype-map 111.6138+-9.5198 ? 113.7448+-9.2987 ? might be 1.0191x slower array-prototype-some 101.5960+-2.8227 ? 105.9019+-2.4381 ? might be 1.0424x slower array-splice-contiguous 44.0683+-0.3376 ! 50.8796+-4.9708 ! definitely 1.1546x slower array-with-double-add 4.4845+-0.2154 4.4835+-0.1250 array-with-double-increment 3.7843+-0.1433 3.7607+-0.1507 array-with-double-mul-add 5.1589+-0.1502 ? 5.1995+-0.1775 ? array-with-double-sum 3.4427+-0.1192 ? 3.4989+-0.1296 ? might be 1.0163x slower array-with-int32-add-sub 8.0975+-1.3772 7.6633+-0.0363 might be 1.0567x faster array-with-int32-or-double-sum 3.5690+-0.1629 3.4716+-0.1304 might be 1.0281x faster ArrayBuffer-DataView-alloc-large-long-lived 44.4770+-4.1496 ? 45.1591+-3.8765 ? might be 1.0153x slower ArrayBuffer-DataView-alloc-long-lived 18.5956+-5.2774 ? 18.9675+-5.6732 ? might be 1.0200x slower ArrayBuffer-Int32Array-byteOffset 4.0450+-0.1870 ? 4.0840+-0.1466 ? ArrayBuffer-Int8Array-alloc-large-long-lived 45.5698+-9.1380 41.2944+-5.6286 might be 1.1035x faster ArrayBuffer-Int8Array-alloc-long-lived-buffer 28.6320+-3.4945 26.9313+-4.5806 might be 1.0631x faster ArrayBuffer-Int8Array-alloc-long-lived 18.8895+-5.2728 15.5963+-0.8311 might be 1.2112x faster ArrayBuffer-Int8Array-alloc 15.6907+-4.6696 14.3686+-4.6200 might be 1.0920x faster asmjs_bool_bug 7.9344+-0.3574 ? 9.1515+-4.3391 ? might be 1.1534x slower assign-custom-setter-polymorphic 3.1378+-0.1019 ? 3.3991+-0.2125 ? might be 1.0833x slower assign-custom-setter 4.7515+-1.7696 ? 5.1623+-1.8482 ? might be 1.0865x slower basic-set 9.4924+-0.5221 9.3685+-0.2768 might be 1.0132x faster big-int-mul 4.4364+-0.1603 ? 4.5220+-0.0757 ? might be 1.0193x slower boolean-test 3.3756+-0.1041 ? 3.4240+-0.1071 ? might be 1.0143x slower branch-fold 4.2162+-0.0816 ? 4.2191+-0.1399 ? by-val-generic 8.7337+-0.4612 ? 8.8880+-0.6735 ? might be 1.0177x slower call-spread-apply 38.1036+-7.3061 36.6840+-2.3367 might be 1.0387x faster call-spread-call 30.9249+-4.8378 26.5129+-0.4207 might be 1.1664x faster captured-assignments 0.4927+-0.0302 0.4842+-0.0166 might be 1.0175x faster cast-int-to-double 5.6306+-0.0442 5.5779+-0.0088 cell-argument 9.1119+-1.3226 8.6714+-0.3333 might be 1.0508x faster cfg-simplify 3.1141+-0.0221 ? 3.2049+-0.1134 ? might be 1.0292x slower chain-getter-access 11.2617+-0.1997 11.1060+-0.1881 might be 1.0140x faster cmpeq-obj-to-obj-other 11.1790+-1.6089 11.0079+-1.0597 might be 1.0155x faster constant-test 5.2217+-0.0431 ? 5.2531+-0.0682 ? DataView-custom-properties 48.0289+-7.9462 ? 48.5387+-8.0144 ? might be 1.0106x slower deconstructing-parameters-overridden-by-function 0.5673+-0.0838 0.5235+-0.0419 might be 1.0836x faster delay-tear-off-arguments-strictmode 15.8000+-2.2125 15.2386+-1.2955 might be 1.0368x faster deltablue-varargs 217.0242+-7.4807 ? 217.2670+-2.3494 ? destructuring-arguments 19.1172+-2.8099 18.6813+-2.5434 might be 1.0233x faster destructuring-swap 5.3843+-0.0411 5.3520+-0.0838 direct-arguments-getbyval 1.2620+-0.0492 1.2408+-0.0355 might be 1.0171x faster div-boolean-double 5.4968+-0.0303 5.4675+-0.0340 div-boolean 8.1074+-0.1328 8.0421+-0.0796 double-get-by-val-out-of-bounds 4.8849+-0.0809 4.8160+-0.0420 might be 1.0143x faster double-pollution-getbyval 9.0854+-0.1473 ? 9.1086+-0.2933 ? double-pollution-putbyoffset 4.6353+-0.0359 ? 4.7155+-0.1055 ? might be 1.0173x slower double-to-int32-typed-array-no-inline 2.4175+-0.0497 2.4010+-0.0167 double-to-int32-typed-array 2.1360+-0.1523 2.1110+-0.0516 might be 1.0119x faster double-to-uint32-typed-array-no-inline 2.4990+-0.0605 2.4684+-0.0269 might be 1.0124x faster double-to-uint32-typed-array 2.1728+-0.0185 ? 2.1794+-0.0300 ? elidable-new-object-dag 50.8287+-5.6290 ? 52.4270+-1.5262 ? might be 1.0314x slower elidable-new-object-roflcopter 55.6030+-8.8764 55.2797+-4.6483 elidable-new-object-then-call 45.5613+-3.9546 ? 47.1598+-6.0367 ? might be 1.0351x slower elidable-new-object-tree 53.7068+-6.9735 53.3727+-1.7876 empty-string-plus-int 6.3603+-0.2021 6.3335+-0.0821 emscripten-cube2hash 41.1296+-1.0144 ? 43.6127+-5.8801 ? might be 1.0604x slower exit-length-on-plain-object 18.0699+-4.9244 ? 18.4573+-6.3777 ? might be 1.0214x slower external-arguments-getbyval 1.3280+-0.0959 1.3194+-0.0318 external-arguments-putbyval 2.6138+-0.0244 2.5289+-0.0980 might be 1.0335x faster fixed-typed-array-storage-var-index 1.3746+-0.0202 1.3567+-0.0306 might be 1.0132x faster fixed-typed-array-storage 0.9754+-0.0238 0.9663+-0.0622 Float32Array-matrix-mult 5.5901+-1.6120 4.7168+-0.1536 might be 1.1852x faster Float32Array-to-Float64Array-set 64.1066+-5.0109 59.9244+-0.6354 might be 1.0698x faster Float64Array-alloc-long-lived 82.0972+-5.4398 ? 84.9577+-5.7491 ? might be 1.0348x slower Float64Array-to-Int16Array-set 74.5553+-5.5475 ? 80.1657+-4.1555 ? might be 1.0753x slower fold-double-to-int 17.3231+-3.3585 16.9415+-2.2147 might be 1.0225x faster fold-get-by-id-to-multi-get-by-offset-rare-int 11.4902+-1.3321 ? 12.2435+-2.0249 ? might be 1.0656x slower fold-get-by-id-to-multi-get-by-offset 10.4667+-0.5116 9.9415+-1.6159 might be 1.0528x faster fold-multi-get-by-offset-to-get-by-offset 9.1735+-1.0589 8.1592+-2.6484 might be 1.1243x faster fold-multi-get-by-offset-to-poly-get-by-offset 8.8982+-1.6991 8.6125+-0.5614 might be 1.0332x faster fold-multi-put-by-offset-to-poly-put-by-offset 8.1171+-2.5081 7.7467+-1.4516 might be 1.0478x faster fold-multi-put-by-offset-to-put-by-offset 5.8921+-0.7891 ? 6.2697+-2.0490 ? might be 1.0641x slower fold-multi-put-by-offset-to-replace-or-transition-put-by-offset 9.7900+-0.3331 ? 10.0150+-0.5047 ? might be 1.0230x slower fold-put-by-id-to-multi-put-by-offset 9.8240+-0.6081 9.7700+-0.9224 fold-put-structure 6.3240+-1.2606 6.1720+-1.1057 might be 1.0246x faster for-of-iterate-array-entries 5.3628+-1.1827 4.9807+-0.0997 might be 1.0767x faster for-of-iterate-array-keys 4.7437+-1.9610 3.9828+-0.0865 might be 1.1910x faster for-of-iterate-array-values 4.1607+-0.1142 4.0010+-0.1786 might be 1.0399x faster fround 20.4772+-3.0756 ? 20.8727+-3.6465 ? might be 1.0193x slower ftl-library-inlining-dataview 82.6436+-5.8080 ? 86.4006+-5.0247 ? might be 1.0455x slower ftl-library-inlining 123.3502+-2.8702 ? 125.5121+-5.1245 ? might be 1.0175x slower function-dot-apply 2.4770+-0.0783 2.4688+-0.0821 function-test 3.6415+-0.1215 3.6253+-0.1249 function-with-eval 111.6100+-6.8682 108.3648+-6.4637 might be 1.0299x faster gcse-poly-get-less-obvious 20.0688+-2.7467 ? 22.3111+-6.3034 ? might be 1.1117x slower gcse-poly-get 22.7343+-4.1244 ? 22.7432+-2.3699 ? gcse 5.0237+-0.2064 4.9036+-0.0216 might be 1.0245x faster get-by-id-bimorphic-check-structure-elimination-simple 2.8990+-0.0921 2.8530+-0.0196 might be 1.0161x faster get-by-id-bimorphic-check-structure-elimination 7.0207+-0.1595 6.9662+-0.0990 get-by-id-chain-from-try-block 7.6265+-1.2428 7.5652+-1.7769 get-by-id-check-structure-elimination 5.6637+-0.1452 5.6178+-0.2028 get-by-id-proto-or-self 17.5852+-0.4055 ? 18.5015+-2.0403 ? might be 1.0521x slower get-by-id-quadmorphic-check-structure-elimination-simple 3.1572+-0.0790 ? 3.2148+-0.1795 ? might be 1.0182x slower get-by-id-self-or-proto 18.7862+-1.5805 18.4402+-1.0163 might be 1.0188x faster get-by-val-out-of-bounds 4.6794+-0.0249 4.6275+-0.0813 might be 1.0112x faster get_callee_monomorphic 4.1124+-0.2595 4.1124+-0.1372 get_callee_polymorphic 3.7286+-0.1939 ? 3.7998+-0.1970 ? might be 1.0191x slower getter-no-activation 5.8640+-0.4520 5.8246+-0.4160 getter-richards 127.3353+-10.9775 126.8831+-26.6790 getter 6.0196+-0.4123 ? 6.2220+-0.3757 ? might be 1.0336x slower global-var-const-infer-fire-from-opt 0.9305+-0.0345 ? 1.0237+-0.1296 ? might be 1.1002x slower global-var-const-infer 0.9650+-0.0534 ? 1.0363+-0.1687 ? might be 1.0739x slower HashMap-put-get-iterate-keys 30.6290+-2.9309 29.6104+-0.8106 might be 1.0344x faster HashMap-put-get-iterate 29.2693+-0.8405 ? 29.9103+-2.8432 ? might be 1.0219x slower HashMap-string-put-get-iterate 30.8719+-2.2467 30.6392+-1.3917 hoist-make-rope 12.8452+-2.5237 12.5597+-2.8502 might be 1.0227x faster hoist-poly-check-structure-effectful-loop 5.6136+-0.2778 5.5510+-0.0803 might be 1.0113x faster hoist-poly-check-structure 4.0495+-0.1805 ? 4.0541+-0.0535 ? imul-double-only 8.6915+-1.7584 8.2130+-0.8261 might be 1.0583x faster imul-int-only 10.8367+-0.5468 ? 11.0787+-1.1923 ? might be 1.0223x slower imul-mixed 8.6910+-0.6408 8.1935+-0.3736 might be 1.0607x faster in-four-cases 19.6133+-0.4348 ? 19.8384+-0.3423 ? might be 1.0115x slower in-one-case-false 10.8155+-0.4761 10.7228+-0.5551 in-one-case-true 10.6777+-0.2263 ? 10.7175+-0.1112 ? in-two-cases 11.0361+-0.1466 10.9828+-0.1107 indexed-properties-in-objects 3.1950+-0.0517 3.1182+-0.0599 might be 1.0246x faster infer-closure-const-then-mov-no-inline 4.4139+-0.8121 ? 4.4752+-0.7489 ? might be 1.0139x slower infer-closure-const-then-mov 18.3303+-0.8290 17.7854+-0.3562 might be 1.0306x faster infer-closure-const-then-put-to-scope-no-inline 14.6163+-4.2070 14.5398+-3.5945 infer-closure-const-then-put-to-scope 24.8979+-2.7023 24.4609+-1.2600 might be 1.0179x faster infer-closure-const-then-reenter-no-inline 57.2660+-4.5529 55.8890+-3.1607 might be 1.0246x faster infer-closure-const-then-reenter 139.7882+-5.0840 134.0381+-6.3494 might be 1.0429x faster infer-constant-global-property 31.6541+-1.1601 ? 33.7977+-4.8929 ? might be 1.0677x slower infer-constant-property 2.9283+-0.1218 2.9065+-0.0166 infer-one-time-closure-ten-vars 12.9520+-0.2507 ? 15.9855+-5.5922 ? might be 1.2342x slower infer-one-time-closure-two-vars 12.5769+-0.1030 ? 12.7465+-0.9491 ? might be 1.0135x slower infer-one-time-closure 15.2996+-2.9804 13.6155+-4.4862 might be 1.1237x faster infer-one-time-deep-closure 23.9952+-3.1120 22.3605+-2.3587 might be 1.0731x faster inline-arguments-access 4.5464+-0.1188 4.5109+-0.1157 inline-arguments-aliased-access 4.5417+-0.0629 ? 4.5875+-0.1283 ? might be 1.0101x slower inline-arguments-local-escape 5.0360+-1.1509 4.7068+-0.1299 might be 1.0699x faster inline-get-scoped-var 5.6251+-0.1057 5.6070+-0.0874 inlined-put-by-id-transition 11.5928+-0.4229 11.3845+-0.5392 might be 1.0183x faster int-or-other-abs-then-get-by-val 5.6438+-0.0327 ? 5.6786+-0.0991 ? int-or-other-abs-zero-then-get-by-val 21.1117+-4.1882 18.7230+-0.2040 might be 1.1276x faster int-or-other-add-then-get-by-val 5.5515+-0.0656 ? 5.9048+-1.1659 ? might be 1.0637x slower int-or-other-add 5.8378+-0.5215 5.6243+-0.0325 might be 1.0380x faster int-or-other-div-then-get-by-val 4.7170+-0.0626 4.6545+-0.0398 might be 1.0134x faster int-or-other-max-then-get-by-val 4.7672+-0.1313 ? 4.7842+-0.0707 ? int-or-other-min-then-get-by-val 4.7142+-0.0355 ? 4.7215+-0.0446 ? int-or-other-mod-then-get-by-val 4.4327+-0.0241 ? 4.4366+-0.0416 ? int-or-other-mul-then-get-by-val 4.3652+-0.0174 ? 4.3847+-0.0439 ? int-or-other-neg-then-get-by-val 5.1298+-0.0126 ? 5.1434+-0.0332 ? int-or-other-neg-zero-then-get-by-val 19.3161+-0.9409 ? 19.5073+-2.4829 ? int-or-other-sub-then-get-by-val 5.5523+-0.0113 ? 5.5609+-0.0291 ? int-or-other-sub 4.1025+-0.0170 ? 4.1075+-0.0538 ? int-overflow-local 5.1008+-0.0186 5.0942+-0.0228 Int16Array-alloc-long-lived 56.8171+-2.4705 ? 61.2926+-8.1071 ? might be 1.0788x slower Int16Array-bubble-sort-with-byteLength 23.4077+-0.8177 ? 24.3641+-1.5292 ? might be 1.0409x slower Int16Array-bubble-sort 23.4490+-0.4587 ? 23.7620+-1.5722 ? might be 1.0134x slower Int16Array-load-int-mul 1.7291+-0.0730 1.7038+-0.0124 might be 1.0148x faster Int16Array-to-Int32Array-set 63.7480+-5.0235 ? 68.3676+-4.7267 ? might be 1.0725x slower Int32Array-alloc-large 30.2376+-3.0001 ? 30.4352+-2.0257 ? Int32Array-alloc-long-lived 63.6934+-2.3283 ? 65.9332+-3.3589 ? might be 1.0352x slower Int32Array-alloc 4.2391+-1.9113 3.6918+-0.2109 might be 1.1482x faster Int32Array-Int8Array-view-alloc 10.0759+-4.4126 ? 10.3337+-4.3582 ? might be 1.0256x slower int52-spill 8.1835+-2.7373 7.2600+-0.1946 might be 1.1272x faster Int8Array-alloc-long-lived 54.5251+-7.9219 ? 57.5613+-3.1825 ? might be 1.0557x slower Int8Array-load-with-byteLength 4.0150+-0.1256 3.9263+-0.0955 might be 1.0226x faster Int8Array-load 3.9050+-0.0178 ? 3.9870+-0.1993 ? might be 1.0210x slower integer-divide 13.0622+-0.4489 12.8813+-0.3228 might be 1.0140x faster integer-modulo 2.2628+-0.0669 ? 2.3257+-0.0449 ? might be 1.0278x slower large-int-captured 6.0165+-0.1364 5.9580+-0.1089 large-int-neg 17.5143+-0.2212 ? 19.8483+-4.0416 ? might be 1.1333x slower large-int 16.1103+-0.4867 ? 16.2080+-0.9052 ? logical-not 5.0500+-0.0268 ? 5.0788+-0.0636 ? lots-of-fields 14.1647+-0.9367 ? 15.1995+-4.3364 ? might be 1.0730x slower make-indexed-storage 3.4475+-0.0654 3.2689+-0.4221 might be 1.0546x faster make-rope-cse 4.8903+-0.1094 ? 6.1658+-2.5313 ? might be 1.2608x slower marsaglia-larger-ints 46.7322+-3.0498 ? 47.2032+-1.3434 ? might be 1.0101x slower marsaglia-osr-entry 25.0485+-0.4717 24.9874+-0.5627 max-boolean 2.9488+-0.0098 ? 2.9703+-0.1102 ? method-on-number 20.6281+-4.4328 19.4338+-0.8502 might be 1.0615x faster min-boolean 2.9882+-0.2777 2.9266+-0.1021 might be 1.0211x faster minus-boolean-double 3.2296+-0.0055 ? 3.3598+-0.3791 ? might be 1.0403x slower minus-boolean 2.8410+-0.1095 2.7985+-0.0660 might be 1.0152x faster misc-strict-eq 40.0060+-0.3973 ? 40.6725+-5.4097 ? might be 1.0167x slower mod-boolean-double 11.2086+-0.2232 ? 11.7517+-1.6330 ? might be 1.0485x slower mod-boolean 8.1829+-0.0462 8.1542+-0.0407 mul-boolean-double 3.8376+-0.1382 ? 3.9221+-0.1262 ? might be 1.0220x slower mul-boolean 2.9673+-0.0622 2.9462+-0.0118 neg-boolean 3.2937+-0.1022 3.2296+-0.0064 might be 1.0198x faster negative-zero-divide 0.3748+-0.0318 ? 0.3782+-0.0215 ? negative-zero-modulo 0.3787+-0.0199 ? 0.3887+-0.0110 ? might be 1.0266x slower negative-zero-negate 0.3629+-0.0078 ? 0.3682+-0.0655 ? might be 1.0148x slower nested-function-parsing 44.4224+-4.9731 43.8958+-4.3827 might be 1.0120x faster new-array-buffer-dead 3.4371+-0.1608 3.3645+-0.1058 might be 1.0216x faster new-array-buffer-push 7.1195+-0.2138 ? 7.4070+-0.6523 ? might be 1.0404x slower new-array-dead 12.5815+-0.4436 ? 14.1165+-2.7435 ? might be 1.1220x slower new-array-push 4.5353+-0.1844 4.4780+-0.0911 might be 1.0128x faster number-test 3.2955+-0.0485 3.2863+-0.0879 object-closure-call 7.1210+-0.3304 6.9329+-0.0535 might be 1.0271x faster object-test 3.4583+-0.1338 ? 3.5635+-0.0597 ? might be 1.0304x slower obvious-sink-pathology-taken 169.6821+-8.4627 167.7992+-4.4834 might be 1.0112x faster obvious-sink-pathology 162.6763+-7.0008 159.3877+-2.7538 might be 1.0206x faster obviously-elidable-new-object 43.4943+-3.2827 42.8768+-7.4929 might be 1.0144x faster plus-boolean-arith 2.8651+-0.0673 ? 2.9943+-0.3252 ? might be 1.0451x slower plus-boolean-double 3.3726+-0.3714 3.2658+-0.1296 might be 1.0327x faster plus-boolean 2.9716+-0.1389 ? 2.9747+-0.0507 ? poly-chain-access-different-prototypes-simple 3.3776+-0.0192 ? 3.4437+-0.1375 ? might be 1.0196x slower poly-chain-access-different-prototypes 2.9786+-0.1681 ? 2.9811+-0.1032 ? poly-chain-access-simpler 3.4057+-0.1101 ? 3.4156+-0.1338 ? poly-chain-access 2.9135+-0.1154 2.9015+-0.0687 poly-stricteq 64.3961+-7.3072 62.4650+-4.5538 might be 1.0309x faster polymorphic-array-call 1.5882+-0.0252 1.4771+-0.1860 might be 1.0752x faster polymorphic-get-by-id 3.3538+-0.1451 3.3067+-0.0439 might be 1.0142x faster polymorphic-put-by-id 31.9478+-2.2442 ? 34.5029+-2.4863 ? might be 1.0800x slower polymorphic-structure 18.7892+-2.6915 ? 19.4900+-3.9978 ? might be 1.0373x slower polyvariant-monomorphic-get-by-id 9.4407+-0.1959 9.4343+-0.1861 proto-getter-access 11.2573+-0.3172 ? 11.4344+-0.3244 ? might be 1.0157x slower put-by-id-replace-and-transition 9.1740+-0.3741 ? 10.1770+-3.0792 ? might be 1.1093x slower put-by-id-slightly-polymorphic 3.1252+-0.1680 3.0940+-0.1040 might be 1.0101x faster put-by-id 15.1715+-0.7557 15.0740+-1.2777 put-by-val-direct 0.6643+-0.0061 ? 0.6725+-0.0081 ? might be 1.0124x slower put-by-val-large-index-blank-indexing-type 6.6175+-0.5142 ? 7.4323+-3.1349 ? might be 1.1231x slower put-by-val-machine-int 2.8854+-0.1205 ? 2.9323+-0.0579 ? might be 1.0163x slower rare-osr-exit-on-local 16.1773+-0.3255 16.1674+-0.1562 register-pressure-from-osr 23.5970+-1.8443 ? 24.5271+-4.5908 ? might be 1.0394x slower setter 5.9088+-0.3518 ? 6.0733+-0.7673 ? might be 1.0278x slower simple-activation-demo 26.3040+-1.3860 25.8064+-0.0978 might be 1.0193x faster simple-getter-access 14.4667+-0.8893 14.3289+-0.4737 simple-poly-call-nested 8.7952+-0.6204 8.4534+-0.1796 might be 1.0404x faster simple-poly-call 1.4139+-0.0601 ? 1.4622+-0.0599 ? might be 1.0341x slower sin-boolean 21.2473+-2.6042 ? 21.6674+-2.4178 ? might be 1.0198x slower sinkable-new-object-dag 78.9600+-2.0671 78.4512+-3.5311 sinkable-new-object-taken 63.3803+-4.9863 57.2175+-2.7497 might be 1.1077x faster sinkable-new-object 47.0610+-3.4063 42.5782+-3.2692 might be 1.1053x faster slow-array-profile-convergence 2.9746+-0.0496 ? 3.0407+-0.1422 ? might be 1.0222x slower slow-convergence 3.5681+-0.1377 3.4756+-0.0740 might be 1.0266x faster sparse-conditional 1.3282+-0.0310 ? 1.3317+-0.0392 ? splice-to-remove 20.5542+-4.9875 ? 21.2714+-8.8804 ? might be 1.0349x slower string-char-code-at 16.5280+-0.3575 ? 16.6666+-0.3801 ? string-concat-object 2.5803+-0.0999 2.5371+-0.1079 might be 1.0170x faster string-concat-pair-object 2.7908+-0.6171 2.6042+-0.2954 might be 1.0716x faster string-concat-pair-simple 13.4805+-1.5290 ? 14.1942+-2.5381 ? might be 1.0529x slower string-concat-simple 14.4798+-1.0644 ? 15.1478+-3.4142 ? might be 1.0461x slower string-cons-repeat 8.9631+-1.6622 ? 10.2755+-3.7308 ? might be 1.1464x slower string-cons-tower 9.7698+-3.7095 9.2383+-1.7193 might be 1.0575x faster string-equality 18.4515+-0.0893 ? 18.6645+-0.8454 ? might be 1.0115x slower string-get-by-val-big-char 8.5444+-0.0984 ? 8.5944+-0.2313 ? string-get-by-val-out-of-bounds-insane 5.4808+-3.1682 4.6411+-0.1854 might be 1.1809x faster string-get-by-val-out-of-bounds 5.6083+-0.1327 ? 5.6335+-0.0352 ? string-get-by-val 3.7397+-0.1721 ? 4.3572+-1.2785 ? might be 1.1651x slower string-hash 2.3397+-0.0840 2.2964+-0.0522 might be 1.0188x faster string-long-ident-equality 16.4313+-3.5977 15.7568+-1.1338 might be 1.0428x faster string-out-of-bounds 18.0363+-3.9297 15.9670+-1.4067 might be 1.1296x faster string-repeat-arith 36.2529+-5.4306 ? 43.3535+-16.1515 ? might be 1.1959x slower string-sub 76.0170+-4.8000 ? 82.1083+-4.6928 ? might be 1.0801x slower string-test 3.2053+-0.0881 ^ 3.1030+-0.0107 ^ definitely 1.0330x faster string-var-equality 33.6245+-3.5411 31.9890+-0.5994 might be 1.0511x faster structure-hoist-over-transitions 2.8537+-0.0593 ? 2.8698+-0.0807 ? substring-concat-weird 45.7450+-4.3758 ? 46.0608+-4.5142 ? substring-concat 45.9540+-1.7221 45.5685+-0.7512 substring 56.1453+-3.5342 51.7229+-1.3717 might be 1.0855x faster switch-char-constant 2.9357+-0.0531 ? 2.9505+-0.0207 ? switch-char 6.9000+-0.7492 ? 7.2941+-0.8205 ? might be 1.0571x slower switch-constant 9.2142+-0.9055 8.8732+-0.1422 might be 1.0384x faster switch-string-basic-big-var 18.5490+-4.4374 16.1915+-0.2627 might be 1.1456x faster switch-string-basic-big 14.3890+-0.1840 ? 16.1319+-4.1024 ? might be 1.1211x slower switch-string-basic-var 16.5894+-3.5638 ? 18.0544+-4.8912 ? might be 1.0883x slower switch-string-basic 14.7998+-3.2656 ? 14.8857+-3.0979 ? switch-string-big-length-tower-var 21.1658+-0.2548 ? 23.5738+-4.7339 ? might be 1.1138x slower switch-string-length-tower-var 20.4707+-2.9906 17.0646+-0.6731 might be 1.1996x faster switch-string-length-tower 13.3731+-0.3007 ? 16.0759+-4.8973 ? might be 1.2021x slower switch-string-short 14.6556+-3.6908 ? 14.9437+-4.4229 ? might be 1.0197x slower switch 13.3380+-0.2929 13.0869+-0.2710 might be 1.0192x faster tear-off-arguments-simple 3.6235+-0.1402 3.5602+-0.0726 might be 1.0178x faster tear-off-arguments 4.9727+-0.0641 4.9572+-0.0548 temporal-structure 14.8188+-0.5051 14.4247+-0.3635 might be 1.0273x faster to-int32-boolean 16.8131+-1.3641 16.2502+-0.6508 might be 1.0346x faster try-catch-get-by-val-cloned-arguments 15.4097+-0.2784 ? 16.4926+-3.9793 ? might be 1.0703x slower try-catch-get-by-val-direct-arguments 7.4035+-1.6140 6.9489+-1.2571 might be 1.0654x faster try-catch-get-by-val-scoped-arguments 10.3892+-3.9857 7.8046+-0.2262 might be 1.3312x faster undefined-property-access 410.9087+-5.3649 ? 412.9125+-3.6969 ? undefined-test 3.3340+-0.0359 3.2776+-0.0610 might be 1.0172x faster unprofiled-licm 23.0465+-0.3783 ? 24.1953+-2.4915 ? might be 1.0498x slower varargs-call 16.1489+-1.2250 16.0524+-0.9205 varargs-construct-inline 25.0800+-1.8999 ? 26.6193+-3.3892 ? might be 1.0614x slower varargs-construct 34.3962+-0.2265 ? 34.7411+-1.1695 ? might be 1.0100x slower varargs-inline 9.2407+-0.2700 ? 9.2935+-0.1866 ? varargs-strict-mode 10.1461+-0.1321 ? 10.2562+-0.3583 ? might be 1.0109x slower varargs 10.5522+-1.2319 10.1722+-0.2603 might be 1.0374x faster weird-inlining-const-prop 2.5197+-0.1656 2.4066+-0.1615 might be 1.0470x faster <geometric> 9.4587+-0.0988 9.4576+-0.0859 might be 1.0001x faster Baseline Mine Baseline Mine Geomean of preferred means: <scaled-result> 42.5097+-0.7088 ? 42.5600+-0.3028 ? might be 1.0012x slower
Darin Adler
Comment 7 2015-04-05 19:57:14 PDT
Comment on attachment 250164 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=250164&action=review I am still slightly worried about that 0.1% performance cost. I don’t find this uniformly more readable than the old version. Saying if (asIndex(x)) seems a bit strange unless you are aware that the return value is an “optional index”. I think I would call the function that converts a string to an index something like parseIndex rather than asIndex or toIndex. > Source/JavaScriptCore/runtime/Identifier.h:54 > + if (value > 0xFFFFFFFFU / 10) Since indices don’t include the maximum value, this could be >= or 0xFFFFFFFEU instead, but I guess it’s OK the way it is. > Source/JavaScriptCore/runtime/Identifier.h:70 > + if (value == UINT_MAX) Since we go to the trouble to use uint32_t and not unsigned above, and use 0xFFFFFFFFU in one place too, it’s really strange to use UINT_MAX here. I would have used "unsigned/UINT_MAX/UINT_MAX" or "uint32_t/0xFFFFFFFFU/0xFFFFFFFFU" or something else consistent, not this mix. > Source/JavaScriptCore/runtime/Identifier.h:75 > +ALWAYS_INLINE Optional<uint32_t> toIndexFromStringImpl(const StringImpl* impl) This should take a StringImpl& rather than a StringImpl*. Also no real point in saying const StringImpl* since StringImpl is immutable. > Source/JavaScriptCore/runtime/Identifier.h:156 > + static const uint32_t NotAnIndex = UINT_MAX; Do we want to remove this? > Source/JavaScriptCore/runtime/Identifier.h:161 > + Optional<uint32_t> asIndex() const > + { > + return !isSymbol() ? toIndexFromStringImpl(m_string.impl()) : Nullopt; > + } Will this work on the null identifier?
Yusuke Suzuki
Comment 8 2015-04-06 03:53:37 PDT
Comment on attachment 250164 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=250164&action=review Thank you for your review. OK, so I just change `asIndex` to `parseIndex`. >> Source/JavaScriptCore/runtime/Identifier.h:70 >> + if (value == UINT_MAX) > > Since we go to the trouble to use uint32_t and not unsigned above, and use 0xFFFFFFFFU in one place too, it’s really strange to use UINT_MAX here. I would have used "unsigned/UINT_MAX/UINT_MAX" or "uint32_t/0xFFFFFFFFU/0xFFFFFFFFU" or something else consistent, not this mix. OK, so use uint32_t/0xFFFFFFFFU/0xFFFFFFFFU. >> Source/JavaScriptCore/runtime/Identifier.h:75 >> +ALWAYS_INLINE Optional<uint32_t> toIndexFromStringImpl(const StringImpl* impl) > > This should take a StringImpl& rather than a StringImpl*. Also no real point in saying const StringImpl* since StringImpl is immutable. Ah, however, we already use `const Identifier&`. It forces to use `const StringImpl*` or `const StringImpl&` since String, which is the member of `const Identifier`, becomes const String&. At that time, I use `const StringImpl&`. >> Source/JavaScriptCore/runtime/Identifier.h:156 >> + static const uint32_t NotAnIndex = UINT_MAX; > > Do we want to remove this? It's used in JSDomWindowCustom.cpp, but it's fairly trivial. So I'll drop it. >> Source/JavaScriptCore/runtime/Identifier.h:161 >> + } > > Will this work on the null identifier? Fixed for null identifier.
Yusuke Suzuki
Comment 9 2015-04-06 04:31:50 PDT
(In reply to comment #7) > Comment on attachment 250164 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=250164&action=review > > I am still slightly worried about that 0.1% performance cost. I don’t find > this uniformly more readable than the old version. Saying if (asIndex(x)) > seems a bit strange unless you are aware that the return value is an > “optional index”. > > I think I would call the function that converts a string to an index > something like parseIndex rather than asIndex or toIndex. OK, so instead of method `asIndex`, `toIndex`, `parseIndex`, I defined new function JSC::parseIndex and used it.
Yusuke Suzuki
Comment 10 2015-04-06 04:52:15 PDT
Comment on attachment 250164 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=250164&action=review >>> Source/JavaScriptCore/runtime/Identifier.h:75 >>> +ALWAYS_INLINE Optional<uint32_t> toIndexFromStringImpl(const StringImpl* impl) >> >> This should take a StringImpl& rather than a StringImpl*. Also no real point in saying const StringImpl* since StringImpl is immutable. > > Ah, however, we already use `const Identifier&`. It forces to use `const StringImpl*` or `const StringImpl&` since String, which is the member of `const Identifier`, becomes const String&. > At that time, I use `const StringImpl&`. Ah! I've found that impl() can return AtomicStringImpl* even when `const Identifier&` case. OK, so use StringImpl& instead.
Yusuke Suzuki
Comment 11 2015-04-06 05:42:01 PDT
Committed r182406: <http://trac.webkit.org/changeset/182406>
Note You need to log in before you can comment on or make changes to this bug.