| Summary: | DFG should support continuous optimization | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Filip Pizlo <fpizlo> | ||||||||||||||||||||||||||||
| Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||||||||||||||||
| Severity: | Normal | CC: | barraclough, fpizlo, ggaren, gustavo, oliver, webkit.review.bot, xan.lopez | ||||||||||||||||||||||||||||
| Priority: | P2 | ||||||||||||||||||||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||||||||||||||||||
| Hardware: | All | ||||||||||||||||||||||||||||||
| OS: | All | ||||||||||||||||||||||||||||||
| Bug Depends on: | 68335 | ||||||||||||||||||||||||||||||
| Bug Blocks: | |||||||||||||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||||||||||||
|
Description
Filip Pizlo
2011-09-18 17:54:21 PDT
It looks like adding the ability to mark CodeBlocks in the conservative marker is mostly performance-neutral.
Benchmark report for SunSpider, V8, and Kraken.
VMs tested:
"TipOfTree" at /Volumes/Data/pizlo/quinary/OpenSource/WebKitBuild/Release/jsc
"JettisonCB" at /Volumes/Data/pizlo/octonary/OpenSource/WebKitBuild/Release/jsc
Collected 12 samples per benchmark/VM, with 4 VM invocations per benchmark. 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 JettisonCB
SunSpider:
3d-cube 7.7796+-0.1853 7.7159+-0.1270
3d-morph 7.5034+-0.1601 7.4898+-0.1230
3d-raytrace 7.6260+-0.1326 7.5335+-0.1745 might be 1.0123x faster
access-binary-trees 2.3017+-0.0509 2.2925+-0.0934
access-fannkuch 11.4303+-0.1660 ? 11.6360+-0.1596 ? might be 1.0180x slower
access-nbody 4.2085+-0.0754 ^ 3.8239+-0.0846 ^ definitely 1.1006x faster
access-nsieve 2.6087+-0.0820 2.5766+-0.0415 might be 1.0125x faster
bitops-3bit-bits-in-byte 1.6489+-0.0373 ? 1.6678+-0.0454 ? might be 1.0115x slower
bitops-bits-in-byte 2.7311+-0.0303 ? 2.7410+-0.0772 ?
bitops-bitwise-and 3.5697+-0.0873 3.5299+-0.0864 might be 1.0113x faster
bitops-nsieve-bits 5.2416+-0.0833 ? 5.2612+-0.0723 ?
controlflow-recursive 1.9781+-0.0433 1.9708+-0.0563
crypto-aes 6.8613+-0.2177 ? 6.9913+-0.2410 ? might be 1.0189x slower
crypto-md5 2.7334+-0.0518 ? 2.7985+-0.0689 ? might be 1.0238x slower
crypto-sha1 2.2083+-0.0603 2.1808+-0.0475 might be 1.0126x faster
date-format-tofte 9.9839+-0.1474 ? 10.1863+-0.1228 ? might be 1.0203x slower
date-format-xparb 9.0108+-0.3826 8.7002+-0.1493 might be 1.0357x faster
math-cordic 6.1725+-0.1297 ? 6.2024+-0.1388 ?
math-partial-sums 7.3134+-0.1048 7.2668+-0.1245
math-spectral-norm 2.5721+-0.0469 ? 2.6382+-0.0664 ? might be 1.0257x slower
regexp-dna 10.8081+-0.2950 ? 10.8432+-0.1563 ?
string-base64 5.6656+-0.1398 ? 5.7008+-0.1246 ?
string-fasta 6.9114+-0.1404 ? 6.9835+-0.1214 ? might be 1.0104x slower
string-tagcloud 11.8874+-0.1724 11.8395+-0.1860
string-unpack-code 18.6655+-0.1657 ? 18.7603+-0.4900 ?
string-validate-input 6.5207+-0.1124 6.4334+-0.1503 might be 1.0136x faster
<arithmetic> 6.3824+-0.0342 6.3755+-0.0252
<geometric> 5.2612+-0.0271 5.2475+-0.0176
<harmonic> 4.2911+-0.0266 4.2802+-0.0274
TipOfTree JettisonCB
V8:
crypto 83.2440+-0.3707 ^ 82.3064+-0.5042 ^ definitely 1.0114x faster
deltablue 241.8391+-2.0837 ? 243.4727+-1.5806 ?
earley-boyer 96.1417+-0.2934 95.7106+-0.5460
raytrace 69.7033+-1.0869 ? 69.7215+-0.7712 ?
regexp 106.1064+-0.3785 ? 106.3670+-0.3294 ?
richards 216.8097+-0.5041 216.5710+-0.7715
splay 98.9097+-0.5848 ? 99.5432+-0.5767 ?
<arithmetic> 130.3934+-0.4094 ? 130.5275+-0.3473 ?
<geometric> 117.3718+-0.3798 117.3549+-0.3047
<harmonic> 107.6083+-0.4179 107.4964+-0.3393
TipOfTree JettisonCB
Kraken:
ai-astar 635.7456+-4.1857 630.3726+-4.3410
audio-beat-detection 463.0082+-1.4343 ! 472.5084+-2.1826 ! definitely 1.0205x slower
audio-dft 420.2750+-2.4124 420.0530+-2.2228
audio-fft 364.8925+-3.5600 ? 369.1498+-0.9085 ? might be 1.0117x slower
audio-oscillator 315.5371+-0.4917 ^ 312.9771+-0.5289 ^ definitely 1.0082x faster
imaging-darkroom 417.1771+-6.5275 ? 423.8325+-15.0247 ? might be 1.0160x slower
imaging-desaturate 217.6311+-1.1461 ? 218.2500+-0.2932 ?
imaging-gaussian-blur 592.2451+-0.8057 590.7516+-2.4122
json-parse-financial 49.6200+-0.3170 ? 50.0082+-0.3647 ?
json-stringify-tinderbox 67.7365+-0.4645 ? 68.2077+-0.3253 ?
stanford-crypto-aes 144.1274+-0.9559 ? 144.7559+-0.8604 ?
stanford-crypto-ccm 110.2297+-0.3680 ! 113.0208+-0.5793 ! definitely 1.0253x slower
stanford-crypto-pbkdf2 394.1354+-3.2859 ? 396.0956+-3.0744 ?
stanford-crypto-sha256-iterative 146.3190+-0.9362 ! 148.6070+-0.7463 ! definitely 1.0156x slower
<arithmetic> 309.9057+-0.6356 ? 311.3279+-1.1983 ?
<geometric> 241.9557+-0.3347 ! 243.5964+-0.7942 ! definitely 1.0068x slower
<harmonic> 172.8240+-0.4237 ! 174.3479+-0.5745 ! definitely 1.0088x slower
TipOfTree JettisonCB
All benchmarks:
<arithmetic> 115.2633+-0.2232 ? 115.7031+-0.4024 ?
<geometric> 26.1323+-0.0832 ? 26.1466+-0.0580 ?
<harmonic> 7.5744+-0.0460 7.5563+-0.0473
Looks like adding a speculative success counter to the code that DFG emits is *almost* performance neutral. It's a slight loss, but hopefully I can make up for it by the increased robustness.
Benchmark report for SunSpider, V8, and Kraken.
VMs tested:
"TipOfTree" at /Volumes/Data/pizlo/quinary/OpenSource/WebKitBuild/Release/jsc
"JettisonCB" at /Volumes/Data/pizlo/octonary/OpenSource/WebKitBuild/Release/jsc
Collected 12 samples per benchmark/VM, with 4 VM invocations per benchmark. 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 JettisonCB
SunSpider:
3d-cube 7.7189+-0.1412 7.7112+-0.1051
3d-morph 7.4322+-0.1846 ? 7.4622+-0.1422 ?
3d-raytrace 7.6861+-0.0888 7.5094+-0.0946 might be 1.0235x faster
access-binary-trees 2.3022+-0.0572 ? 2.3973+-0.0883 ? might be 1.0413x slower
access-fannkuch 11.5342+-0.1283 ? 11.5562+-0.1893 ?
access-nbody 4.1764+-0.1071 ^ 3.8803+-0.0972 ^ definitely 1.0763x faster
access-nsieve 2.5951+-0.0377 ? 2.6183+-0.0797 ?
bitops-3bit-bits-in-byte 1.6526+-0.0440 ! 1.7839+-0.0379 ! definitely 1.0794x slower
bitops-bits-in-byte 2.7086+-0.0479 2.6894+-0.0520
bitops-bitwise-and 3.6113+-0.0852 ? 3.6227+-0.0851 ?
bitops-nsieve-bits 5.3445+-0.0918 ? 5.3558+-0.1016 ?
controlflow-recursive 1.9275+-0.0509 ! 2.1499+-0.0869 ! definitely 1.1154x slower
crypto-aes 6.9565+-0.2272 6.8406+-0.3221 might be 1.0169x faster
crypto-md5 2.7340+-0.0502 ? 2.7975+-0.0356 ? might be 1.0232x slower
crypto-sha1 2.2154+-0.0499 ? 2.3144+-0.0540 ? might be 1.0447x slower
date-format-tofte 9.7958+-0.1211 ? 9.9403+-0.1487 ? might be 1.0147x slower
date-format-xparb 8.7927+-0.1389 ? 8.8095+-0.1998 ?
math-cordic 6.2006+-0.1075 6.1583+-0.0908
math-partial-sums 7.2989+-0.1076 7.2459+-0.0633
math-spectral-norm 2.6274+-0.0671 2.5728+-0.0299 might be 1.0212x faster
regexp-dna 10.9608+-0.1526 10.7005+-0.1278 might be 1.0243x faster
string-base64 5.7498+-0.1633 5.7256+-0.1321
string-fasta 6.9099+-0.1548 ? 7.0445+-0.1264 ? might be 1.0195x slower
string-tagcloud 11.5898+-0.1934 ? 11.8337+-0.1978 ? might be 1.0210x slower
string-unpack-code 18.4722+-0.1787 ? 18.8110+-0.5315 ? might be 1.0183x slower
string-validate-input 6.4533+-0.1169 ? 6.4611+-0.1352 ?
<arithmetic> 6.3633+-0.0157 ? 6.3843+-0.0220 ?
<geometric> 5.2546+-0.0131 ! 5.2953+-0.0155 ! definitely 1.0077x slower
<harmonic> 4.2874+-0.0205 ! 4.3699+-0.0179 ! definitely 1.0193x slower
TipOfTree JettisonCB
V8:
crypto 82.8718+-0.5337 82.4999+-0.4342
deltablue 241.8472+-2.3797 ! 245.6910+-1.1746 ! definitely 1.0159x slower
earley-boyer 98.3761+-2.6772 96.7450+-0.2958 might be 1.0169x faster
raytrace 69.0379+-0.2578 ! 70.1706+-0.4662 ! definitely 1.0164x slower
regexp 106.5903+-1.2884 105.7796+-0.9258
richards 217.2081+-0.8096 ! 225.6307+-1.1982 ! definitely 1.0388x slower
splay 98.6183+-0.5517 98.4457+-0.6877
<arithmetic> 130.6500+-0.3123 ! 132.1375+-0.3785 ! definitely 1.0114x slower
<geometric> 117.5667+-0.3564 ? 118.2467+-0.3255 ?
<harmonic> 107.6964+-0.4021 ? 107.9803+-0.3229 ?
TipOfTree JettisonCB
Kraken:
ai-astar 630.8458+-4.8120 628.8718+-4.5694
audio-beat-detection 465.4545+-2.0257 ! 473.6872+-0.9470 ! definitely 1.0177x slower
audio-dft 424.4798+-5.0051 418.3906+-2.0215 might be 1.0146x faster
audio-fft 362.5508+-1.6881 ? 367.1407+-2.9210 ? might be 1.0127x slower
audio-oscillator 315.8901+-0.7736 ^ 312.8390+-1.2768 ^ definitely 1.0098x faster
imaging-darkroom 413.5906+-2.0065 412.9257+-1.1696
imaging-desaturate 217.8714+-0.7518 212.2578+-5.0701 might be 1.0264x faster
imaging-gaussian-blur 591.4198+-1.2417 ! 598.7139+-2.5048 ! definitely 1.0123x slower
json-parse-financial 49.7857+-0.5397 49.6653+-0.6754
json-stringify-tinderbox 67.6122+-0.3064 67.4863+-0.2974
stanford-crypto-aes 144.6965+-1.1703 ^ 141.7946+-0.6062 ^ definitely 1.0205x faster
stanford-crypto-ccm 111.3790+-0.7516 ? 111.6174+-0.6448 ?
stanford-crypto-pbkdf2 393.2011+-2.0680 391.7593+-2.1244
stanford-crypto-sha256-iterative 147.4451+-0.6313 ? 148.8170+-0.9358 ?
<arithmetic> 309.7302+-0.4355 309.7119+-0.6984
<geometric> 242.2064+-0.3787 241.6892+-0.5748
<harmonic> 173.2442+-0.5841 172.6992+-0.6739
TipOfTree JettisonCB
All benchmarks:
<arithmetic> 115.2387+-0.1474 ? 115.4664+-0.2303 ?
<geometric> 26.1288+-0.0404 ! 26.2463+-0.0382 ! definitely 1.0045x slower
<harmonic> 7.5683+-0.0355 ! 7.7104+-0.0309 ! definitely 1.0188x slower
Created attachment 107808 [details] work in progress This is still a work in progress, and has known problems: https://bugs.webkit.org/show_bug.cgi?id=68335 Created attachment 108051 [details]
work in progress - ruggedized it some more
This still needs some tuning. I'm not yet sure if the control logic is right.
OK, this is starting to pay dividends.
Benchmark report for SunSpider, V8, and Kraken.
VMs tested:
"TipOfTree" at /Volumes/Data/pizlo/quinary/OpenSource/WebKitBuild/Release/jsc
"JettisonCB" at /Volumes/Data/pizlo/octonary/OpenSource/WebKitBuild/Release/jsc
Collected 12 samples per benchmark/VM, with 4 VM invocations per benchmark. 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 JettisonCB
SunSpider:
3d-cube 7.7479+-0.1731 ? 7.9242+-0.1771 ? might be 1.0227x slower
3d-morph 7.6610+-0.1180 ? 7.7199+-0.1663 ?
3d-raytrace 7.7748+-0.2494 7.7302+-0.2144
access-binary-trees 2.3929+-0.1053 ? 2.4276+-0.0592 ? might be 1.0145x slower
access-fannkuch 11.6314+-0.2240 11.6311+-0.2365
access-nbody 3.6431+-0.0896 3.5825+-0.0991 might be 1.0169x faster
access-nsieve 2.7376+-0.0882 2.6075+-0.0662 might be 1.0499x faster
bitops-3bit-bits-in-byte 1.7140+-0.0182 ! 1.8333+-0.0347 ! definitely 1.0696x slower
bitops-bits-in-byte 2.9296+-0.0530 2.8637+-0.0790 might be 1.0230x faster
bitops-bitwise-and 3.7218+-0.1415 3.6575+-0.1019 might be 1.0176x faster
bitops-nsieve-bits 5.2566+-0.0682 ? 5.3743+-0.1044 ? might be 1.0224x slower
controlflow-recursive 2.0677+-0.0778 ? 2.1981+-0.0528 ? might be 1.0631x slower
crypto-aes 7.0142+-0.3549 ? 7.4064+-0.3338 ? might be 1.0559x slower
crypto-md5 2.8135+-0.0794 2.7879+-0.0848
crypto-sha1 2.3213+-0.0602 ? 2.3260+-0.0753 ?
date-format-tofte 10.3116+-0.2344 ? 10.4378+-0.2828 ? might be 1.0122x slower
date-format-xparb 8.4533+-0.2499 ! 9.1867+-0.2216 ! definitely 1.0868x slower
math-cordic 6.4422+-0.1535 ^ 6.1436+-0.1015 ^ definitely 1.0486x faster
math-partial-sums 7.5496+-0.1807 7.5091+-0.1570
math-spectral-norm 2.6941+-0.0547 2.6068+-0.0639 might be 1.0335x faster
regexp-dna 10.9435+-0.1595 10.9386+-0.3098
string-base64 5.9421+-0.2571 5.7747+-0.1178 might be 1.0290x faster
string-fasta 7.2544+-0.2391 7.2107+-0.2189
string-tagcloud 12.1558+-0.3529 ? 12.4166+-0.3872 ? might be 1.0215x slower
string-unpack-code 18.8967+-0.5173 ? 19.1674+-0.5064 ? might be 1.0143x slower
string-validate-input 6.5833+-0.2060 ? 6.6301+-0.2131 ?
<arithmetic> 6.4867+-0.0486 ? 6.5420+-0.0324 ?
<geometric> 5.3690+-0.0395 ? 5.4003+-0.0259 ?
<harmonic> 4.4092+-0.0436 ? 4.4385+-0.0324 ?
TipOfTree JettisonCB
V8:
crypto 74.6046+-0.4070 ^ 72.2672+-0.7195 ^ definitely 1.0323x faster
deltablue 225.3433+-1.7379 ! 236.4885+-1.9668 ! definitely 1.0495x slower
earley-boyer 92.6612+-0.3991 ! 93.8201+-0.3120 ! definitely 1.0125x slower
raytrace 64.1926+-0.8124 ? 65.7157+-0.7643 ? might be 1.0237x slower
regexp 108.8970+-1.1601 107.2254+-0.9922 might be 1.0156x faster
richards 201.0203+-1.6227 199.9424+-1.4012
splay 99.7791+-0.9625 ? 99.9235+-1.1515 ?
<arithmetic> 123.7854+-0.3773 ! 125.0547+-0.4416 ! definitely 1.0103x slower
<geometric> 111.7982+-0.2680 ? 112.3245+-0.2848 ?
<harmonic> 102.3350+-0.3228 ? 102.5040+-0.2686 ?
TipOfTree JettisonCB
Kraken:
ai-astar 616.7712+-6.1676 ? 645.2968+-46.3494 ? might be 1.0462x slower
audio-beat-detection 473.3009+-3.6924 ? 475.1594+-1.1196 ?
audio-dft 451.4413+-8.3904 ^ 431.5748+-10.9037 ^ definitely 1.0460x faster
audio-fft 365.7535+-3.3272 ! 370.9031+-1.0182 ! definitely 1.0141x slower
audio-oscillator 397.7187+-1.8295 ^ 257.9338+-1.5908 ^ definitely 1.5419x faster
imaging-darkroom 418.8900+-1.2035 ! 427.1599+-5.2615 ! definitely 1.0197x slower
imaging-desaturate 220.7046+-1.7507 ^ 209.8197+-1.0989 ^ definitely 1.0519x faster
imaging-gaussian-blur 579.1448+-2.5212 ! 601.6267+-4.1585 ! definitely 1.0388x slower
json-parse-financial 49.3908+-0.4090 ^ 48.4404+-0.3049 ^ definitely 1.0196x faster
json-stringify-tinderbox 67.7263+-0.3530 ! 69.1170+-0.8160 ! definitely 1.0205x slower
stanford-crypto-aes 141.1874+-1.5834 138.7216+-1.0138 might be 1.0178x faster
stanford-crypto-ccm 112.6815+-1.0384 ^ 109.8747+-0.7353 ^ definitely 1.0255x faster
stanford-crypto-pbkdf2 376.7632+-2.9107 ^ 210.0397+-2.0894 ^ definitely 1.7938x faster
stanford-crypto-sha256-iterative 85.6741+-0.6838 ! 98.8062+-5.4666 ! definitely 1.1533x slower
<arithmetic> 311.2249+-0.8779 ^ 292.4624+-4.0149 ^ definitely 1.0642x faster
<geometric> 236.9719+-0.3964 ^ 222.2244+-1.5810 ^ definitely 1.0664x faster
<harmonic> 164.4145+-0.4336 ^ 159.6218+-0.9891 ^ definitely 1.0300x faster
TipOfTree JettisonCB
All benchmarks:
<arithmetic> 114.7298+-0.2931 ^ 109.3606+-1.2316 ^ definitely 1.0491x faster
<geometric> 26.0741+-0.1104 ^ 25.6799+-0.1002 ^ definitely 1.0153x faster
<harmonic> 7.7682+-0.0751 ? 7.8152+-0.0554 ?
Another set of numbers, after I retuned heuristics.
Benchmark report for SunSpider, V8, and Kraken.
VMs tested:
"TipOfTree" at /Volumes/Data/pizlo/quinary/OpenSource/WebKitBuild/Release/jsc
"JettisonCB" at /Volumes/Data/pizlo/octonary/OpenSource/WebKitBuild/Release/jsc
Collected 30 samples per benchmark/VM, with 10 VM invocations per benchmark. 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 JettisonCB
SunSpider:
3d-cube 7.8287+-0.1801 7.7857+-0.1290
3d-morph 7.4884+-0.0773 ? 7.5740+-0.0923 ? might be 1.0114x slower
3d-raytrace 7.7084+-0.1338 ? 7.7715+-0.1481 ?
access-binary-trees 2.3010+-0.0647 ? 2.3496+-0.0664 ? might be 1.0211x slower
access-fannkuch 11.7645+-0.1926 11.7477+-0.1384
access-nbody 3.5246+-0.0608 ? 3.5652+-0.0426 ? might be 1.0115x slower
access-nsieve 2.6844+-0.0391 2.6315+-0.0415 might be 1.0201x faster
bitops-3bit-bits-in-byte 1.7098+-0.0180 ! 1.8508+-0.0221 ! definitely 1.0825x slower
bitops-bits-in-byte 2.8851+-0.0596 2.8684+-0.0465
bitops-bitwise-and 3.6722+-0.0491 3.6144+-0.0580 might be 1.0160x faster
bitops-nsieve-bits 5.3017+-0.0575 5.2930+-0.0508
controlflow-recursive 2.0053+-0.0217 ! 2.2261+-0.0320 ! definitely 1.1101x slower
crypto-aes 6.8837+-0.1819 ? 7.1632+-0.1964 ? might be 1.0406x slower
crypto-md5 2.7919+-0.0505 ? 2.8382+-0.0552 ? might be 1.0166x slower
crypto-sha1 2.2869+-0.0389 ? 2.3167+-0.0327 ? might be 1.0131x slower
date-format-tofte 10.2693+-0.1475 ? 10.4425+-0.1998 ? might be 1.0169x slower
date-format-xparb 8.5748+-0.1433 ! 8.9042+-0.1078 ! definitely 1.0384x slower
math-cordic 6.2616+-0.0786 ? 6.3709+-0.2127 ? might be 1.0175x slower
math-partial-sums 7.3857+-0.0780 ? 7.4007+-0.0649 ?
math-spectral-norm 2.6995+-0.0357 2.6817+-0.0456
regexp-dna 11.0040+-0.1446 10.8350+-0.0903 might be 1.0156x faster
string-base64 5.8660+-0.1334 ? 5.8988+-0.1038 ?
string-fasta 7.1117+-0.1103 7.1099+-0.0881
string-tagcloud 12.2060+-0.2481 11.9507+-0.1893 might be 1.0214x faster
string-unpack-code 18.6964+-0.2333 ? 18.8706+-0.2691 ?
string-validate-input 6.5685+-0.1292 6.4310+-0.0923 might be 1.0214x faster
<arithmetic> 6.4415+-0.0217 ? 6.4805+-0.0219 ?
<geometric> 5.3131+-0.0157 ! 5.3704+-0.0167 ! definitely 1.0108x slower
<harmonic> 4.3488+-0.0208 ! 4.4321+-0.0200 ! definitely 1.0192x slower
TipOfTree JettisonCB
V8:
crypto 73.6284+-0.4434 ^ 71.4785+-0.3800 ^ definitely 1.0301x faster
deltablue 226.9347+-1.8829 ! 234.8437+-1.0423 ! definitely 1.0349x slower
earley-boyer 92.7009+-0.1703 ! 93.5526+-0.2832 ! definitely 1.0092x slower
raytrace 63.5931+-0.2230 ! 65.9612+-0.4625 ! definitely 1.0372x slower
regexp 107.0856+-0.3063 ^ 105.8491+-0.2453 ^ definitely 1.0117x faster
richards 199.1884+-0.7157 198.4284+-0.3962
splay 98.6054+-0.4033 ? 98.6821+-0.3222 ?
<arithmetic> 123.1052+-0.3103 ! 124.1137+-0.2069 ! definitely 1.0082x slower
<geometric> 110.9591+-0.2091 ! 111.5271+-0.1670 ! definitely 1.0051x slower
<harmonic> 101.4372+-0.1854 ! 101.8516+-0.1787 ! definitely 1.0041x slower
TipOfTree JettisonCB
Kraken:
ai-astar 617.5456+-3.9583 ! 651.3233+-28.0772 ! definitely 1.0547x slower
audio-beat-detection 472.5390+-1.3604 472.4413+-0.8757
audio-dft 421.9768+-1.8677 420.5146+-1.2780
audio-fft 362.8723+-0.7421 ! 369.4401+-0.6687 ! definitely 1.0181x slower
audio-oscillator 395.1065+-0.5801 ^ 256.0258+-0.6453 ^ definitely 1.5432x faster
imaging-darkroom 416.9501+-0.7603 ! 422.6921+-2.3323 ! definitely 1.0138x slower
imaging-desaturate 218.0053+-0.5017 ^ 209.1474+-0.3462 ^ definitely 1.0424x faster
imaging-gaussian-blur 577.9357+-0.9834 ! 594.7917+-1.6422 ! definitely 1.0292x slower
json-parse-financial 48.9424+-0.2693 48.7625+-0.2108
json-stringify-tinderbox 67.5042+-0.1889 ! 68.2908+-0.3009 ! definitely 1.0117x slower
stanford-crypto-aes 140.8445+-0.3586 ^ 137.5372+-0.4014 ^ definitely 1.0240x faster
stanford-crypto-ccm 110.5309+-0.6147 109.6368+-0.5200
stanford-crypto-pbkdf2 371.6750+-1.0003 ^ 207.7955+-0.7730 ^ definitely 1.7887x faster
stanford-crypto-sha256-iterative 85.3478+-0.2156 85.3476+-0.2550
<arithmetic> 307.6983+-0.3252 ^ 289.5533+-2.1522 ^ definitely 1.0627x faster
<geometric> 234.4093+-0.2309 ^ 218.6223+-0.7991 ^ definitely 1.0722x faster
<harmonic> 162.9500+-0.2738 ^ 156.3084+-0.3215 ^ definitely 1.0425x faster
TipOfTree JettisonCB
All benchmarks:
<arithmetic> 113.5530+-0.0900 ^ 108.3199+-0.6472 ^ definitely 1.0483x faster
<geometric> 25.8106+-0.0427 ^ 25.4498+-0.0588 ^ definitely 1.0142x faster
<harmonic> 7.6628+-0.0358 ! 7.8014+-0.0344 ! definitely 1.0181x slower
Created attachment 108101 [details]
the patch
I'll let this simmer here for a bit. I'm expecting bot failures.
Attachment 108101 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1
Source/JavaScriptCore/heap/Heap.h:103: The parameter name "codeBlock" adds no information, so it should be removed. [readability/parameter_name] [5]
Source/JavaScriptCore/bytecode/CodeBlock.cpp:2001: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5]
Source/JavaScriptCore/bytecode/CodeBlock.cpp:2002: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5]
Source/JavaScriptCore/heap/ConservativeRoots.h:41: The parameter name "codeBlocks" adds no information, so it should be removed. [readability/parameter_name] [5]
Total errors found: 4 in 19 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 108102 [details]
the patch
Fixed conflict that caused a file to get dropped from the patch. Fixed style.
Attachment 108102 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1
Source/JavaScriptCore/heap/Heap.h:103: The parameter name "codeBlock" adds no information, so it should be removed. [readability/parameter_name] [5]
Total errors found: 1 in 20 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 108103 [details]
the patch - fix more style
Comment on attachment 108103 [details] the patch - fix more style Attachment 108103 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/9767540 Created attachment 108104 [details]
the patch
Apparently the last patch did not have enough kilobytes in it. Added necessary build stuff to the various project files.
Comment on attachment 108104 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=108104&action=review > Source/JavaScriptCore/heap/JettisonedCodeBlocks.cpp:18 > +/* > + * Copyright (C) 2011 Apple Inc. All rights reserved. > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA > + * > + */ We usually use a BSD license for new code. > Source/JavaScriptCore/heap/JettisonedCodeBlocks.h:18 > +/* > + * Copyright (C) 2011 Apple Inc. All rights reserved. > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA > + * > + */ We usually use a BSD license for new code. > Source/JavaScriptCore/wtf/BitVector.h:27 > +/* > + * Copyright (C) 2010, 2011 Apple Inc. All rights reserved. > + * > + * Redistribution and use in source and binary forms, with or without > + * modification, are permitted provided that the following conditions > + * are met: > + * > + * 1. Redistributions of source code must retain the above copyright > + * notice, this list of conditions and the following disclaimer. > + * 2. Redistributions in binary form must reproduce the above copyright > + * notice, this list of conditions and the following disclaimer in the > + * documentation and/or other materials provided with the distribution. > + * 3. Neither the name of Apple Computer, Inc. ("Apple") nor the names of > + * its contributors may be used to endorse or promote products derived > + * from this software without specific prior written permission. > + * > + * THIS SOFTWARE IS PROVIDED BY APPLE AND ITS CONTRIBUTORS "AS IS" AND ANY > + * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED > + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE > + * DISCLAIMED. IN NO EVENT SHALL APPLE OR ITS CONTRIBUTORS BE LIABLE FOR ANY > + * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES > + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; > + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND > + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT > + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF > + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > + */ We usually use the 2 clause BSD license (http://www.webkit.org/coding/bsd-license.html) for new code. Created attachment 108106 [details]
the patch - fix license
Changed the licenses on the three new files to 2-clause BSD.
Comment on attachment 108106 [details]
the patch - fix license
I'm getting sick of these #if ENABLE(JIT_VERBOSE_OSR) blocks, can we please add a #define JIT_OSR_LOG(...
Comment on attachment 108106 [details] the patch - fix license View in context: https://bugs.webkit.org/attachment.cgi?id=108106&action=review Alas i won't finish reviewing this tonight -- i'm a little tired to follow the logic for keeping the jettisoned code blocks live for the right amount of time :-( It may be worth making code block refcounted though if you think that would simplify things > Source/JavaScriptCore/heap/JettisonedCodeBlocks.h:73 > + // It would be great to use an OwnPtr<CodeBlock> here but that would > + // almost certainly not work. > + HashMap<CodeBlock*, bool> m_map; Why can't we make CodeBlock RefCounted? Then just use RefPtr<CodeBlock> -- a single word for the ref count vs. the hulking mass of code block doesn't seem too expensive (In reply to comment #17) > (From update of attachment 108106 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=108106&action=review > > Alas i won't finish reviewing this tonight -- i'm a little tired to follow the logic for keeping the jettisoned code blocks live for the right amount of time :-( > > It may be worth making code block refcounted though if you think that would simplify things > > > Source/JavaScriptCore/heap/JettisonedCodeBlocks.h:73 > > + // It would be great to use an OwnPtr<CodeBlock> here but that would > > + // almost certainly not work. > > + HashMap<CodeBlock*, bool> m_map; > > Why can't we make CodeBlock RefCounted? Then just use RefPtr<CodeBlock> -- a single word for the ref count vs. the hulking mass of code block doesn't seem too expensive There is a benefit to the way it works now: I know when a CodeBlock gets destroyed by looking at the code. Comment on attachment 108106 [details] the patch - fix license View in context: https://bugs.webkit.org/attachment.cgi?id=108106&action=review Some good stuff here, but the ConservativeRoots thing needs reworking, and some refactoring would improve clarity. > Source/JavaScriptCore/heap/ConservativeRoots.cpp:72 > + m_codeBlocks->mark(p); Doing a full pointer-hashed lookup on every pointer defeats a lot of prior optimization work on conservative root gathering, which was a win at the time. It also kind of confuses the role of the ConservativeRoots class, since the ConservativeRoots class is used to gather roots form anywhere -- currently, the C stack or the JS stack -- but you're only interested in CodeBlock entries in the JS stack (and then, only in call frames). You should rework this. One option is just to wait and blow away all jettisoned CodeBlocks unconditionally when exiting the VM. (In some cases, this is better, since there's no guarantee that a given VM invocation that fails lots of speculations will ever run the GC.) Another option is to do a specific scan for CodeBlocks in the JS stack if and only if the set of jettisoned CodeBlocks reaches a threshold. (This scan can become much more efficient in the future, once JC's top-of-stack work is done, and we have exact backtrace information.) Or you can combine the two options. > Source/JavaScriptCore/heap/Heap.cpp:397 > +void Heap::jettisonCodeBlock(PassOwnPtr<CodeBlock> codeBlock) Since this doesn't actually do the jettisoning, I'd call it "addJettisonedCodeBlock" instead. > Source/JavaScriptCore/heap/Heap.h:197 > + JettisonedCodeBlocks m_codeBlocks; "m_jettisonedCodeBlocks", please. Otherwise, m_codeBlocks reads like it's the set of all CodeBlocks. > Source/JavaScriptCore/heap/JettisonedCodeBlocks.cpp:42 > + HashMap<CodeBlock*, bool>::iterator begin = m_map.begin(); > + HashMap<CodeBlock*, bool>::iterator end = m_map.end(); > + for (HashMap<CodeBlock*, bool>::iterator iter = begin; iter != end; ++iter) > + delete iter->first; Would deleteAllKeys work here? > Source/JavaScriptCore/heap/JettisonedCodeBlocks.h:72 > + // It would be great to use an OwnPtr<CodeBlock> here but that would > + // almost certainly not work. Yeah, HashMap doesn't support OwnPtr. > Source/JavaScriptCore/wtf/BitVector.h:57 > + : m_bitsOrPointer(static_cast<uintptr_t>(1) << maxInlineBits()) Since tagging bits with the inline tag comes up a lot, I'd suggest a helper function: uintptr_t makeInlineBits(uintptr_t bits). So, this would turn into a call to makeInlineBits(0). The helper function can also ASSERT that bits does not already have the tag bit set. > Source/JavaScriptCore/wtf/BitVector.h:90 > + size_t bounds() const Since this is modified by resize() and ensureSize(), "size()" is probably a better name for it. > Source/JavaScriptCore/wtf/BitVector.h:112 > + m_bitsOrPointer = (static_cast<uintptr_t>(1) << maxInlineBits()) | *myOutOfLineBits->bits(); This would become "makeInlineBits(*myOutOfLineBits->bits())", which seems cleaner. > Source/JavaScriptCore/wtf/BitVector.h:120 > + void clear() I think this would read a little better as "clearAll", matching Bitmap::clearAll. Our other collection types treat clear as setSize(0), so seeing clear() right after setSize(n) in client code is pretty weird. > Source/JavaScriptCore/wtf/BitVector.h:125 > + bzero(outOfLineBits()->bits(), bounds() >> 3); >> 3 would be a little less magical as a helper function: size_t byteCount(size_t bitCount); Seems to come up a lot. So this discussion uncovered a bug: JettisonedCodeBlocks traces all CodeBlocks it has, not just the ones that were marked. I'll fix that. But that observation also uncovers why your two proposals (blowing away JettisonedCodeBlocks on VM exit and/or having a separate mini-GC for JettisonedCodeBlocks) will cause a memory leak. When a GC happens, we want all dead state to go away, within the limits of conservative scanning. CodeBlocks can refer to a lot of stuff. So we only want to scan them if they're live. That means marking those jettisoned CodeBlocks that are found to be referenced from the RegisterFile. Your two proposals, as I understand them, won't do this, since they both imply scanning the RegisterFile for CodeBlocks in a separate phase that is not part of the GC. This means that the GC risks marking too much stuff, and hence, leaking memory. A memory leak is likely to be less performant than calling JettisonedCodeBlocks::mark() in ConservativeRoots. I see two options that are robust, and that address some of your concerns: 1) Add a markCodeBlocks(JettisonedCodeBlocks&) method to RegisterFile. This can have a fast check to see if JettisonedCodeBlocks is empty, in which case it does no work. This is probably the easiest conceptually, and will probably perform just fine. 2) Add an enum parameter to ConservativeRoots::add() that says if you're looking at a RegisterFile. This is the best option. ConservativeRoots already does a linear scan over the RegisterFile. It's most efficient to have that scan include CodeBlock detection, so we're only touching each location in the RegisterFile once during a collection. If that enum tells ConservativeRoots::add() that it's looking at the RegisterFile, it can go and look at JettisonedCodeBlocks. Otherwise it can ignore them. This parameter could even be a template parameter, or something like that, to ensure that we don't do extra checks when scanning the C stack. There's a separate question of whether or not to add the bloom filter (or similar) optimizations to JettisonedCodeBlocks. I don't think that will be necessary. JettisonedCodeBlocks should be small or empty. If it's empty then in (1) we can just bail out immediately. It's more likely to be small, but not empty, though, since the continuous optimization system does seem to want to recompile stuff in roughly half of our benchmarks. A small set of JettisonedCodeBlocks means a small hashtable, which is likely to be no slower than a bloom filter. If JettisonedCodeBlocks gets big, then it means that we've already lost, since it could only happen if a lot of recompilation occurred. My goal is to ensure that this doesn't happen. This should already be the case, because of the exponential back-off in recompilation that is already part of this patch. Created attachment 108112 [details]
the patch - fix review
I addressed Geoff's review, mostly. I believe that the GC must mark jettisoned CodeBlocks. I changed this so that ConservativeMarker no longer marks CodeBlocks unconditionally; it will only do so if add() is passed a JettisonedCodeBlocks. Internally this is templatized, so we could support any number of different "MarkHooks" in the ConservativeMarker.
All of the other issues are addressed.
I haven't addressed Oliver's two concerns (RefCounted on CodeBlock and logging). I'll look at the logging tomorrow.
Attachment 108112 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1
Source/JavaScriptCore/heap/ConservativeRoots.h:55: The parameter name "markHook" adds no information, so it should be removed. [readability/parameter_name] [5]
Source/JavaScriptCore/heap/ConservativeRoots.h:58: The parameter name "markHook" adds no information, so it should be removed. [readability/parameter_name] [5]
Total errors found: 2 in 27 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 108112 [details] the patch - fix review Attachment 108112 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/9774617 Created attachment 108113 [details]
the patch - fix style, attempt to fix GTK
Comment on attachment 108113 [details] the patch - fix style, attempt to fix GTK Attachment 108113 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/9773576 Created attachment 108116 [details]
the patch - another GTK attempt
Attachment 108116 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1
Source/JavaScriptCore/wtf/BitVector.h:33: Alphabetical sorting problem. [build/include_order] [4]
Total errors found: 1 in 27 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 108117 [details]
the patch - fix style
Comment on attachment 108117 [details] the patch - fix style View in context: https://bugs.webkit.org/attachment.cgi?id=108117&action=review Looks like the Windows build is still angry: 2>c:\cygwin\home\buildbot\WebKit\Source\JavaScriptCore\wtf/BitVector.h(31) : fatal error C1083: Cannot open include file: 'strings.h': No such file or directory > CodeBlocks can refer to a lot of stuff. So we only want to scan them if they're live. Fair point. > It's most efficient to have that scan include CodeBlock detection, so we're only touching each location in the RegisterFile once during a collection. Another fair point. I'm happy with the solution you came up with. (Thinking about this, it occurred to me that, long-term, if we GC-allocate CodeBlocks, the normal conservative scan will "just work".) r- for the Windows build. > Source/JavaScriptCore/heap/Heap.cpp:504 > + m_jettisonedCodeBlocks.traceCodeBlocks(visitor); This should be followed by visitor.drain(). > Source/JavaScriptCore/heap/Heap.h:103 > + void addJettisonCodeBlock(PassOwnPtr<CodeBlock>); Typo: Should be "addJettisonedCodeBlock", not "addJettisonCodeBlock". > Source/JavaScriptCore/wtf/BitVector.h:96 > + if (m_bitsOrPointer >> maxInlineBits()) Should be "isInline()". Created attachment 108215 [details]
the patch - fix review, windows
Fixed the vaerious typos that Geoff found. Fixed Windows, hopefully.
Comment on attachment 108215 [details]
the patch - fix review, windows
Windows build failure:
2>c:\cygwin\home\buildbot\WebKit\Source\JavaScriptCore\wtf/BitVector.h(132) : error C3861: 'bzero': identifier not found
Created attachment 108230 [details]
the patch - another attempt to fix Windows
Today I learned that bzero is deprecated.
Comment on attachment 108230 [details]
the patch - another attempt to fix Windows
I'm guessing this will build. Hope I don't jinx it.
(In reply to comment #33) > (From update of attachment 108230 [details]) > I'm guessing this will build. Hope I don't jinx it. Thanks, I'll wait until it's happy. Landed in r95681. Comment on attachment 108230 [details] the patch - another attempt to fix Windows View in context: https://bugs.webkit.org/attachment.cgi?id=108230&action=review > Source/JavaScriptCore/heap/JettisonedCodeBlocks.h:58 > + // This checks for both of those nasty cases in one go. > + // 0 + 1 = 1 > + // -1 + 1 = 0 > + if (value + 1 <= 1) > + return; Why not just value < 1 ? I guess ptr_t is unsigned? (In reply to comment #36) > (From update of attachment 108230 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=108230&action=review > > > Source/JavaScriptCore/heap/JettisonedCodeBlocks.h:58 > > + // This checks for both of those nasty cases in one go. > > + // 0 + 1 = 1 > > + // -1 + 1 = 0 > > + if (value + 1 <= 1) > > + return; > > Why not just value < 1 ? I guess ptr_t is unsigned? value is a uintptr_t, so yes. value < 1 would only check for the zero case. |