Bug 213317 – Promise built-in functions should be anonymous non-constructors

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

[Alexey Shvayka]

Promise built-in functions should be anonymous non-constructors

[Alexey Shvayka]

Created attachment 402158 [details]
Patch

[Yusuke Suzuki]

Comment on attachment 402158 [details]
Patch

Can you ensure what bytecode is actually emitted for the rewritten functions and Promise/InternalPromise constructors? This part is super performance critical part. So we need to be extra careful.

[Alexey Shvayka]

Comment on attachment 402158 [details]
Patch

Thank you for review, Darin!

I am denying commit queue to make sure that comma operator doesn't impact emitted bytecode & Promise microbenchmarks are neutral.

[Alexey Shvayka]

Created attachment 402207 [details]
Patch

Add info on microbenchmarks and bytecode to ChangeLog.

[Alexey Shvayka]

(In reply to Yusuke Suzuki from comment #2)
> Comment on attachment 402158 [details]
> Patch
> 
> Can you ensure what bytecode is actually emitted for the rewritten functions
> and Promise/InternalPromise constructors? This part is super performance
> critical part. So we need to be extra careful.

Promise constructors bytecode is unchanged (new_func_exp is emitted), while @createResolvingFunctions (used in slow path for `Promise.resolve(thenable)`) bytecode is reduced by 2 ops:

```
-createResolvingFunctions#BmeHaL:[0x10cfbc480->0x10cfe5600, NoneFunctionCall, 66 (StrictMode)]: 17 instructions (0 16-bit instructions, 0 32-bit instructions, 5 instructions with metadata); 186 bytes (120 metadata bytes); 2 parameter(s); 12 callee register(s); 10 variable(s); scope at loc4
+createResolvingFunctions#DBfCeH:[0x1035bc480->0x1035e5600, NoneFunctionCall, 60 (StrictMode)]: 15 instructions (0 16-bit instructions, 0 32-bit instructions, 5 instructions with metadata); 180 bytes (120 metadata bytes); 2 parameter(s); 12 callee register(s); 10 variable(s); scope at loc4
 [   0] enter              
 [   1] get_scope          loc4
 [   3] mov                loc5, loc4
 [   6] check_traps        
 [   7] mov                loc6, callee
-[  10] create_lexical_environment loc7, loc4, Cell: 0x10cfb80c0 (0x10cff8460:[0x383, SymbolTable, {}, NonArray, Leaf]), StructureID: 899(const0), Undefined(const1)
+[  10] create_lexical_environment loc7, loc4, Cell: 0x1035b80c0 (0x1035f8460:[0x450a, SymbolTable, {}, NonArray, Leaf]), StructureID: 17674(const0), Undefined(const1)
 [  15] mov                loc4, loc7
 [  18] put_to_scope       loc7, 0, arg1, 1073743876<ThrowIfNotFound|LocalClosureVar|NotInitialization|StrictMode>, 0, 0
-[  26] new_func           loc10, loc4, 0
-[  30] mov                loc8, loc10
-[  33] new_func           loc10, loc4, 1
-[  37] mov                loc9, loc10
-[  40] put_to_scope       loc7, 1, False(const2), 1073741828<ThrowIfNotFound|LocalClosureVar|Initialization|StrictMode>, 0, 1
-[  48] new_object         loc10, 2
-[  52] put_by_id          loc10, 2, loc8, IsDirect|Strict
-[  58] put_by_id          loc10, 3, loc9, IsDirect|Strict
-[  64] ret                loc10
+[  26] put_to_scope       loc7, 1, False(const2), 1073741828<ThrowIfNotFound|LocalClosureVar|Initialization|StrictMode>, 0, 1
+[  34] new_func_exp       loc9, loc4, 0
+[  38] new_func_exp       loc8, loc4, 1
+[  42] new_object         loc10, 2
+[  46] put_by_id          loc10, 2, loc9, IsDirect|Strict
+[  52] put_by_id          loc10, 3, loc8, IsDirect|Strict
+[  58] ret                loc10
 
 Identifiers:
   id0 = promise
   id1 = alreadyResolved
   id2 = PrivateSymbol.resolve
   id3 = PrivateSymbol.reject
 
 Constants:
-   k0 = Cell: 0x10cfb80c0 (0x10cff8460:[0x383, SymbolTable, {}, NonArray, Leaf]), StructureID: 899
+   k0 = Cell: 0x1035b80c0 (0x1035f8460:[0x450a, SymbolTable, {}, NonArray, Leaf]), StructureID: 17674
    k1 = Undefined
    k2 = False
```

Promise microbenchmarks are neutral.

[Yusuke Suzuki]

Comment on attachment 402207 [details]
Patch

Looks good!

[EWS]

Committed r263222: <https://trac.webkit.org/changeset/263222>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 402207 [details].

[Radar WebKit Bug Importer]

<rdar://problem/64497346>