Closed wlandau closed 6 months ago
The callback may cause overhead if it happens on every single task…
I wonder if this just reflects the disconnect between the multi-task approach of crew vs the single-task approach of promises and Shiny. In the two coin flip examples in the Shiny crew vignette, the first version has less overhead because it polls gently and collects tasks in bulk.
{targets} does inevitably operate on single-task collection, but it is event-driven because it waits on condition variables.
I will have to think more about whether %…>% callbacks on mirai tasks are really a wise choice for crew…
On the other hand, check_all_controller_promises() might not actually be very expensive. It should just resolve the single-task promises and check the condition for multi-task ones.
On further reflection, crew
can entirely rely on mirai
promises and Shiny extended tasks for everything you would want to do in an app. The extended task can take a mirai
object, but then crew
can pop the task that actually finishes. The generative art app is really snappy if I implement it that way.
The upshot of all this is I can deprecate all the native promises functionality in crew
and the apps will still work great.
@shikokuchuo, it turns out that crew
can now rely on mirai
for all promise-related functionality. The new Shiny vignette in https://github.com/wlandau/crew/pull/164 shows how this works. I will merge https://github.com/wlandau/crew/pull/164 as soon as the next mirai
/nanonext
releases come out.
Promises in development
mirai
are now truly event-driven. Thanks @shikokuchuo and the work inspired by https://github.com/shikokuchuo/nanonext/pull/28, the callbacks for promises plug directly into the event loop of R itself. No more polling for results every 0.1 seconds.crew
could use this to make its own promises truly event-driven as well. Incontroller$push()
andcontroller$shove()
, instead of calling plainmirai::mirai()
, the controller could call something likemirai::mirai() %...>% check_all_controller_promises()
, where the controller keeps an actively maintained list of the unresolved promises it creates.Depending on the overhead this creates (need to benchmark and profile on workloads with 10000+ tasks) I may need to make support for promises optional, either when the controller is created or on
push()
. The latter is back-compatible with all plugins, so it is my current preference.I will wait until after the next
crew
release to begin work on this. Maybe also until the next versions ofnanonext
andmirai
get closer to their own release dates.