Skip to content

[css-layout-api] Generator vs. Promise design for the API #750

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

Open
bfgeek opened this issue Apr 9, 2018 · 5 comments
Open

[css-layout-api] Generator vs. Promise design for the API #750

bfgeek opened this issue Apr 9, 2018 · 5 comments

Comments

@bfgeek
Copy link
Contributor

bfgeek commented Apr 9, 2018

There isn't actually a large difference between the two APIs from just looking at the script, i.e.

  *layout(children) {
    const fragment = yield children[0].layoutNextFragment();
  }
  layout(children) {
    const fragment = await children[0].layoutNextFragment();
  }

There are various pros/cons of each of these.

Generator:

  • Pro: May be faster wrt. bindings.
  • Pro: More restrictive API, can't await on other APIs which might leak into the LWGS.
  • Con: Less obvious error handling to for web developers.
  • Con: Less obvious API as this pattern isn't common, and not many people use them (generators).

Promise:

  • Pro: More obvious error handling.
  • Pro: Common pattern for developers.
@css-meeting-bot
Copy link
Member

The Working Group just discussed Generator vs. Promise design for the API, and agreed to the following resolutions:

  • RESOLVED: We will continue adopting generators for layout functions.
The full IRC log of that discussion Topic: Generator vs. Promise design for the API
github: https://github.com//issues/750
iank_: From the author PoV...
Rossen: The only difference is you added a *
dbaron: You need an async in the second line.
iank_: True.
iank_: These are the pros and cons [futher down the issue]
iank_: If anyone else has something to add here, I'll come back to the next meeting with data.
Rossen: Are generators broadly impl?
iank_: Yes.
iank_: Everyone does generators, though they haven't been used like this before.
TabAtkins: We won't expose the functions you can't run.
iank_: We'll have to make sure in the future. We have to code with eyes open.
fremy: You like this better because you can only allow some things to be sent?
iank_: It makes a tighter API. Promises are more familiar.
fremy: Dev are familiar with converting in the background
dbaron: After thinking about this, I'm feeling like the way devs interface with the engine here feels better desc by generator and not promise.
dbaron: I think because promises have this tie to an event loop thing where promises are running async on this micro-task. You're in the middle of this thing in the layout engine and it needs to call you many times.
??: I agree you're generating the list
s/??/surma/
TabAtkins: You're not though. We can call you fresh.
surma: That's details. Conceptually it works as a generator.
surma: I'm wondering if there's a place where scheduling with rpomises may become and issue and it wouldn't wiht promises.
iank_: I think there's work in FF to work promises at the right time. For the syncronous engines we would all the layout engine, make sure task queue is exhausted, make sure we've got a promise.
surma: It makes more sense with generators even if it's 100% correct.
dbaron: I think one of the weird things about generator is it produces the last thing at the end. You're using yield and return syntax to do much deeper things.
TabAtkins: The yield at the top of the issue is not usefully yielding into the engine. It's saying do this layout and give me results. It's using a generator as an async iterator. It's possible, but not the preferred pattern.
TabAtkins: It off doing it's own thing. If it was synchronous we wouldn't have to do this.
surma: Both allow us to think about async and parallel layouts in the future?
iank_: Yeah.
Rossen: I'm hearing mostly people leaning word generators.
TabAtkins: No.
Rossen: Except TabAtkins.
iank_: We can also give two different versions and see what people expect
Rossen: TabAtkins would you object? Any objections?
TabAtkins: I have weak objection and would like wider review.
Rossen: Has there been review? TAG looked, did they say anything?
Rossen: I went through Travis's comments and they were high level.
dbaron: I don't remember this one.
iank_: I think notes from the TAG meeting are up.
[everyone hunts meeting notes from TAG]
dbaron: Minutes say "Travis: I have some review feedback to post to the issue."
dbaron: We came back to it the next day, though.
TAG minutes in https://github.com/w3ctag/meetings/blob/gh-pages/2018/04-tokyo/04-06-minutes.md
iank_: [skim-reads]
Rossen: There's a couple ways to get out. 1) We agree to adopt promise-based API 2) We stick with generators with a light objection from TabAtkins and then he can figure out if we should avoid it. If he comes back with reasons and we can change.
TabAtkins: I'll bother Dominic
Rossen: Sticking with Generators will force the discussion.
Rossen: Do we have people that object to promise-based APIs?
TabAtkins: Performance aspect
iank_: I want to do some benchmarking
iank_: I think layout will be particularly sensitive to bindings.
TabAtkins: My hope is we don't need to re-apply promises.
iank_: We might need special APIs.
iank_: I'll have everything hopefully done.
Rossen: Prop: We will continue adopting generators for layout functions. TabAtkins will followup
RESOLVED: We will continue adopting generators for layout functions.

