w3c / css-houdini-drafts

Mirror of https://hg.css-houdini.org/drafts
https://drafts.css-houdini.org/
Other
1.84k stars 141 forks source link

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

Open bfgeek opened 6 years ago

bfgeek commented 6 years ago

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:

Promise:

css-meeting-bot commented 6 years ago

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

The full IRC log of that discussion <dael> Topic: Generator vs. Promise design for the API
<dael> github: https://github.com/w3c/css-houdini-drafts/issues/750
<dael> iank_: From the author PoV...
<dael> Rossen: The only difference is you added a *
<dael> dbaron: You need an async in the second line.
<dael> iank_: True.
<dael> iank_: These are the pros and cons [futher down the issue]
<dael> iank_: If anyone else has something to add here, I'll come back to the next meeting with data.
<dael> Rossen: Are generators broadly impl?
<dael> iank_: Yes.
<dael> iank_: Everyone does generators, though they haven't been used like this before.
<dael> TabAtkins: We won't expose the functions you can't run.
<dael> iank_: We'll have to make sure in the future. We have to code with eyes open.
<dael> fremy: You like this better because you can only allow some things to be sent?
<dael> iank_: It makes a tighter API. Promises are more familiar.
<dael> fremy: Dev are familiar with converting in the background
<dael> 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.
<dael> 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.
<dael> ??: I agree you're generating the list
<gsnedders> s/??/surma/
<dael> TabAtkins: You're not though. We can call you fresh.
<dael> surma: That's details. Conceptually it works as a generator.
<dael> surma: I'm wondering if there's a place where scheduling with rpomises may become and issue and it wouldn't wiht promises.
<dael> 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.
<dael> surma: It makes more sense with generators even if it's 100% correct.
<dael> 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.
<dael> 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.
<dael> TabAtkins: It off doing it's own thing. If it was synchronous we wouldn't have to do this.
<dael> surma: Both allow us to think about async and parallel layouts in the future?
<dael> iank_: Yeah.
<dael> Rossen: I'm hearing mostly people leaning word generators.
<dael> TabAtkins: No.
<dael> Rossen: Except TabAtkins.
<dael> iank_: We can also give two different versions and see what people expect
<dael> Rossen: TabAtkins would you object? Any objections?
<dael> TabAtkins: I have weak objection and would like wider review.
<dael> Rossen: Has there been review? TAG looked, did they say anything?
<dael> Rossen: I went through Travis's comments and they were high level.
<dael> dbaron: I don't remember this one.
<dael> iank_: I think notes from the TAG meeting are up.
<dael> [everyone hunts meeting notes from TAG]
<dael> dbaron: Minutes say "Travis: I have some review feedback to post to the issue."
<dael> dbaron: We came back to it the next day, though.
<dbaron> TAG minutes in https://github.com/w3ctag/meetings/blob/gh-pages/2018/04-tokyo/04-06-minutes.md
<dael> iank_: [skim-reads]
<dael> 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.
<dael> TabAtkins: I'll bother Dominic
<dael> Rossen: Sticking with Generators will force the discussion.
<dael> Rossen: Do we have people that object to promise-based APIs?
<dael> TabAtkins: Performance aspect
<dael> iank_: I want to do some benchmarking
<dael> iank_: I think layout will be particularly sensitive to bindings.
<dael> TabAtkins: My hope is we don't need to re-apply promises.
<dael> iank_: We might need special APIs.
<dael> iank_: I'll have everything hopefully done.
<dael> Rossen: Prop: We will continue adopting generators for layout functions. TabAtkins will followup
<dael> RESOLVED: We will continue adopting generators for layout functions.
tabatkins commented 6 years ago

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

dbaron commented 6 years ago

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 commented 6 years ago

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

css-meeting-bot commented 6 years ago

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

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