Bug 192880 – Changing for-in loop variable from var to let should not prevent bytecode loop optimization.

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Mark Lam]

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.

[Radar WebKit Bug Importer]

<rdar://problem/46852075>

[Tadeu Zagallo]

Created attachment 364015 [details]
Patch

[Saam Barati]

Comment on attachment 364015 [details]
Patch

R=me. Please add a micro benchmark.

[Tadeu Zagallo]

Created attachment 364033 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 364033 [details]
Patch for landing

Clearing flags on attachment: 364033

Committed r242649: <https://trac.webkit.org/changeset/242649>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.