Re: Avoiding jank in async functions/promises?
On Sun, May 21, 2017, at 09:29 PM, Mark Hammond wrote: > As I mentioned at the start of the thread, in one concrete example we > had code already written that we identified being janky - > http://searchfox.org/mozilla-central/rev/f55349994fdac101d121b11dac769f3f17fbec4b/toolkit/components/places/PlacesUtils.jsm#2022 Is this a scenario where we could move this intensive logic off the (parent process) main thread by fulfilling the dream of the "SQLite interface for Workers" bug[1] by using WebIDL instead of js-ctypes to let the Sqlite.jsm abstraction operate on ChromeWorkers? The only XPConnect leakage on the Sqlite.jsm API surface is mozIStorageStatementRow[2], and although it's a bit unwieldy in terms of method count, we never exposed any of the nsIXPCScriptable magic on it that we did on statements. (And thankfully SQLite.jsm neither uses or otherwise exposes the API.) It wouldn't be a trivial undertaking, but it's also not impossible either. And if Sync is chewing up a lot of main thread time both directly (processing) and indirectly (generating lots of of garbage that lengthens parent-process main-thread GC's), it may be worth considering rather than trying to optimize the time-slicing of Sync. This does, of course, assume that Sync can do meaningful work without access to XPConnect and that there aren't major gotchas in coordinating with Places' normal operation. Note: I'm talking exclusively about using the existing asynchronous Sqlite.jsm API on top of the existing async mozStorage API usage. Andrew 1: https://bugzilla.mozilla.org/show_bug.cgi?id=853438 2: https://searchfox.org/mozilla-central/source/storage/mozIStorageRow.idl subclasses https://searchfox.org/mozilla-central/source/storage/mozIStorageValueArray.idl ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Avoiding jank in async functions/promises?
On 5/19/17 8:00 PM, Andreas Farre wrote: So if you have a look at how the idle callback algorithm is defined[1] and what timeRemaining is supposed to return[2] you see that timeRemaining doesn't update its sense of idleness, it only concerns itself with the deadline. So if you save the IdleDeadline object and resolve early, then timeRemaining won't know that the idle period entered upon calling the idle callback might have ended. Right - I was guessing something like that to be the case. I do think that you need to invert this somehow, actually doing the work inside a rIC callback. Something like[3]: let idleTask = { total: 10, progress: 0, doWork: async function(deadline) { The problem is that this is typically an unnatural way to express what is being done - particularly when attempting to address jank after the fact - and even more-so now that we have async functions, which making writing async code extremely natural and expressive. As I mentioned at the start of the thread, in one concrete example we had code already written that we identified being janky - http://searchfox.org/mozilla-central/rev/f55349994fdac101d121b11dac769f3f17fbec4b/toolkit/components/places/PlacesUtils.jsm#2022 Re-writing this code to work as you describe is certainly possible, but unlikely. So I'm still hoping there is something we can do to avoid jank without losing expressiveness and rewriting code identified as janky after the fact. Thanks, Mark ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Avoiding jank in async functions/promises?
On 05/18/2017 09:25 PM, Domenic Denicola wrote: On Thursday, May 18, 2017 at 4:34:37 AM UTC-4, smaug wrote: FWIW, I just yesterday suggested in #whatwg that the platform should have something like IdlePromise or AsyncPromise. And there is the related spec bug https://github.com/whatwg/html/issues/512#issuecomment-171498578 In my opinion there's no need for a whole new promise subclass, just make requestIdleCallback() return a promise when there's no argument passed. That wouldn't really catch my idea. I was hoping to have a whole Promise chain using async or idle scheduling. Running tons of Promises even at rIC time is still running tons of promises and possibly causing jank. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Avoiding jank in async functions/promises?
On Fri, May 19, 2017 at 4:18 AM, Mark Hammond wrote: > On 5/18/17 12:03 PM, Boris Zbarsky wrote: >> >> On 5/17/17 9:22 PM, Mark Hammond wrote: >>> >>> I'm wondering if there are any ideas about how to solve this optimally? >> >> >> I assume >> https://w3c.github.io/requestidlecallback/#the-requestidlecallback-method >> doesn't have quite the right semantics here? That would let you run when >> the browser is idle, and give you some idea of how long you can run for >> before you should yield. > > > I didn't quite expect this to work, but by abusing rIC I can almost make > something work - I just have the callback stash the IdleDeadline object and > return immediately, but continue to refer to it anyway. eg: > > let idleChecker = { > resolved: Promise.resolve(), > deadline: null, > promiseIdle() { > if (this.deadline && this.deadline.timeRemaining() > 0) { > return this.resolved; > } > this.deadline = null > return new Promise(resolve => { > window.requestIdleCallback(deadline => { > this.deadline = deadline; > resolve(); > }); > }) > } > } > > async function janky() { > let start = Date.now(); > for (let i = 0; i < 1000; i++) { > await Promise.resolve(); > await idleChecker.promiseIdle(); > } > console.log("took", Date.now() - start) > } > janky().then(() => console.log("done")); > > I 1/2 expect this to defeat the intent/implementation of rIC, but it does > work, causing ~2x-5x slowdown of the loop. I wonder if this is worth > experimenting with some more? > > Mark > So if you have a look at how the idle callback algorithm is defined[1] and what timeRemaining is supposed to return[2] you see that timeRemaining doesn't update its sense of idleness, it only concerns itself with the deadline. So if you save the IdleDeadline object and resolve early, then timeRemaining won't know that the idle period entered upon calling the idle callback might have ended. I do think that you need to invert this somehow, actually doing the work inside a rIC callback. Something like[3]: let idleTask = { total: 10, progress: 0, doWork: async function(deadline) { for (; this.progress < this.total && deadline.timeRemaining() > 0; ++this.progress) { await Promise.resolve(); } console.log(this.total - this.progress) }, schedule(deadline) { this.doWork(deadline).then(() => { if (this.progress < this.total) { requestIdleCallback(this.schedule.bind(this)); } else { this.resolve(); } }) }, run() { this.resolve = null; return new Promise(resolve => { this.resolve = resolve; requestIdleCallback(this.schedule.bind(this)); }) } } async function janky() { var start = Date.now(); await idleTask.run(); console.log("took", Date.now() - start) } var handle = setInterval(function (){ console.log("I shouldn't be blocked!")}, 500) janky().then(() => console.log("janky done")).then(() => clearTimeout(handle)); perhaps? The doWork function is indeed async, and executed within a loop that both handles how much work that should be done as well as using IdleDeadline.timeRemaining as expected. [1] https://w3c.github.io/requestidlecallback/#invoke-idle-callbacks-algorithm [2] https://w3c.github.io/requestidlecallback/#the-timeremaining-method Cheers, Andreas ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Avoiding jank in async functions/promises?
On 5/18/17 12:03 PM, Boris Zbarsky wrote: On 5/17/17 9:22 PM, Mark Hammond wrote: I'm wondering if there are any ideas about how to solve this optimally? I assume https://w3c.github.io/requestidlecallback/#the-requestidlecallback-method doesn't have quite the right semantics here? That would let you run when the browser is idle, and give you some idea of how long you can run for before you should yield. I didn't quite expect this to work, but by abusing rIC I can almost make something work - I just have the callback stash the IdleDeadline object and return immediately, but continue to refer to it anyway. eg: let idleChecker = { resolved: Promise.resolve(), deadline: null, promiseIdle() { if (this.deadline && this.deadline.timeRemaining() > 0) { return this.resolved; } this.deadline = null return new Promise(resolve => { window.requestIdleCallback(deadline => { this.deadline = deadline; resolve(); }); }) } } async function janky() { let start = Date.now(); for (let i = 0; i < 1000; i++) { await Promise.resolve(); await idleChecker.promiseIdle(); } console.log("took", Date.now() - start) } janky().then(() => console.log("done")); I 1/2 expect this to defeat the intent/implementation of rIC, but it does work, causing ~2x-5x slowdown of the loop. I wonder if this is worth experimenting with some more? Mark ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Avoiding jank in async functions/promises?
On Thursday, May 18, 2017 at 4:34:37 AM UTC-4, smaug wrote: > FWIW, I just yesterday suggested in #whatwg that the platform should have > something like IdlePromise or AsyncPromise. > And there is the related spec bug > https://github.com/whatwg/html/issues/512#issuecomment-171498578 In my opinion there's no need for a whole new promise subclass, just make requestIdleCallback() return a promise when there's no argument passed. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Avoiding jank in async functions/promises?
FWIW, I just yesterday suggested in #whatwg that the platform should have something like IdlePromise or AsyncPromise. And there is the related spec bug https://github.com/whatwg/html/issues/512#issuecomment-171498578 On 05/18/2017 04:22 AM, Mark Hammond wrote: Given our recent performance push, I thought I'd bring this topic up again. When writing loops using async functions in browser code, it's still very easy to cause jank. For example, a trivial function: async function janky() { for (let i = 0; i < 10; i++) { await Promise.resolve(); } } janky().then(() => console.log("done")); will cause the browser to hang. While that's not considered a bug in the implementation of the promise scheduler, and might not be particularly surprising in that trivial example, lots of real-world code can still hit this case - which both isn't obvious from casual inspection, and even if it was, doesn't have an obvious solution. Concretely, we struck this a couple of years ago in bug 1186714 - creating a backup of all bookmarks ends up looking alot like the loop above. In addition, the Sync team is moving away from nested event loops towards promises, and our work to date is hitting this problem. In bug 1186714, we solved the problem by inserting code into the loop that looks like: if (i % 50 == 0) { await new Promise(resolve => { Services.tm.dispatchToMainThread(resolve); }); } http://searchfox.org/mozilla-central/rev/f55349994fdac101d121b11dac769f3f17fbec4b/toolkit/components/places/PlacesUtils.jsm#2022 so we explicitly yield to the event loop every 50 iterations. However, this isn't optimal as the 50 is obviously a magic number, determined by experimentation on a couple of machines - when running on low spec hardware, this loop is almost certainly still janky. If we try and err on the side of caution (eg, yielding every iteration) we see the wall time of the loop take a massive hit (around twice as long in that bug). I'm wondering if there are any ideas about how to solve this optimally? Naively, it seems that the (broadest sense of the term) "platform" might be able to help here using it's knowledge of the event-loop state - eg, a function that indicates "are we about to starve the event loop and become janky?", or possibly even the whole hog (ie, "please yield to the event loop if you think now is a good time, otherwise just hand back a pre-resolved promise"). Or maybe there are simpler options I've overlooked? Thanks, Mark ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Avoiding jank in async functions/promises?
On Thu, May 18, 2017, at 10:10 AM, L. David Baron wrote: > On Thursday 2017-05-18 11:22 +1000, Mark Hammond wrote: > > I'm wondering if there are any ideas about how to solve this optimally? > > Naively, it seems that the (broadest sense of the term) "platform" might be > > able to help here using it's knowledge of the event-loop state - eg, a > > function that indicates "are we about to starve the event loop and become > > janky?", or possibly even the whole hog (ie, "please yield to the event loop > > if you think now is a good time, otherwise just hand back a pre-resolved > > promise"). > > One other option would be to use time rather than iterations as a > measure of when to return. > > A platform API that's somewhat like this is requestIdleCallback, > whose spec is at https://w3c.github.io/requestidlecallback/ -- in > particular, the timeRemaining() method on the IdleDeadline object > passed to the callback. See also: > https://developer.mozilla.org/en-US/docs/Web/API/Window/requestIdleCallback > https://developer.mozilla.org/en-US/docs/Web/API/Background_Tasks_API > > I'm not sure whether it works for privileged (chrome) JS, but it > seems like it ought to... The non-DOM version of rIC is being tracked by https://bugzilla.mozilla.org/show_bug.cgi?id=1353206 and https://bugzilla.mozilla.org/show_bug.cgi?id=1358476 Kanru ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Avoiding jank in async functions/promises?
On 5/18/17 12:19 PM, Ben Kelly wrote: On Wed, May 17, 2017 at 10:03 PM, Boris Zbarsky wrote: On 5/17/17 9:22 PM, Mark Hammond wrote: I'm wondering if there are any ideas about how to solve this optimally? I assume https://w3c.github.io/requestidlecallback/#the-requestidleca llback-method doesn't have quite the right semantics here? That would let you run when the browser is idle, and give you some idea of how long you can run for before you should yield. If rIC is not the right semantics, we could also set a time budget instead of a magic flat limit. Every N operations call performance.now() and check to see if the configured time exceeds the limit. FWIW, we have a similar problem in the native TimeoutManager::RunTImeout() method. I'm using a time budget approach to make it adapt to different hardware better. That sounds interesting and is almost certainly better than a hard-coded 50, but doesn't it have the same basic problem? That depending on what other things are going on, the magic time-limit chosen will either be too conservative (ie, sacrificing wall time) or too liberal (ie, still causing jank)? Thanks, Mark. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Avoiding jank in async functions/promises?
On 5/18/17 12:03 PM, Boris Zbarsky wrote: On 5/17/17 9:22 PM, Mark Hammond wrote: I'm wondering if there are any ideas about how to solve this optimally? I assume https://w3c.github.io/requestidlecallback/#the-requestidlecallback-method doesn't have quite the right semantics here? That would let you run when the browser is idle, and give you some idea of how long you can run for before you should yield. The only way I could see that work would be if the code was explicitly written as a consumer of a queue of promises (along the lines of that "Cooperative Scheduling of Background Tasks API" example). However, I can't see how code written "naturally" (eg, that loop in PlacesUtils.jsm I linked to) could leverage that. IdleDeadline.timeRemaining() does appear to have the semantics I want, but just seems "inverted" in terms of how it is used. A function that, basically, returns this value synchronously sounds like what I am after though. Thanks, Mark ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Avoiding jank in async functions/promises?
On Wed, May 17, 2017 at 10:19 PM, Ben Kelly wrote: > FWIW, we have a similar problem in the native TimeoutManager::RunTImeout() > method. I'm using a time budget approach to make it adapt to different > hardware better. > I meant to include the bug number here: https://bugzilla.mozilla.org/show_bug.cgi?id=1343912 ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Avoiding jank in async functions/promises?
On Wed, May 17, 2017 at 10:03 PM, Boris Zbarsky wrote: > On 5/17/17 9:22 PM, Mark Hammond wrote: > >> I'm wondering if there are any ideas about how to solve this optimally? >> > > I assume https://w3c.github.io/requestidlecallback/#the-requestidleca > llback-method doesn't have quite the right semantics here? That would > let you run when the browser is idle, and give you some idea of how long > you can run for before you should yield. > If rIC is not the right semantics, we could also set a time budget instead of a magic flat limit. Every N operations call performance.now() and check to see if the configured time exceeds the limit. FWIW, we have a similar problem in the native TimeoutManager::RunTImeout() method. I'm using a time budget approach to make it adapt to different hardware better. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Avoiding jank in async functions/promises?
On Thursday 2017-05-18 11:22 +1000, Mark Hammond wrote: > I'm wondering if there are any ideas about how to solve this optimally? > Naively, it seems that the (broadest sense of the term) "platform" might be > able to help here using it's knowledge of the event-loop state - eg, a > function that indicates "are we about to starve the event loop and become > janky?", or possibly even the whole hog (ie, "please yield to the event loop > if you think now is a good time, otherwise just hand back a pre-resolved > promise"). One other option would be to use time rather than iterations as a measure of when to return. A platform API that's somewhat like this is requestIdleCallback, whose spec is at https://w3c.github.io/requestidlecallback/ -- in particular, the timeRemaining() method on the IdleDeadline object passed to the callback. See also: https://developer.mozilla.org/en-US/docs/Web/API/Window/requestIdleCallback https://developer.mozilla.org/en-US/docs/Web/API/Background_Tasks_API I'm not sure whether it works for privileged (chrome) JS, but it seems like it ought to... -David -- 𝄞 L. David Baron http://dbaron.org/ 𝄂 𝄢 Mozilla https://www.mozilla.org/ 𝄂 Before I built a wall I'd ask to know What I was walling in or walling out, And to whom I was like to give offense. - Robert Frost, Mending Wall (1914) signature.asc Description: PGP signature ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Avoiding jank in async functions/promises?
On 5/17/17 9:22 PM, Mark Hammond wrote: I'm wondering if there are any ideas about how to solve this optimally? I assume https://w3c.github.io/requestidlecallback/#the-requestidlecallback-method doesn't have quite the right semantics here? That would let you run when the browser is idle, and give you some idea of how long you can run for before you should yield. -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Avoiding jank in async functions/promises?
Given our recent performance push, I thought I'd bring this topic up again. When writing loops using async functions in browser code, it's still very easy to cause jank. For example, a trivial function: > async function janky() { > for (let i = 0; i < 10; i++) { > await Promise.resolve(); > } > } > > janky().then(() => console.log("done")); will cause the browser to hang. While that's not considered a bug in the implementation of the promise scheduler, and might not be particularly surprising in that trivial example, lots of real-world code can still hit this case - which both isn't obvious from casual inspection, and even if it was, doesn't have an obvious solution. Concretely, we struck this a couple of years ago in bug 1186714 - creating a backup of all bookmarks ends up looking alot like the loop above. In addition, the Sync team is moving away from nested event loops towards promises, and our work to date is hitting this problem. In bug 1186714, we solved the problem by inserting code into the loop that looks like: >if (i % 50 == 0) { > await new Promise(resolve => { >Services.tm.dispatchToMainThread(resolve); > }); >} http://searchfox.org/mozilla-central/rev/f55349994fdac101d121b11dac769f3f17fbec4b/toolkit/components/places/PlacesUtils.jsm#2022 so we explicitly yield to the event loop every 50 iterations. However, this isn't optimal as the 50 is obviously a magic number, determined by experimentation on a couple of machines - when running on low spec hardware, this loop is almost certainly still janky. If we try and err on the side of caution (eg, yielding every iteration) we see the wall time of the loop take a massive hit (around twice as long in that bug). I'm wondering if there are any ideas about how to solve this optimally? Naively, it seems that the (broadest sense of the term) "platform" might be able to help here using it's knowledge of the event-loop state - eg, a function that indicates "are we about to starve the event loop and become janky?", or possibly even the whole hog (ie, "please yield to the event loop if you think now is a good time, otherwise just hand back a pre-resolved promise"). Or maybe there are simpler options I've overlooked? Thanks, Mark ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform