Non-API owner LGTM, as this is a very low-risk change. Its only expected impact will be a performance improvement/simplification.
And a reminder to API owners to look at this, since we've just switched V8/JS features over to using the Blink Intent process. On Thu, Apr 25, 2019 at 11:44 AM Sathya Gunasekaran <gsat...@chromium.org> wrote: > Contact emails > > gsat...@chromium.org > > Explainer > > https://github.com/tc39/proposal-promise-allSettled/pull/40 > > https://github.com/tc39/ecma262/issues/1505 > > Spec > > https://github.com/tc39/ecma262/pull/1506/files > > https://github.com/tc39/proposal-promise-allSettled/pull/40/files > > Summary > > Promise.{all <https://tc39.github.io/ecma262/#sec-performpromiseall>, race > <https://tc39.github.io/ecma262/#sec-performpromiserace>, allSettled > <https://tc39.github.io/proposal-promise-allSettled/>} lookup the > constructor of the receiver on every iteration of the loop. Instead, this > change makes the lookup happen only once outside the loop. > > Motivation > > To optimize away the Get and Call of the resolve method on the > constructor, I'd have to check the constructor to see if its resolve method > has been modified or not. If it's not been modified, I can directly call > (or even inline) the builtin %PromiseResolve%, saving the lookup and call > overhead for faster performance. > > Without this patch, I would have to check against the constructor for > every iteration of the loop before going to the fast path. With this patch, > I can do a check against the constructor just once at the beginning. > > The change in behavior with this patch is that if you modify the > constructor's resolve property in the middle of iterating the iterable > argument, then it is not observed. > > Risks > > Interoperability and Compatibility > > There is a very small risk of interop and web compatibility in the case of > Promise.all and Promise.race, as we are modifying the behavior of methods > that have been shipping for a while. There is no risk of interop or web > compat with Promise.allSettled as it is a new proposal. > > I expect the risk to be very low considering the surprising side-effecting > nature of changing the resolve method *while* iterating over the argument > or if there is a user installed ‘resolve’ getter that side effects. > > Unfortunately it’s not possible to use counters to determine if this will > break or not -- we can’t count the side effects of calling a method. We can > only determine if the resolve method has been patched or not, but that’s > not fully sufficient to determine the risk. > > Given the very low risk of this surprising behavior, I’d like to ship this > to determine if there is any breakage. > > Ergonomics > > N/A > > Activation > > N/A > > > Is this feature supported on all six Blink platforms (Windows, Mac, Linux, > Chrome OS, Android, and Android WebView)? > > Yes. > > Debuggability > > This doesn’t change any of the debugging functionality of Promises. > > Is this feature fully tested by web-platform-tests > <https://chromium.googlesource.com/chromium/src/+/master/docs/testing/web_platform_tests.md>? > Link to test suite results from wpt.fyi. > > V8 passes all the test262 tests: https://github.com/tc39/test262/pull/2131 > > Entry on the feature dashboard <http://www.chromestatus.com/> > > https://www.chromestatus.com/feature/5171235300311040 > > Requesting approval to ship? > > Yes > > -- > You received this message because you are subscribed to the Google Groups > "blink-dev" group. > To view this discussion on the web visit > https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAMd%2BM7wqinO2%3DbAMH%2BhyKzAEQowJXodfV9ioBEzemDfYDshPEg%40mail.gmail.com > <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAMd%2BM7wqinO2%3DbAMH%2BhyKzAEQowJXodfV9ioBEzemDfYDshPEg%40mail.gmail.com?utm_medium=email&utm_source=footer> > . > -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups "v8-dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
