[JSC] Optimize Promise runtime functions
Created attachment 382005 [details] Patch
Created attachment 382006 [details] Patch
Created attachment 382007 [details] Patch
<rdar://problem/56644056>
Comment on attachment 382007 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=382007&action=review > Source/JavaScriptCore/builtins/AsyncFunctionPrototype.js:53 > @resolveWithoutPromise(value, > - function(value) { @asyncFunctionResume(generator, promise, value, @GeneratorResumeModeNormal); }, > - function(error) { @asyncFunctionResume(generator, promise, error, @GeneratorResumeModeThrow); }); > + function(value) { @asyncFunctionResume(capturedGenerator, capturedPromise, value, @GeneratorResumeModeNormal); }, > + function(error) { @asyncFunctionResume(capturedGenerator, capturedPromise, error, @GeneratorResumeModeThrow); }); I'm also planning to introduce some special promise-reaction here to avoid 2 function allocations (that is never phantom-ed since it is posted to the next microtask, so we should avoid allocation manually as much as possible).
Comment on attachment 382007 [details] Patch Attachment 382007 [details] did not pass jsc-ews (mac): Output: https://webkit-queues.webkit.org/results/13180699 New failing tests: mozilla-tests.yaml/js1_5/Array/regress-101964.js.mozilla-ftl-eager-no-cjit-validate-phases
Comment on attachment 382007 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=382007&action=review >> Source/JavaScriptCore/builtins/AsyncFunctionPrototype.js:53 >> + function(error) { @asyncFunctionResume(capturedGenerator, capturedPromise, error, @GeneratorResumeModeThrow); }); > > I'm also planning to introduce some special promise-reaction here to avoid 2 function allocations (that is never phantom-ed since it is posted to the next microtask, so we should avoid allocation manually as much as possible). Yeah, seems like this could be global private method. I guess we'd need to handle them specially though to make sure we pass the state. > Source/JavaScriptCore/builtins/PromiseConstructor.js:269 > + var @reject = function @reject(reason) { > return @rejectPromiseWithFirstResolvingFunctionCallCheck(capturedPromise, reason); > - } > + }; Why make this change? > Source/JavaScriptCore/builtins/PromiseOperations.js:96 > + return { > + @resolve: @resolve, > + @reject: @reject, > + @promise: promise, > + }; Can you just do `return { @resolve, @reject, @promise: promise }` or does that not work for private identifiers? > JSTests/microbenchmarks/promise-reject.js:3 > +drainMicrotasks(); I don't think you need this? Don't we drain microtasks before exiting anyway?
Comment on attachment 382007 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=382007&action=review Thanks, soon, I'll upload the patch again. >>> Source/JavaScriptCore/builtins/AsyncFunctionPrototype.js:53 >>> + function(error) { @asyncFunctionResume(capturedGenerator, capturedPromise, error, @GeneratorResumeModeThrow); }); >> >> I'm also planning to introduce some special promise-reaction here to avoid 2 function allocations (that is never phantom-ed since it is posted to the next microtask, so we should avoid allocation manually as much as possible). > > Yeah, seems like this could be global private method. I guess we'd need to handle them specially though to make sure we pass the state. Yeah. I think we can create a reaction which holds generator and promise. Then chain this reaction to promise's queue, and it will be resolved appropriately. Then, we can avoid three allocations here, 1. One for fullfill handler, 2. One for reject handler, 3. One for lexical environment holding capturedGenerator and capturedPromise. >> Source/JavaScriptCore/builtins/PromiseConstructor.js:269 >> + }; > > Why make this change? It is hacky but this reduces bytecode size to make bytecodes of this function tight to make inlining available. I'm planning to make global private function resolution as constant loading and make this bytecode further smaller. >> Source/JavaScriptCore/builtins/PromiseOperations.js:96 >> + }; > > Can you just do `return { @resolve, @reject, @promise: promise }` or does that not work for private identifiers? I'll fix this.
Comment on attachment 382007 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=382007&action=review >>> Source/JavaScriptCore/builtins/PromiseConstructor.js:269 >>> + }; >> >> Why make this change? > > It is hacky but this reduces bytecode size to make bytecodes of this function tight to make inlining available. I'm planning to make global private function resolution as constant loading and make this bytecode further smaller. Oh, from offline you said this is because we add extra moves when emitting the function var syntax. Can you file a bug to fix those extra moves. It seems like this problem would extend to websites too.
Comment on attachment 382007 [details] Patch r=me.
Comment on attachment 382007 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=382007&action=review Thanks! >>>> Source/JavaScriptCore/builtins/PromiseConstructor.js:269 >>>> + }; >>> >>> Why make this change? >> >> It is hacky but this reduces bytecode size to make bytecodes of this function tight to make inlining available. I'm planning to make global private function resolution as constant loading and make this bytecode further smaller. > > Oh, from offline you said this is because we add extra moves when emitting the function var syntax. Can you file a bug to fix those extra moves. It seems like this problem would extend to websites too. Filed this in https://bugs.webkit.org/show_bug.cgi?id=203502
Committed r251671: <https://trac.webkit.org/changeset/251671>
*** Bug 201452 has been marked as a duplicate of this bug. ***