@tabatkins
Copy link
Member

(Note that the resolution was explicitly meant to require discussion, not to shut down the debate and settle on one or the other.)

@dbaron
Copy link
Member

dbaron commented Apr 27, 2018

A colleague noted the relationship of what's being done conceptually here to coroutines, although I'm not sure if that yields anything useful. (I vaguely recall that Ian might have mentioned that at the face-to-face meeting as well, though I don't see it minuted.)

@tabatkins
Copy link
Member

Yeah, JS's iterator/yield and async/await are basically one-sided coroutines.

@css-meeting-bot
Copy link
Member

The Working Group just discussed Generator vs. Promise design for the API, and agreed to the following:

  • RESOLVED: use Promises
The full IRC log of that discussion Topic: Generator vs. Promise design for the API
github:
github: https://github.com//issues/750
iank_: Did some benchmarking
iank_: The Promise solution is slightly slower, but not that much slower
iank_: That makes me lean towards Promises solution
iank_: Advantage of gnerator is that it's faster, don't have to kick the ? every time
iank_: there's some overhead for doing that
iank_: However, I think that's acceptable from an author expectation perspective
iank_: If it was 30-40% overhead, that might be a concern
iank_: but turned out to be not as much of a problem
Rossen: What did you test?
iank_: Test was 100 custom layout elements
iank_: went about 6 layers deep, and had two child nodes at each level and 4 leaf nodes
iank_: tree structure, roughly 100 nodes
iank_: used our perf testing framework
iank_: I did a synchronous version of the API, didn't have any generators, promises, etc. Just exectued synchornously
iank_: That was about 650 runs per second
iank_: Our current impl was about 430 runs per second
iank_: so 50% off the synchronous one
iank_: Promises was 450 runs per second
iank_: Might be able to get it faster, push around 500 or something
iank_: We do lose a lot of perf by allowing async here
iank_: so something else to consider
iank_: One thing we could revisit later is, asnchornous layout engines
iank_: we could potentially get perfr by exposing synchronous versions of layout APIs
iank_: so leave that door open
iank_: I'll need to do some work, spec wise, to get it so that an impl can run this synchronously
iank_: Still requres a queue to go thorugh layout requests
iank_: Once I've got that written down, make sure it makes sense to everyone
frremy: What if you await Promise that never returns?
iank_: IWhen you call layoutNextFragment(), pushes a request onto an internal queue. If layout engine has exhausted that queue
iank_: It'll keep queuing its own tasks, flush that queue and echeck if resolved prmise is done
iank_: If ....
iank_: Summary of the engine is, layoutnextfragment), pushes onton internal queue for custom layotu instance. Layout engine will enqueue a microtask.
iank_: lay everything out and then queue tiself again in case extra work
iank_: if it finishes, then... if promis is resolved, then done
Rossen: So, resolution is to switch back to Promises
Rossen: Any additional comments or objections?
eae: Can we add a note about err-handlign expectations?
ACTION: iank to add note about error-handling expectations
Error finding 'iank'. You can review and register nicknames at .
RESOLVED: use Promises
(and add note)

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

No branches or pull requests

4 participants