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

Define processing algorithm #4

Closed
igrigorik opened this issue May 11, 2017 · 4 comments
Closed

Define processing algorithm #4

igrigorik opened this issue May 11, 2017 · 4 comments

Comments

@igrigorik
Copy link
Member

From an email thread with @domenic...

Here's how I would do it.

In the paint timing spec, define an algorithm "report paint timing" which takes a Document. HTML will call this during step 7.12 of its "update the rendering" step, something like

For each fully active Document in docs, update the rendering or user interface of that Document and its browsing context to reflect the current state, and report paint timing for that Document while doing so.

(We'll modify HTML via a pull request.)

Then the contents of the "report paint timing" algorithm contains most of https://github.com/WICG/paint-timing#definition, adapted to algorithm form. To adapt to algorithm form, you'll want to do things like storing a state ("no paint timing sent", "first-paint sent", "first-contentful-paint sent") which you check. And it will need to use the appropriate invocations around firing a performance entry. Otherwise the existing text there is pretty good, with regard to trying to be precise about the timing.

@domenic
Copy link

domenic commented Jul 13, 2017

I realized that the processing model in the spec right now (even after my polish advisements in #15) doesn't quite work. Right now it effectively calls the "report paint timing" algorithm during every "update the rendering", but that's bad, because it queues a new entry every frame.

Let me propose the following refactoring on top of #15:

  • A new algorithm, "report paint timing when appropriate", is invoked every "update the rendering". It accepts a timestamp.
    • If this instance of update the rendering is the first paint, then it calls "report paint timing" with argument "first-paint" and the timestamp.
    • If this instance of update the rendering is the first contentful paint, then it calls "report paint timing" with argument "first-contentful-paint" and the timestamp.
    • (Otherwise it does nothing.)

@spanicker
Copy link
Contributor

Updated with a few tweaks:

  • new algorithm is called "mark paint timing" instead of "report paint timing when appropriate"
  • the timestamp is taken inside of "mark paint timing" as opposed to being passed in (this is more efficient for implementation)
    PTAL.

@domenic
Copy link

domenic commented Jul 18, 2017

Looks good apart from the one question about timestamps I mentioned in https://github.com/WICG/paint-timing/issues/15#issuecomment-316204404. Maybe we should close this issue and move over there.

@spanicker
Copy link
Contributor

sounds good.

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

3 participants