Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Yieldy API Naming #95

Open
shaseley opened this issue Jul 3, 2024 · 19 comments
Open

Yieldy API Naming #95

shaseley opened this issue Jul 3, 2024 · 19 comments

Comments

@shaseley
Copy link
Collaborator

shaseley commented Jul 3, 2024

As pointed out in WebKit/standards-positions#361, scheduler.render() is inconsistently named with .wait() and .yield() because the method doesn't actually render.

Some additional concerns that came up in discussion:

  1. .wait() (as well as .waitForRender()) is a bit awkward with await, and that will probably be more common then .wait().then(...) (which does read well)
  2. .render() is ambiguous: is the promise resolved before, during, or after rendering?
  3. The APIs should use the same part of speech, per https://www.w3.org/TR/design-principles/#naming-consistency

Trying to alleviate all of these concerns is tricky, but here's an attempt:

async function task() {
  await scheduler.continuation();
  await scheduler.timeout(1000);
  await scheduler.nextFrame();
}

.wait(): .timeout() represents what you're waiting for, and it's in line with both setTimeout() and AbortSignal.timeout().

.yield(): If .wait() becomes .timeout(), this should be a noun too (per (3)). The promise returned by .yield() represents a continuation of the current task, and awaiting a continuation is yielding, so .continuation() seems natural. I'm a little worried that .continuation() will be less intuitive for developers than .yield(), but maybe that's a documentation problem?

.render(): This is tricky because "rendering" is both the relevant noun ("update the rendering") and verb ("render"), which makes await scheduler.rendering() awkward and ambiguous. I chose .nextFrame() because I think it aligns with the LoAF processing model: in LoAF, the frame timing is reset after the task if no documents need rendering, or after rendering if they do. That matches the the intended behavior of the promise resolution for this API.

@shaseley
Copy link
Collaborator Author

shaseley commented Jul 3, 2024

@noamr to keep me honest on the LoAF bits.

@mmocny
Copy link
Collaborator

mmocny commented Jul 4, 2024

This is a great summary, and I agree with your points carefully laid out--- yet I don't love the outcome.


+1 to scheduler.timeout() change, that seems great.


scheduler.nextFrame() I think is towards the right direction, but:

  • "frame" is ambiguous
  • the problem of "before, during, or after rendering?" I still think remains.

With the naming of LoAF it was explicitly prefixed with Animation Frame to differentiate all the different types of "frames" that might be associated by developers (it was initially called "Long Frames" and was renamed). Most developers will not think of "animation frames" first and foremost when seeing unqualified "frames" (though I only have a few anecdotes to form this opinion), even when in the context of scheduling it is the only one that makes sense.

nextAnimationFrame seems a logical direction. its wordy, but not more so than requestAnimationFrame and so maybe not so bad? It shouldn't be used by most developers much of the time, anyway.

The biggest issues is that I still think it doesn't solve the problem you highlighted: is it before, during, or after? I know what is being implied by next, but with the context of requestAnimationFrame api I still think the natural assumption is for "current". To add to confusion, a request for rAF from within a rAF callback DOES request the next frame.

What about something like scheduler.animationFrame().finished to match view transitions and animations apis? (it looks like we also use ended and complete property for some other apis...).


Finally, scheduler.continuation().

scheduler.yield() has a certain feel to it that really resonated with developers, I think. I'm just not getting that vibe with scheduler.continuation().

If the feedback was primarily about render() I wonder if yield should be left as is? Even if its not fully consistent, I think the ergonomics is justified. This might be a popular api for day to day usage, unlike render().

On the other hand, one other concern with yield() is the potential confusion with the yield keyword.

Ideas for alternatives:

  • scheduler.nextTick()
    • used by a lot of web frameworks to represent "scheduling after the underlying framework has a chance to reflect existing changes". I think it's a good match conceptually.
    • process.nextTick() from Node.js I think is also conceptually similar-- Has higher priority than setTimeout or setImmediate, but is not a microtask... however it seems it might actually get scheduled before microtasks and wont allow other work to get scheduled first...
  • scheduler.reschedule()
    • Gemini suggested this one. Its cute :)
  • scheduler.defer()
    • Both Gemini and ChatGPT thought this was a top option.
  • scheduler.pause()
    • ChatGPT suggested this one. I think it's simple but isn't implied that the goal is to "resume"...
  • scheduler.break()
    • Another GPT suggestion, but might be confused with debugging?

