192880 2018-12-19 14:13:54 -0800 Changing for-in loop variable from var to let should not prevent bytecode loop optimization. 2019-03-08 11:18:26 -0800 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 mark.lam tzagallo commit-queue fpizlo keith_miller msaboff rmorisset saam tzagallo webkit-bug-importer ysuzuki oldest_to_newest 1489472 0 mark.lam 2018-12-19 14:13:54 -0800 Consider the difference between the following 2 cases where the only difference in the JS source is changing the for-in loop variable from a var to a let: Case 1: the test in JSTests/microbenchmarks/for-in-on-object-with-lazily-materialized-properties.js: function foo(o) { var count = 0; for (var p in o) { //<============ declares p as a var if (o[p]) count ++; } return count; } noInline(foo); var total = 0; for (let j = 0; j < 100000; ++j) total += foo(new Error); This generates the following bytecode: foo#DjvThs:[0x115348000->0x11537ce70, BaselineFunctionCall, 153 (NeverInline)]: 55 instructions (2 wide instructions, 4 instructions with metadata); 379 bytes (188 metadata bytes); 2 parameter(s); 16 callee register(s); 9 variable(s); scope at loc4 [ 0] enter [ 1] get_scope loc4 [ 3] mov loc5, loc4 [ 6] check_traps [ 7] mov loc7, Int32: 0(const0) [ 10] mov loc9, arg1 [ 13] get_property_enumerator loc10, loc9 [ 16] get_enumerable_length loc11, loc10 [ 19] mov loc12, Int32: 0(const1) [ 22] loop_hint [ 23] check_traps [ 24] less loc14, loc12, loc11 [ 28] jfalse loc14, 49 [ 31] has_indexed_property loc14, loc9, loc12 [ 36] jfalse loc14, 35 [ 39] to_index_string loc13, loc12 [ 42] mov loc6, loc13 [ 45] *get_by_val loc15, arg1, loc12 [ 66] jfalse loc15, 5 [ 69] inc loc7 [ 71] inc loc12 [ 73] jmp -51 [ 75] jmp 114 [ 77] mov loc12, Int32: 0(const1) [ 80] enumerator_structure_pname loc13, loc10, loc12 [ 84] loop_hint [ 85] check_traps [ 86] eq_null loc14, loc13 [ 89] jtrue loc14, 58 [ 92] has_structure_property loc14, loc9, loc13, loc10 [ 97] jfalse loc14, 40 [ 100] mov loc6, loc13 [ 103] *get_direct_pname loc15, arg1, loc6, loc12, loc10 // <======== for-in loop uses the optimizing get_direct_pname. [ 132] jfalse loc15, 5 [ 135] inc loc7 [ 137] inc loc12 [ 139] enumerator_structure_pname loc13, loc10, loc12 [ 143] jmp -59 [ 145] jmp 44 [ 147] enumerator_generic_pname loc13, loc10, loc12 [ 151] loop_hint [ 152] check_traps [ 153] eq_null loc14, loc13 [ 156] jtrue loc14, 33 [ 159] has_generic_property loc14, loc9, loc13 [ 163] jfalse loc14, 16 [ 166] mov loc6, loc13 [ 169] get_by_val loc15, arg1, loc6 [ 174] jfalse loc15, 5 [ 177] inc loc7 [ 179] inc loc12 [ 181] enumerator_generic_pname loc13, loc10, loc12 [ 185] jmp -34 [ 187] jmp 2 [ 189] ret loc7 Constants: k0 = Int32: 0: in source as integer k1 = Int32: 0 Jump targets: 22, 71, 77, 84, 137, 147, 151, 179, 189 Case 2: the test in JSTests/microbenchmarks/for-in-on-object-with-lazily-materialized-properties.js with the var loop variable changed to a let: function foo(o) { var count = 0; for (let p in o) { //<============ declares p as a let (changed from var) if (o[p]) count ++; } return count; } noInline(foo); var total = 0; for (let j = 0; j < 100000; ++j) total += foo(new Error); The generated bytecode now looks like this: Parsing foo#BSyug2:[0x10cd48660->0x10cd48000->0x10cd7ce70, NoneFunctionCall, 168 (NeverInline)], isStrictMode = false foo#BSyug2:[0x10cd48000->0x10cd7ce70, BaselineFunctionCall, 168 (NeverInline)]: 67 instructions (2 wide instructions, 4 instructions with metadata); 388 bytes (188 metadata bytes); 2 parameter(s); 16 callee register(s); 8 variable(s); scope at loc4 [ 0] enter [ 1] get_scope loc4 [ 3] mov loc5, loc4 [ 6] check_traps [ 7] mov loc6, Int32: 0(const0) [ 10] mov loc8, <JSValue()>(const1) [ 13] mov loc9, arg1 [ 16] get_property_enumerator loc10, loc9 [ 19] get_enumerable_length loc11, loc10 [ 22] mov loc12, Int32: 0(const2) [ 25] loop_hint [ 26] check_traps [ 27] less loc14, loc12, loc11 [ 31] jfalse loc14, 51 [ 34] has_indexed_property loc14, loc9, loc12 [ 39] jfalse loc14, 37 [ 42] to_index_string loc13, loc12 [ 45] mov loc8, loc13 [ 48] check_tdz loc8 [ 50] *get_by_val loc15, arg1, loc8 [ 71] jfalse loc15, 5 [ 74] inc loc6 [ 76] inc loc12 [ 78] jmp -53 [ 80] jmp 118 [ 82] mov loc12, Int32: 0(const2) [ 85] enumerator_structure_pname loc13, loc10, loc12 [ 89] loop_hint [ 90] check_traps [ 91] eq_null loc14, loc13 [ 94] jtrue loc14, 60 [ 97] has_structure_property loc14, loc9, loc13, loc10 [ 102] jfalse loc14, 42 [ 105] mov loc8, loc13 [ 108] check_tdz loc8 [ 110] *get_by_val loc15, arg1, loc8 // <======== for-in loop no longer uses the optimizing get_direct_pname. [ 131] nop [ 132] nop [ 133] nop [ 134] nop [ 135] nop [ 136] nop [ 137] nop [ 138] nop [ 139] jfalse loc15, 5 [ 142] inc loc6 [ 144] inc loc12 [ 146] enumerator_structure_pname loc13, loc10, loc12 [ 150] jmp -61 [ 152] jmp 46 [ 154] enumerator_generic_pname loc13, loc10, loc12 [ 158] loop_hint [ 159] check_traps [ 160] eq_null loc14, loc13 [ 163] jtrue loc14, 35 [ 166] has_generic_property loc14, loc9, loc13 [ 170] jfalse loc14, 18 [ 173] mov loc8, loc13 [ 176] check_tdz loc8 [ 178] get_by_val loc15, arg1, loc8 [ 183] jfalse loc15, 5 [ 186] inc loc6 [ 188] inc loc12 [ 190] enumerator_generic_pname loc13, loc10, loc12 [ 194] jmp -36 [ 196] jmp 2 [ 198] ret loc6 Constants: k0 = Int32: 0: in source as integer k1 = <JSValue()> k2 = Int32: 0 Jump targets: 25, 76, 82, 89, 144, 154, 158, 188, 198 This was run on WebKit trunk r239389. 1489473 1 webkit-bug-importer 2018-12-19 14:14:47 -0800 <rdar://problem/46852075> 1514011 2 364015 tzagallo 2019-03-08 08:53:33 -0800 Created attachment 364015 Patch 1514014 3 364015 saam 2019-03-08 08:57:49 -0800 Comment on attachment 364015 Patch R=me. Please add a micro benchmark. 1514070 4 364033 tzagallo 2019-03-08 10:40:42 -0800 Created attachment 364033 Patch for landing 1514095 5 364033 commit-queue 2019-03-08 11:18:24 -0800 Comment on attachment 364033 Patch for landing Clearing flags on attachment: 364033 Committed r242649: <https://trac.webkit.org/changeset/242649> 1514096 6 commit-queue 2019-03-08 11:18:26 -0800 All reviewed patches have been landed. Closing bug. 364015 2019-03-08 08:53:33 -0800 2019-03-08 10:40:38 -0800 Patch bug-192880-20190308175332.patch text/plain 1768 tzagallo U3VidmVyc2lvbiBSZXZpc2lvbjogMjQyNjQwCmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCBi OTEyYmRmMjNmMjBiNzZjZmViYmU5NzgwMGRkNjA0MzM2Y2FhZmNjLi45NGIzYzc0NTU4NDliNGI4 NmJmNjQwNDI4MzE5ZTM5MDBjMTgzZjMxIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxNyBAQAorMjAxOS0wMy0wOCAgVGFkZXUgWmFnYWxsbyAgPHR6YWdhbGxvQGFwcGxlLmNv bT4KKworICAgICAgICBvcF9jaGVja190ZHogZG9lcyBub3QgZGVmIGl0cyBhcmd1bWVudAorICAg ICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTkyODgwCisgICAg ICAgIDxyZGFyOi8vcHJvYmxlbS80NjIyMTU5OD4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JP RFkgKE9PUFMhKS4KKworICAgICAgICBUaGlzIHByZXZlbnRlZCB0aGUgZm9yLWluIGxvb3Agb3B0 aW1pemF0aW9uIGluIHRoZSBieXRlY29kZSBnZW5lcmF0b3IsIHNpbmNlCisgICAgICAgIHRoZSBh bmFseXNpcyBzZWVzIGEgcmVkZWZpbml0aW9uIG9mIHRoZSBsb29wIHZhcmlhYmxlLgorCisgICAg ICAgICogYnl0ZWNvZGUvQnl0ZWNvZGVVc2VEZWYuaDoKKyAgICAgICAgKEpTQzo6Y29tcHV0ZURl ZnNGb3JCeXRlY29kZU9mZnNldCk6CisKIDIwMTktMDMtMDcgIFl1c3VrZSBTdXp1a2kgIDx5c3V6 dWtpQGFwcGxlLmNvbT4KIAogICAgICAgICBbSlNDXSBNYWtlIG1vcmUgZmllbGRzIGxhenkgaW4g SlNHbG9iYWxPYmplY3QKZGlmZiAtLWdpdCBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9ieXRlY29k ZS9CeXRlY29kZVVzZURlZi5oIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL2J5dGVjb2RlL0J5dGVj b2RlVXNlRGVmLmgKaW5kZXggMWVlZTU4NDQ2MDdmZmIxZWI2NjU5ZjU5ZGVmYjJjODU2YmM2Njg1 My4uNmE5ZWU1MzU1ODg0ZmNkODY5NTI4OThmNmRhYjE0OGUyMzc1OWVjYSAxMDA2NDQKLS0tIGEv U291cmNlL0phdmFTY3JpcHRDb3JlL2J5dGVjb2RlL0J5dGVjb2RlVXNlRGVmLmgKKysrIGIvU291 cmNlL0phdmFTY3JpcHRDb3JlL2J5dGVjb2RlL0J5dGVjb2RlVXNlRGVmLmgKQEAgLTI5NSw2ICsy OTUsNyBAQCB2b2lkIGNvbXB1dGVEZWZzRm9yQnl0ZWNvZGVPZmZzZXQoQmxvY2sqIGNvZGVCbG9j aywgT3Bjb2RlSUQgb3Bjb2RlSUQsIGNvbnN0IElucwogICAgIGNhc2Ugb3BfZW5kOgogICAgIGNh c2Ugb3BfdGhyb3c6CiAgICAgY2FzZSBvcF90aHJvd19zdGF0aWNfZXJyb3I6CisgICAgY2FzZSBv cF9jaGVja190ZHo6CiAgICAgY2FzZSBvcF9kZWJ1ZzoKICAgICBjYXNlIG9wX3JldDoKICAgICBj YXNlIG9wX2ptcDoKQEAgLTQ0OCw3ICs0NDksNiBAQCB2b2lkIGNvbXB1dGVEZWZzRm9yQnl0ZWNv ZGVPZmZzZXQoQmxvY2sqIGNvZGVCbG9jaywgT3Bjb2RlSUQgb3Bjb2RlSUQsIGNvbnN0IElucwog ICAgIERFRlMoT3BNb3YsIGRzdCkKICAgICBERUZTKE9wTmV3T2JqZWN0LCBkc3QpCiAgICAgREVG UyhPcFRvVGhpcywgc3JjRHN0KQotICAgIERFRlMoT3BDaGVja1RkeiwgdGFyZ2V0VmlydHVhbFJl Z2lzdGVyKQogICAgIERFRlMoT3BHZXRTY29wZSwgZHN0KQogICAgIERFRlMoT3BDcmVhdGVEaXJl Y3RBcmd1bWVudHMsIGRzdCkKICAgICBERUZTKE9wQ3JlYXRlU2NvcGVkQXJndW1lbnRzLCBkc3Qp Cg== 364033 2019-03-08 10:40:42 -0800 2019-03-08 11:18:24 -0800 Patch for landing bug-192880-20190308194041.patch text/plain 2975 tzagallo U3VidmVyc2lvbiBSZXZpc2lvbjogMjQyNjQwCmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCBi OTEyYmRmMjNmMjBiNzZjZmViYmU5NzgwMGRkNjA0MzM2Y2FhZmNjLi4wODIyNDBiOTA5YWY3NDRm OGNhNWVkYjI5NDFjZTk3MzU1MWVhZTZhIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxNyBAQAorMjAxOS0wMy0wOCAgVGFkZXUgWmFnYWxsbyAgPHR6YWdhbGxvQGFwcGxlLmNv bT4KKworICAgICAgICBvcF9jaGVja190ZHogZG9lcyBub3QgZGVmIGl0cyBhcmd1bWVudAorICAg ICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTkyODgwCisgICAg ICAgIDxyZGFyOi8vcHJvYmxlbS80NjIyMTU5OD4KKworICAgICAgICBSZXZpZXdlZCBieSBTYWFt IEJhcmF0aS4KKworICAgICAgICBUaGlzIHByZXZlbnRlZCB0aGUgZm9yLWluIGxvb3Agb3B0aW1p emF0aW9uIGluIHRoZSBieXRlY29kZSBnZW5lcmF0b3IsIHNpbmNlCisgICAgICAgIHRoZSBhbmFs eXNpcyBzZWVzIGEgcmVkZWZpbml0aW9uIG9mIHRoZSBsb29wIHZhcmlhYmxlLgorCisgICAgICAg ICogYnl0ZWNvZGUvQnl0ZWNvZGVVc2VEZWYuaDoKKyAgICAgICAgKEpTQzo6Y29tcHV0ZURlZnNG b3JCeXRlY29kZU9mZnNldCk6CisKIDIwMTktMDMtMDcgIFl1c3VrZSBTdXp1a2kgIDx5c3V6dWtp QGFwcGxlLmNvbT4KIAogICAgICAgICBbSlNDXSBNYWtlIG1vcmUgZmllbGRzIGxhenkgaW4gSlNH bG9iYWxPYmplY3QKZGlmZiAtLWdpdCBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9ieXRlY29kZS9C eXRlY29kZVVzZURlZi5oIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL2J5dGVjb2RlL0J5dGVjb2Rl VXNlRGVmLmgKaW5kZXggMWVlZTU4NDQ2MDdmZmIxZWI2NjU5ZjU5ZGVmYjJjODU2YmM2Njg1My4u NmE5ZWU1MzU1ODg0ZmNkODY5NTI4OThmNmRhYjE0OGUyMzc1OWVjYSAxMDA2NDQKLS0tIGEvU291 cmNlL0phdmFTY3JpcHRDb3JlL2J5dGVjb2RlL0J5dGVjb2RlVXNlRGVmLmgKKysrIGIvU291cmNl L0phdmFTY3JpcHRDb3JlL2J5dGVjb2RlL0J5dGVjb2RlVXNlRGVmLmgKQEAgLTI5NSw2ICsyOTUs NyBAQCB2b2lkIGNvbXB1dGVEZWZzRm9yQnl0ZWNvZGVPZmZzZXQoQmxvY2sqIGNvZGVCbG9jaywg T3Bjb2RlSUQgb3Bjb2RlSUQsIGNvbnN0IElucwogICAgIGNhc2Ugb3BfZW5kOgogICAgIGNhc2Ug b3BfdGhyb3c6CiAgICAgY2FzZSBvcF90aHJvd19zdGF0aWNfZXJyb3I6CisgICAgY2FzZSBvcF9j aGVja190ZHo6CiAgICAgY2FzZSBvcF9kZWJ1ZzoKICAgICBjYXNlIG9wX3JldDoKICAgICBjYXNl IG9wX2ptcDoKQEAgLTQ0OCw3ICs0NDksNiBAQCB2b2lkIGNvbXB1dGVEZWZzRm9yQnl0ZWNvZGVP ZmZzZXQoQmxvY2sqIGNvZGVCbG9jaywgT3Bjb2RlSUQgb3Bjb2RlSUQsIGNvbnN0IElucwogICAg IERFRlMoT3BNb3YsIGRzdCkKICAgICBERUZTKE9wTmV3T2JqZWN0LCBkc3QpCiAgICAgREVGUyhP cFRvVGhpcywgc3JjRHN0KQotICAgIERFRlMoT3BDaGVja1RkeiwgdGFyZ2V0VmlydHVhbFJlZ2lz dGVyKQogICAgIERFRlMoT3BHZXRTY29wZSwgZHN0KQogICAgIERFRlMoT3BDcmVhdGVEaXJlY3RB cmd1bWVudHMsIGRzdCkKICAgICBERUZTKE9wQ3JlYXRlU2NvcGVkQXJndW1lbnRzLCBkc3QpCmRp ZmYgLS1naXQgYS9KU1Rlc3RzL0NoYW5nZUxvZyBiL0pTVGVzdHMvQ2hhbmdlTG9nCmluZGV4IGJi YTVmMWNmZThlZDI1ZGNmYjlmNTc0NGI2ZDE5MTVmMzFkOGViOGIuLmU5NThmODZiN2JlMjVhZmJk OTRjMDZhYTgzMTI2MmM4MDE4ZDE3ZjcgMTAwNjQ0Ci0tLSBhL0pTVGVzdHMvQ2hhbmdlTG9nCisr KyBiL0pTVGVzdHMvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTQgQEAKKzIwMTktMDMtMDggIFRhZGV1 IFphZ2FsbG8gIDx0emFnYWxsb0BhcHBsZS5jb20+CisKKyAgICAgICAgb3BfY2hlY2tfdGR6IGRv ZXMgbm90IGRlZiBpdHMgYXJndW1lbnQKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcv c2hvd19idWcuY2dpP2lkPTE5Mjg4MAorICAgICAgICA8cmRhcjovL3Byb2JsZW0vNDYyMjE1OTg+ CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgU2FhbSBCYXJhdGkuCisKKyAgICAgICAgKiBtaWNyb2Jl bmNobWFya3MvbGV0LWZvci1pbi5qczogQWRkZWQuCisgICAgICAgIChmb28pOgorCiAyMDE5LTAz LTA3ICBZdXN1a2UgU3V6dWtpICA8eXN1enVraUBhcHBsZS5jb20+CiAKICAgICAgICAgW0pTQ10g U3RyaW5nRnJvbUNoYXJDb2RlIGZhc3QgcGF0aCBzaG91bGQgYWNjZXB0IDB4ZmYgaW4gREZHIGFu ZCBGVEwKZGlmZiAtLWdpdCBhL0pTVGVzdHMvbWljcm9iZW5jaG1hcmtzL2xldC1mb3ItaW4uanMg Yi9KU1Rlc3RzL21pY3JvYmVuY2htYXJrcy9sZXQtZm9yLWluLmpzCm5ldyBmaWxlIG1vZGUgMTAw NjQ0CmluZGV4IDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAuLmY1NzMz ZDA5ODFkYWM3OGIyNzBkYmVjNWEzOGQyYmEwZmE2MWU5OWEKLS0tIC9kZXYvbnVsbAorKysgYi9K U1Rlc3RzL21pY3JvYmVuY2htYXJrcy9sZXQtZm9yLWluLmpzCkBAIC0wLDAgKzEsMTUgQEAKK2Z1 bmN0aW9uIGZvbyhvKSB7CisgICAgdmFyIGNvdW50ID0gMDsKKyAgICBmb3IgKGxldCBwIGluIG8p IHsKKyAgICAgICAgaWYgKG9bcF0pCisgICAgICAgICAgICBjb3VudCsrOworICAgIH0KKyAgICBy ZXR1cm4gY291bnQ7Cit9Citub0lubGluZShmb28pOworCit2YXIgdG90YWwgPSAwOworZm9yIChs ZXQgaiA9IDA7IGogPCAxMDAwMDA7ICsraikKKyAgICB0b3RhbCArPSBmb28obmV3IEVycm9yKTsK K2lmICh0b3RhbCAhPSAzMDAwMDApCisgICAgdGhyb3cgbmV3IEVycm9yKCJCYWQgcmVzdWx0OiAi ICsgdG90YWwpOwo=