[webkit-reviews] review canceled: [Bug 200898] [JSC] Make Promise implementation faster : [Attachment 377482] Patch

[bugzilla-daemon at webkit.org]

Yusuke Suzuki <ysuzuki at apple.com> has canceled Yusuke Suzuki
<ysuzuki at apple.com>'s request for review:
Bug 200898: [JSC] Make Promise implementation faster
https://bugs.webkit.org/show_bug.cgi?id=200898

Attachment 377482: Patch

https://bugs.webkit.org/attachment.cgi?id=377482&action=review




--- Comment #67 from Yusuke Suzuki <ysuzuki at apple.com> ---
Comment on attachment 377482
  --> https://bugs.webkit.org/attachment.cgi?id=377482
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=377482&action=review

Thanks, I'll quickly update the patch.

>> Source/JavaScriptCore/ChangeLog:57
>> +		takes `onFulfilled` and `onRejected` handlers and they do not
need an intermediate promise for resolving. This removes internal promise
allocations
> 
> Nit: They takes => they take

Fixed.

>> Source/JavaScriptCore/builtins/AsyncFromSyncIteratorPrototype.js:37
>> +	var syncIterator = @getByIdDirectPrivate(this, "syncIterator");
> 
> Is there a reason you made these all vars?

const/let emits JSEmpty initialization mov bytecode, and I do not want to emit
unnecessary bytecode especially for builtin JS.
Since builtin-js is shared and standard library, I think it is worth doing
micro-optimization.

>> Source/JavaScriptCore/builtins/PromiseOperations.js:86
>> +	    return { @promise: promise, @resolve: resolvingFunctions. at resolve,
@reject: resolvingFunctions. at reject };
> 
> Why not do:
> 
> @putDirectPrivate(resolvingFunctions, “promise”, promise);
> return resolvingFunctions;

No specific reasons. We can just put here. Fixed.