(For what it's worth, various GPTs both stack ranked scheduler.continuation() very low, sometimes dead last, while yield was high, sometimes the top spot)

@noamr
Copy link

noamr commented Jul 4, 2024

I also liked yield, but I didn't like that it collided with the generator yield thingy. And continuation feels confusing and long...

How about:

  • await scheduler.timeout(1000) (great!),
  • await scheduler.animationFrame(),
  • await scheduler.tick()

@shaseley
Copy link
Collaborator Author

shaseley commented Jul 8, 2024

Yield:

scheduler.yield() has a certain feel to it that really resonated with developers, I think. I'm just not getting that vibe with scheduler.continuation().

And continuation feels confusing and long...

+1. I thought alternatives were worth exploring, but I think scheduler.yield() is a much better name. I think "yield-to-the-event-loop" is a familiar concept (or at least it's becoming one!), making the intent at the callsite clear, whereas "continuation" is appropriate, but not well known.

On the other hand, one other concern with yield() is the potential confusion with the yield keyword.

I also liked yield, but I didn't like that it collided with the generator yield thingy.

I had considered this a while back, and:

  • I think it's fine since it's a method on self.scheduler; I would have been more worried about a bare await yield()
  • It's the best match for the concept/intent, compared to the alternatives
  • This didn't come up as a concern up with developers (e.g. during OT), at least not that I'm aware of

The "tick" idea is interesting, but I'd be wary of introducing that concept into HTML since I'm not sure how well it maps to HTML concepts. For example:

  • Does "after the underlying framework has a chance to reflect existing changes" imply after rendering in HTML (which isn't guaranteed by .yield())?
  • How does a tick relate to an HTML task? A "tick" in HTML sounds like it could be a turn of the event loop, but then wouldn't that be ~= "next task" (and then would .nextTask() be a better fit for HTML)?

Wait/timeout:

+1 to scheduler.timeout() change, that seems great.

I don't mind scheduler.wait() personally. I agree it's a bit awkward with await, but being a scheduler method helps. .timeout() is nicer (with await, not with .then()), but if there's concern about (part of speech) consistency, I'd much rather keep .yield() and live with .wait().

Render:

What about folding this into scheduler.yield() rather than having a separate method, e.g. scheduler.yield({afterRender: true})?

  • The intent is largely the same: yield to higher priority work and resume ASAP
    • afterRender --> imminent rendering is high priority
    • can be thought of as a fine-grained yield optimization, especially where the UA doesn't have enough info to make that decision
  • The continuation prioritization works the same, regardless of afterRender
  • If rendering isn't expected after the task ends (no documents need it), afterRender is effectively ignored

OTOH, I'd rather keep .wait() (or .timeout()) separate:

  • The timeout also constrains the continuation, but the intent is different --> run ASAP (% constraint) vs. run at a specific point in the future
    • the timeout is the point vs. being a yield optimization
  • The plan is for .wait() continuations to be prioritized like tasks rather than .yield() continuations, so .wait(0) != .yield({timeout: 0}).
  • scheduler.wait(X) is a much more ergonomic shorthand for a promise-based setTimeout() compared to a .yield() option

What about something like scheduler.animationFrame().finished to match view transitions and animations apis? (it looks like we also use ended and complete property for some other apis...).

What other properties would .animationFrame() have? I could imagine this as part of a more comprehensive frame lifecycle API, but I wonder if that's a little far beyond the scope of what we need for yielding purposes?


To summarize, what about:

  • await scheduler.yield()
  • await scheduler.yield({afterRender: true})
  • await scheduler.wait(1000) or await scheduler.timeout(1000), depending on consistency concerns

Thoughts?

@noamr
Copy link

noamr commented Jul 9, 2024

I like this direction. Though are we OK with afterRender not having feature detection? As in, browsers who don't support it would fold into normal scheduler.yield(). I think it makes some sense because it only yields to an existing scheduled render and doesn't wait for an animation frame.

@mmocny
Copy link
Collaborator

mmocny commented Jul 9, 2024

What other properties would .animationFrame() have

The main one I was expecting was a property to represent requestAnimationFrame() equivalent (i.e. perhaps called .ready to also match Web Animations API).

wonder if that's a little far beyond the scope of what we need for yielding purposes

Fair enough-- it's not strictly needed for scheduling, but developers seem to desire a promise based rAF() alongside promise based setTimeout() and I thought... give the people what they want ;)


I think scheduler.yield({afterRender: true}) has the advantages you mention, but like with continuation(), also loses a certain ergonomics. I'm less concerned about it, since it should either be used sparingly (in very specific use cases)-- or more likely just gets integrated and wrapped by framework specific utilities (unlike plain yield).

Earlier in the thread you suggested the name waitForRender instead of afterRender. I think that's a bit clearer, given how this shook out.

await scheduler.yield({ waitForRender: true })


Question: if I had timeout: property on yield, and after/waitForRender:, together? Would it be BOTH or EITHER? We've seen polyfills that schedule after-render UNLESS 100ms pass, then fallback to timer. Either way, you can trivially build that by racing promises with wait().

(I also wonder when the continuation would get scheduled if e.g. tab goes to background and rendering is throttled, which is a problem with rAF based scheduling)

@shaseley
Copy link
Collaborator Author

shaseley commented Jul 9, 2024

I like this direction. Though are we OK with afterRender not having feature detection? As in, browsers who don't support it would fold into normal scheduler.yield(). I think it makes some sense because it only yields to an existing scheduled render and doesn't wait for an animation frame.

Maybe a non-boolean parameter would be easier to feature detect, e.g. passing an invalid enum value should throw? I like the direction of your offline suggestion of {rendering: 'eagar'}, or something like that. I was going down that path too, but couldn't think of a good name 🙃.

I think scheduler.yield({afterRender: true}) has the advantages you mention, but like with continuation(), also loses a certain ergonomics. I'm less concerned about it, since it should either be used sparingly (in very specific use cases)-- or more likely just gets integrated and wrapped by framework specific utilities (unlike plain yield).

Yep, that's what I was thinking. I think the pros outweigh the cons, and it's easy enough to wrap in a waitForRenderingIfNeeded() method.

Earlier in the thread you suggested the name waitForRender instead of afterRender. I think that's a bit clearer, given how this shook out.

await scheduler.yield({ waitForRender: true })

Question: if I had timeout: property on yield, and after/waitForRender:, together? Would it be BOTH or EITHER? We've seen polyfills that schedule after-render UNLESS 100ms pass, then fallback to timer. Either way, you can trivially build that by racing promises with wait().

Yeah you could use Promise.all() or Promise.race() to get the desired behavior. IF we had both parameters (which I'm not suggesting), I'm not actually sure what the behavior should be (which is maybe another argument for keeping them as separate methods).

(I also wonder when the continuation would get scheduled if e.g. tab goes to background and rendering is throttled, which is a problem with rAF based scheduling)

The plan is to ensure pending yield() blocked on rendering can run in the background. Two cases:

  1. Calling yield({placeholder-rendering-option} in the background will ignore the argument
  2. When a document is backgrounded, any pending promises blocked on rendering associated with that document's scheduler are unblocked (the pending continuation tasks are allowed to run).

I wonder if this behavior would obviate the need for doing the scheduler after-render unless 100 ms thing?

@ghost
Copy link

ghost commented Aug 12, 2024

Another possibility for timeouts: dwell. It comes from electronics.

The purpose of the dwell-time gate is to provide a time delay, or dwell time, between the leading edge of the input and trailing edge of the output.

I think it works for both time and the rendered condition.

await scheduler.yield() // I want to run now but I'm politely yielding
await scheduler.dwell(1_000) // Let me dwell here for 1 second
await scheduler.dwell('rendered') // Let me dwell here until the next render is done

They're both imperative verbs. They're similarly short. It's unfamiliar but I think the meaning is sufficiently clear that devs will understand it easily. It also gives a nice place for future expansion to new "dwell until" conditions, a desire mentioned in other threads.

Some definitions from wiktionary.

  1. (engineering) A period of time in which a system or component remains in a given state.
  2. (engineering) A brief pause in the motion of part of a mechanism to allow an operation to be completed.
  3. (electrical engineering) A planned delay in a timed control program.

@mmocny
Copy link
Collaborator

mmocny commented Aug 12, 2024

I'm not sure I love dwell but it might just be my lack of familiarity with that term.

However, I did want to point out that we didn't consider the yield({placeholder-rendering-option} to instead of wait({placeholder-rendering-option}) and I think thats quite an interesting suggestion to evaluate.

Maybe any feature that is okay to delay for rendering is also okay to continue being scheduled at non-continuation priority?

@mmocny
Copy link
Collaborator

mmocny commented Aug 12, 2024

I forgot to say: I'll dwell on it for a bit :)

@martinthomson
Copy link

My personal perspective here is that scheduler.yield() and scheduler.wait(t) are fine. These are nice, short verbs with a direct line on what happens. Anne points out that in his comment and I agree.

If anything, a single scheduler.wait() with an optional argument that describes what you wait for is the only improvement I can see for those two:

await scheduler.wait(); // Until the next available task.
await scheduler.wait(100); // Wait for at least 100 milliseconds.
await scheduler.wait({ time: 100 }); // ... a more verbose version of the previous.

I have real problems with scheduler.render(). You could see how a generic wait() might work for render() as well:

await scheduler.wait({animationFrame: true}); // Wait for the next animation frame.
await scheduler.wait({waitForRender: true}); // or this, but why does it say wait twice?
await scheduler.wait({render: 'end'}) // or this.

But this could get a little out of hand.

// Wait for the next holiday... to commence? ...or is it end?
await scheduler.wait({christmas: true});
// Hmm, is this like Promise.all() or Promise.any()?
await scheduler.wait({christmas: true, easter: true});

To avoid that explosion, I think you have to regard the scheduler is a useful primitive that underlies a bunch of other stuff that the platform does. Though all events and callbacks logically need to pass through the scheduler, the scheduler itself is rarely the direct target of calls.

await scheduler.wait({fetch: fetch("https://example.com")}); // This is madness.
await fetch("https://example.com"); // This is how most APIs will be used instead.

So while there might be a few core things that are exposed directly, most stuff that uses the scheduler won't need to involve the globalThis.scheduler instance.

So, when it comes to render(), it's tough. We have this thing attached to a window, which leads to a global function that doesn't really make a huge amount of sense in its naming or placement. But it's not really its own thing. It's another floating function that isn't bound to any deeper concept. Adding a new function for that attached to scheduler isn't that much better unless you consider the scheduling of rendering to be a central function of the scheduler. Though it is as a practical matter, is that an implementation detail that you want to continuously remind developers of? My problem is that there isn't an obvious place to put that sort of logic, other than the window itself.

I'm wondering whether a fix for the rendering piece could be deferred. requestAnimationFrame().then(_ => scheduler.yield()) isn't so horrible, is it?

p.s., I prefer wait() over yield() for the more generic version. Having yield() as an alias for wait(0) is fine. It pairs with await nicely. My understanding of the word "yield" is less open-ended, even if it is really "I yield to X" where X == a generic "others" by default. But that's a colouring/coloring issue that doesn't much matter.

@noamr
Copy link

noamr commented Sep 4, 2024

I have real problems with scheduler.render(). You could see how a generic wait() might work for render() as well:

await scheduler.wait({animationFrame: true}); // Wait for the next animation frame.
await scheduler.wait({waitForRender: true}); // or this, but why does it say wait twice?
await scheduler.wait({render: 'end'}) // or this.

I'm not a big fan of these boolean dictionaries, perhaps

await scheduler.wait({for: "render"})

?

@martinthomson
Copy link

@noamr, You'll note that my comment goes on a little from that point, where I used those examples to illustrate how things were not necessarily headed in a good direction.

Also, isn't "for" a reserved word in JS? I'd hate to have to quote it all the time.

@noamr
Copy link

noamr commented Sep 4, 2024

@noamr, You'll note that my comment goes on a little from that point, where I used those examples to illustrate how things were not necessarily headed in a good direction.

Fair enough, I was bikeshedding instead of responding to the point.

The thing with requestAnimationFrame(() => {yield...}) is that it forces an animation frame, which is a different behavior from waiting until the next frame renders naturally.

Reading what you said, I'm not sure the scheduler namespace adds value at all, it's a bit of an implementation detail as you say. Also scheduler.wait(timeout) feels more like a promise-based setTimeout than something low-level.

In addition, not sure if scheduler makes sense in workers or in node/deno, while promise-based timeouts certainly do.
Since render definitely belongs only in the window, I'm not sure that all these things belong in the same namespace or in a namespace at all.
@shaseley have you discussed this with WinterCG at all? Perhaps we should?

Perhaps we could have this as independent promise getters in the global object, where render is only exposed on Window:

await globalThis.afterCurrentTask;
await globalThis.timeout(3000);
await window.afterNextAnimationFrame;

Also, isn't "for" a reserved word in JS? I'd hate to have to quote it all the time.

Can be wait({until: "render"}), but yea it's a bikeshed :)

@annevk
Copy link

annevk commented Sep 4, 2024

Pretty sure reserved keywords don't impact what you can write there.

@smaug----
Copy link

Why would render() belong to window only?

DedicatedWorkerGlobalScope includes AnimationFrameProvider;

@noamr
Copy link

noamr commented Sep 4, 2024

Why would render() belong to window only?

DedicatedWorkerGlobalScope includes AnimationFrameProvider;

Right! Forgot about offscreen canvas.

@ghost
Copy link

ghost commented Sep 4, 2024

What if it were just a string?

await scheduler.wait('rendered')

Then you have numbers for a delay, and strings that name a handful of core conditions.

await scheduler.wait(500) // Wait half a second
await scheduler.wait('frame') // Wait until the next frame, without forcing one
await scheduler.wait('rendered') // Wait until next render is done

In addition, not sure if scheduler makes sense in workers or in node/deno, while promise-based timeouts certainly do.

Re node, it's something I'd like to use. My server that runs tasks has things with different priorities that should be scheduled differently. Sometimes priorities change and the scheduling should follow that. I want a big website spider that will run for several weeks to be background while an analysis I'm actively waiting on should be user-blocking / high priority.

Perhaps we could have this as independent promise getters in the global object, where render is only exposed on Window:

await globalThis.afterCurrentTask;
await globalThis.timeout(3000);
await window.afterNextAnimationFrame;

Current ethos seems to hate this pattern, but I love it. It feels good to use generated promises. But, wait() is envisioned to take a priority / signal, so it has to be a method.

Still, it is so tempting. It would look like this with current signal propagation.

// Adopts current priority
// Follows priority changes of requesting task
await scheduler.frame // Wait until next frame
await scheduler.rendered // Wait until next render is done

@shaseley
Copy link
Collaborator Author

shaseley commented Sep 5, 2024

My personal perspective here is that scheduler.yield() and scheduler.wait(t) are fine. These are nice, short verbs with a direct line on what happens. Anne points out that in his comment and I agree.

If anything, a single scheduler.wait() with an optional argument that describes what you wait for is the only improvement I can see for those two:

scheduler.yield() and scheduler.wait(0) continuation tasks are likely to have different underlying scheduling, at least as currently envisioned. scheduler.yield() is meant to yield to "higher priority work", which helps resolve developer concerns about regaining control of the thread while still enabling responsiveness improvements (if UAs prioritize input, but this is implementation-defined). So this is different than a promise-based setTimeout(), which is approximately what scheduler.wait() would be. This is the main reason I'd prefer to keep them separate, with a secondary reason of making .wait() less verbose (if using a dictionary to future-proof, especially given we may add priority/signal parameters).

I have real problems with scheduler.render(). You could see how a generic wait() might work for render() as well:

await scheduler.wait({animationFrame: true}); // Wait for the next animation frame.
await scheduler.wait({waitForRender: true}); // or this, but why does it say wait twice?
await scheduler.wait({render: 'end'}) // or this.

But this could get a little out of hand.

// Wait for the next holiday... to commence? ...or is it end?
await scheduler.wait({christmas: true});
// Hmm, is this like Promise.all() or Promise.any()?
await scheduler.wait({christmas: true, easter: true});

Or it could take a single, non-optional argument that could be either a number or enum, and the promises could be combined with Promise.all() or Promise.any()? At one point I was thinking about pursuing this direction, e.g. scheduler.wait('load') might be nice, but doing something via events or observables would probably be better (if needed).

I'm wondering whether a fix for the rendering piece could be deferred. requestAnimationFrame().then(_ => scheduler.yield()) isn't so horrible, is it?

The main differences are that a built-in API wouldn't need to schedule rendering (as @noamr mentioned), so there's a potential performance benefit from not having to wait and not running the rendering machinery if not needed. And it could take into consideration visibility (current and changing), which a polyfill could as well, but it makes it more complex.

Aside from the "yield and don't block rendering" use case, we're also interested in exploring a way to indicate that "rendering now would be ideal." How browsers schedule and yield-from-tasks for rendering is complex, and providing a signal could help augment heuristics (e.g. parsing). For example, some sites have a good idea of where above-the-fold content ends, and inserting a signal indicating a request for rendering could be helpful in optimizing page load. I'm interested in exploring if this would be beneficial, although I'm not sure what the best way to expose this is (the imperative scheduler.render() seemed to match the intent). I was thinking to start with supporting both use cases at least for experimentation to prove this out, but potentially splitting it out depending on feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants