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

[css-layout-api] Design to allow for more layout engine optimizations. #898

Open
bfgeek opened this issue Jun 7, 2019 · 4 comments
Open

Comments

@bfgeek
Copy link
Contributor

bfgeek commented Jun 7, 2019

Layout engines currently have various optimizations which apply to inputs to layout. E.g.

<div id="parent" style="height: 100px;">
  <div>
      <div id="child1" style="height: 50px;"></div>
  </div>
</div>

<div id="parent2" style="height: 100px;">
  <div>
    <div id="child2" style="height: 50%;"></div>
  </div>
</div>

If the parents in the above example change from 100px to 200px only the second case has to perform layout on its immediate child (due to its percentage height descendant).

We currently have the inputProperties and childInputProperties for the style related inputs, however we don't have any "filter" for the layout constraints. E.g.

interface LayoutConstraints {
    readonly attribute double availableInlineSize;
    readonly attribute double availableBlockSize;

    readonly attribute double? fixedInlineSize;
    readonly attribute double? fixedBlockSize;

    readonly attribute double percentageInlineSize;
    readonly attribute double percentageBlockSize;
   // snip
}

For example if the layout doesn't care about the percentageBlockSize we'll still trigger layout if this changes. There are also valid usecases for ignoring the other inputs.

There are two different possible approaches for allowing these optimizations.

  1. Passing the used params on the output fragment. E.g.
registerLayout('param-filter', class {
  async layout() {
    return {
       children: [],
       autoBlockSize: {},
       usedConstraints: {
         availableInlineSize: true,
         percentageBlockSize: true,
       }
    };
  }
});

In the above example the engine is allowed to re-use the result if the any constraints change except for the availableInlineSize, and the percentageBlockSize.

This is forwards compatible for other layout constraints are added over time.

  1. Declaring up front:
registerLayout('upfront-filter', class {
  static inputConstraints = {
    availableInlineSize: true,
    percentageBlockSize: true,
   };
});

There are various pros/cons of each approach.

  • (1) Is more consistent with the rest of the API surface.
  • (2) Allows for more fine grained optimizations. E.g. the layout may switch modes and start depending on a different set of constraints.
@emilio
Copy link
Contributor

emilio commented Jun 7, 2019

Would it be possible to, instead of making the constraints a regular object, make it a WebIDL interface? If so, the engine can implement getters and track which parts of the constraints are used. This would be totally transparent to authors and would involve no API changes.

@emilio
Copy link
Contributor

emilio commented Jun 7, 2019

Oh, it's already a WebIDL interface. Then I don't know why the engine wouldn't be able to track which constraints are used during the algorithm.

Edit: Ah, though I guess it's more tricky, since you could have something like if (input.availableInlineSize < something) { use other part of the input }. So never mind me.

@css-meeting-bot
Copy link
Member

css-meeting-bot commented Jun 7, 2019

The Houdini Task Force just discussed Design to allow more layout engine optimizations, and agreed to the following:

  • RESOLVED: no change, engines are allowed to do analysis to optimize the invalidation logic; add a note about this
  • RESOLVED: Republish Layout API once Ian makes the edits from previous resolution.
The full IRC log of that discussion <TabAtkins> Topic: Design to allow more layout engine optimizations
<astearns> github: https://github.com//issues/898
<TabAtkins> iank_: Layout engines have a lot of optimizations for the inputs to layout
<TabAtkins> iank_: Gave one example in the issue that I think all UAs have today
<TabAtkins> iank_: % height descendant, if your height changes you have to lay out that subtree; if your height doesn't change you don't need to
<TabAtkins> iank_: While performing optiizations for layoutng, I realized it owuld be good for the layout api to get those optimizations too
<TabAtkins> iank_: We currently pass the layout() a bunch of information
<TabAtkins> iank_: It would be nice for the webdev to let us know which of these they actually use when generating the fragment
<TabAtkins> iank_: So if they only use the available sizes, and a % size changes, we don't need to re-call them.
<fremy> q+
<TabAtkins> iank_: Today we can't optimize that
<TabAtkins> iank_: emilio this morning had the same idea I had; could we just figure out when a dev reads a particular prop off the constraints?
<fremy> q- emilio's question is exactly what I was going to ask
<fremy> q0
<fremy> q-
<TabAtkins> iank_: Turns out not good enough. So we do need explicit dependencies expressed.
<TabAtkins> AmeliaBR: They might check that property, but then switch and read a different property for acutal use.
<TabAtkins> iank_: Right. You might read availableInlineSize, then switch to using % size if it's small enough.
<TabAtkins> dbaron: But you do still have a dependency then
<TabAtkins> emilio: I commented about that, yeah.
<TabAtkins> emilio: I thought the engine could be smart, but that's not enough. Yo uneed an allowlist, as your might branch off of something else to see which one you need.
<fremy> +1 to what dbaron just said
<TabAtkins> dbaron: If you have `if(A) { if(B) {...}}`, if you take the else, you ddon't have a dependency on B. You'll rerun layout when *A* changes, at which point you might be checking B, but before then you won't need to invalidate based on B
<TabAtkins> myles: The story dbaron just told sounds scary...
<TabAtkins> dbaron: I think it's less scary than making authors do it themselves
<TabAtkins> dbaron: If authors do it themselves, they'll make the same mistake Ian did, and not mark dependencies (and thus get broken layouts)
<TabAtkins> AmeliaBR: But that list of dependencies can change each time.
<Rossen_> q?
<TabAtkins> emilio: If we have an allowlist I"d like to not expose the values that aren't allowed. But if we're just doing dependencies, I'm fine with all.
<TabAtkins> proposed resolution: no change, engines are allowed to do analysis to optimize the invalidation logic
<TabAtkins> RESOLVED: no change, engines are allowed to do analysis to optimize the invalidation logic
<TabAtkins> dbaron: I think there should be a note in the spec about this
<TabAtkins> iank_: Definitely
<TabAtkins> Rossen_: Ian, can you repub this?
<TabAtkins> iank_: Yes.
<TabAtkins> RESOLVED: Republish Layout API once Ian makes the edits from previous resolution.

@tabatkins
Copy link
Member

@bfgeek I couldn't figure out where we talk about invocation and invalidation in the spec, so I couldn't figure out where to place this note. Mind finding that spot? ^_^

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