Suggesting to enable paint timing by default

classic Classic list List threaded Threaded
12 messages Options
Reply | Threaded
Open this post in threaded view
|

Suggesting to enable paint timing by default

Noam Rosenthal
Following the discussion with Simon in https://bugs.webkit.org/show_bug.cgi?id=78011

Since we have a pretty stable spec (https://w3c.github.io/paint-timing/), lots of passing web platform tests, other browser vendor support and a working implementation of first contentful paint, I am planning to submit a patch to enable paint timing by default. 

Any objections? Other thoughts?

_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|

Re: Suggesting to enable paint timing by default

Maciej Stachowiak

I noticed from comments in one of the Radars that the patch may result in an additional “fake paint”, so it should probably be performance tested. Have you done any testing? We’ll likely want to A/B some of Apple’s page load speed benchmarks.

I’d like to hear from others on maturity of the spec and readiness of the code.

 - Maciej

On May 11, 2020, at 11:44 AM, Noam Rosenthal <[hidden email]> wrote:

Following the discussion with Simon in https://bugs.webkit.org/show_bug.cgi?id=78011

Since we have a pretty stable spec (https://w3c.github.io/paint-timing/), lots of passing web platform tests, other browser vendor support and a working implementation of first contentful paint, I am planning to submit a patch to enable paint timing by default. 

Any objections? Other thoughts?
_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev


_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|

Re: Suggesting to enable paint timing by default

Noam Rosenthal


On Tue, May 12, 2020 at 1:36 AM Maciej Stachowiak <[hidden email]> wrote:

I noticed from comments in one of the Radars that the patch may result in an additional “fake paint”, so it should probably be performance tested. Have you done any testing?
I've tested it locally, I haven't noticed any significant side effect, because in complex situations the fake paint only happens once per page and bails early once contentfulness is detected. but I can run any additional test needed.
 
We’ll likely want to A/B some of Apple’s page load speed benchmarks.
A/B testing load speed sounds sensible. How do we go about doing that?
 

I’d like to hear from others on maturity of the spec and readiness of the code.

 - Maciej

On May 11, 2020, at 11:44 AM, Noam Rosenthal <[hidden email]> wrote:

Following the discussion with Simon in https://bugs.webkit.org/show_bug.cgi?id=78011

Since we have a pretty stable spec (https://w3c.github.io/paint-timing/), lots of passing web platform tests, other browser vendor support and a working implementation of first contentful paint, I am planning to submit a patch to enable paint timing by default. 

Any objections? Other thoughts?
_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev


_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|

Re: Suggesting to enable paint timing by default

Maciej Stachowiak


On May 11, 2020, at 9:53 PM, Noam Rosenthal <[hidden email]> wrote:



On Tue, May 12, 2020 at 1:36 AM Maciej Stachowiak <[hidden email]> wrote:

I noticed from comments in one of the Radars that the patch may result in an additional “fake paint”, so it should probably be performance tested. Have you done any testing?
I've tested it locally, I haven't noticed any significant side effect, because in complex situations the fake paint only happens once per page and bails early once contentfulness is detected. but I can run any additional test needed.
 
We’ll likely want to A/B some of Apple’s page load speed benchmarks.
A/B testing load speed sounds sensible. How do we go about doing that?

Unfortunately our page load speed benchmarks are not public because they incorporate captured page content, which we can’t freely redistribute.

A helpful person from Apple may be able to set up an A/B test for this patch.

 - Maciej


_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|

Re: Suggesting to enable paint timing by default

Noam Rosenthal
On Tue, May 12, 2020 at 10:28 AM Maciej Stachowiak <[hidden email]> wrote:


On May 11, 2020, at 9:53 PM, Noam Rosenthal <[hidden email]> wrote:



On Tue, May 12, 2020 at 1:36 AM Maciej Stachowiak <[hidden email]> wrote:

I noticed from comments in one of the Radars that the patch may result in an additional “fake paint”, so it should probably be performance tested. Have you done any testing?
I've tested it locally, I haven't noticed any significant side effect, because in complex situations the fake paint only happens once per page and bails early once contentfulness is detected. but I can run any additional test needed.
 
We’ll likely want to A/B some of Apple’s page load speed benchmarks.
A/B testing load speed sounds sensible. How do we go about doing that?

Unfortunately our page load speed benchmarks are not public because they incorporate captured page content, which we can’t freely redistribute.

A helpful person from Apple may be able to set up an A/B test for this patch.
 Understood. I'd be happy to facilitate/support/do what's necessary to move this forward.
Thanks!

_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|

Re: Suggesting to enable paint timing by default

Noam Rosenthal


Following up on this.
FOn Tue, May 12, 2020 at 10:28 AM Maciej Stachowiak <[hidden email]> wrote:


On May 11, 2020, at 9:53 PM, Noam Rosenthal <[hidden email]> wrote:



On Tue, May 12, 2020 at 1:36 AM Maciej Stachowiak <[hidden email]> wrote:

I noticed from comments in one of the Radars that the patch may result in an additional “fake paint”, so it should probably be performance tested. Have you done any testing?
I've tested it locally, I haven't noticed any significant side effect, because in complex situations the fake paint only happens once per page and bails early once contentfulness is detected. but I can run any additional test needed.
 
We’ll likely want to A/B some of Apple’s page load speed benchmarks.
A/B testing load speed sounds sensible. How do we go about doing that?

Unfortunately our page load speed benchmarks are not public because they incorporate captured page content, which we can’t freely redistribute.

So, can someone else from Apple review that the code is mature enough for this? Simon had reviewed the original patch. Maybe Zalan/Darin? 

A helpful person from Apple may be able to set up an A/B test for this patch.
What's required to ask for help from a helpful person at Apple? :) 

_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|

Re: Suggesting to enable paint timing by default

Yoav Weiss
[hidden email] [hidden email] who were involved in the spec discussions.

On Wed, May 27, 2020 at 10:29 AM Noam Rosenthal <[hidden email]> wrote:


Following up on this.
FOn Tue, May 12, 2020 at 10:28 AM Maciej Stachowiak <[hidden email]> wrote:


On May 11, 2020, at 9:53 PM, Noam Rosenthal <[hidden email]> wrote:



On Tue, May 12, 2020 at 1:36 AM Maciej Stachowiak <[hidden email]> wrote:

I noticed from comments in one of the Radars that the patch may result in an additional “fake paint”, so it should probably be performance tested. Have you done any testing?
I've tested it locally, I haven't noticed any significant side effect, because in complex situations the fake paint only happens once per page and bails early once contentfulness is detected. but I can run any additional test needed.
 
We’ll likely want to A/B some of Apple’s page load speed benchmarks.
A/B testing load speed sounds sensible. How do we go about doing that?

Unfortunately our page load speed benchmarks are not public because they incorporate captured page content, which we can’t freely redistribute.

So, can someone else from Apple review that the code is mature enough for this? Simon had reviewed the original patch. Maybe Zalan/Darin? 

A helpful person from Apple may be able to set up an A/B test for this patch.
What's required to ask for help from a helpful person at Apple? :) 
_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev

_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|

Re: Suggesting to enable paint timing by default

Noam Rosenthal


On Wed, May 27, 2020 at 12:04 PM Yoav Weiss <[hidden email]> wrote:
[hidden email] [hidden email] who were involved in the spec discussions.

On Wed, May 27, 2020 at 10:29 AM Noam Rosenthal <[hidden email]> wrote:


Following up on this.
FOn Tue, May 12, 2020 at 10:28 AM Maciej Stachowiak <[hidden email]> wrote:


On May 11, 2020, at 9:53 PM, Noam Rosenthal <[hidden email]> wrote:



On Tue, May 12, 2020 at 1:36 AM Maciej Stachowiak <[hidden email]> wrote:

I noticed from comments in one of the Radars that the patch may result in an additional “fake paint”, so it should probably be performance tested. Have you done any testing?
I've tested it locally, I haven't noticed any significant side effect, because in complex situations the fake paint only happens once per page and bails early once contentfulness is detected. but I can run any additional test needed.
 
We’ll likely want to A/B some of Apple’s page load speed benchmarks.
A/B testing load speed sounds sensible. How do we go about doing that?

Unfortunately our page load speed benchmarks are not public because they incorporate captured page content, which we can’t freely redistribute.

So, can someone else from Apple review that the code is mature enough for this? Simon had reviewed the original patch. Maybe Zalan/Darin? 

A helpful person from Apple may be able to set up an A/B test for this patch.
What's required to ask for help from a helpful person at Apple? :) 
Hola
Pinging about this again :)
The code for paint timing API is sitting there in the repo, waiting either for internal Apple A/B tests, for an additional code maturity review, or for enabling it by default... I'm here if any changes in the code need to be made.

Trying to figure out how we can proceed with this... [hidden email]?
Cheers

_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|

Re: Suggesting to enable paint timing by default

Keith Miller
If you tell me how to enable paint timing by default, I can start an A/B task for you. I’m probably not qualified to review it for code maturity though.

Cheers,
Keith

On Jul 13, 2020, at 3:02 AM, Noam Rosenthal <[hidden email]> wrote:



On Wed, May 27, 2020 at 12:04 PM Yoav Weiss <[hidden email]> wrote:
[hidden email] [hidden email] who were involved in the spec discussions.

On Wed, May 27, 2020 at 10:29 AM Noam Rosenthal <[hidden email]> wrote:


Following up on this.
FOn Tue, May 12, 2020 at 10:28 AM Maciej Stachowiak <[hidden email]> wrote:


On May 11, 2020, at 9:53 PM, Noam Rosenthal <[hidden email]> wrote:



On Tue, May 12, 2020 at 1:36 AM Maciej Stachowiak <[hidden email]> wrote:

I noticed from comments in one of the Radars that the patch may result in an additional “fake paint”, so it should probably be performance tested. Have you done any testing? 
I've tested it locally, I haven't noticed any significant side effect, because in complex situations the fake paint only happens once per page and bails early once contentfulness is detected. but I can run any additional test needed.
 
We’ll likely want to A/B some of Apple’s page load speed benchmarks.
A/B testing load speed sounds sensible. How do we go about doing that?

Unfortunately our page load speed benchmarks are not public because they incorporate captured page content, which we can’t freely redistribute.

So, can someone else from Apple review that the code is mature enough for this? Simon had reviewed the original patch. Maybe Zalan/Darin? 

A helpful person from Apple may be able to set up an A/B test for this patch.
What's required to ask for help from a helpful person at Apple? :) 
Hola
Pinging about this again :)
The code for paint timing API is sitting there in the repo, waiting either for internal Apple A/B tests, for an additional code maturity review, or for enabling it by default... I'm here if any changes in the code need to be made.

Trying to figure out how we can proceed with this... [hidden email]?
Cheers
_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev


_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|

Re: Suggesting to enable paint timing by default

Noam Rosenthal


On Mon, Jul 13, 2020 at 9:15 PM Keith Miller <[hidden email]> wrote:
If you tell me how to enable paint timing by default, I can start an A/B task for you. I’m probably not qualified to review it for code maturity though.
Awesome, thanks!
It's an experimental runtime flag calledPaintTimingEnabled
I have a patch for enabling it by default here: https://bugs.webkit.org/show_bug.cgi?id=211736
We mainly need to test that measuring paint timing doesn't (badly) influence loading performance.

  

Cheers,
Keith

On Jul 13, 2020, at 3:02 AM, Noam Rosenthal <[hidden email]> wrote:



On Wed, May 27, 2020 at 12:04 PM Yoav Weiss <[hidden email]> wrote:
[hidden email] [hidden email] who were involved in the spec discussions.

On Wed, May 27, 2020 at 10:29 AM Noam Rosenthal <[hidden email]> wrote:


Following up on this.
FOn Tue, May 12, 2020 at 10:28 AM Maciej Stachowiak <[hidden email]> wrote:


On May 11, 2020, at 9:53 PM, Noam Rosenthal <[hidden email]> wrote:



On Tue, May 12, 2020 at 1:36 AM Maciej Stachowiak <[hidden email]> wrote:

I noticed from comments in one of the Radars that the patch may result in an additional “fake paint”, so it should probably be performance tested. Have you done any testing? 
I've tested it locally, I haven't noticed any significant side effect, because in complex situations the fake paint only happens once per page and bails early once contentfulness is detected. but I can run any additional test needed.
 
We’ll likely want to A/B some of Apple’s page load speed benchmarks.
A/B testing load speed sounds sensible. How do we go about doing that?

Unfortunately our page load speed benchmarks are not public because they incorporate captured page content, which we can’t freely redistribute.

So, can someone else from Apple review that the code is mature enough for this? Simon had reviewed the original patch. Maybe Zalan/Darin? 

A helpful person from Apple may be able to set up an A/B test for this patch.
What's required to ask for help from a helpful person at Apple? :) 
Hola
Pinging about this again :)
The code for paint timing API is sitting there in the repo, waiting either for internal Apple A/B tests, for an additional code maturity review, or for enabling it by default... I'm here if any changes in the code need to be made.

Trying to figure out how we can proceed with this... [hidden email]?
Cheers
_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev


_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|

Re: Suggesting to enable paint timing by default

Keith Miller
Results appear to be neutral on the page load time benchmark, so you should be good on that front. I don’t know who the best person to vet the maturity of the code is though, sorry.

Cheers,
Keith

On Jul 13, 2020, at 11:38 AM, Noam Rosenthal <[hidden email]> wrote:



On Mon, Jul 13, 2020 at 9:15 PM Keith Miller <[hidden email]> wrote:
If you tell me how to enable paint timing by default, I can start an A/B task for you. I’m probably not qualified to review it for code maturity though.
Awesome, thanks!
It's an experimental runtime flag calledPaintTimingEnabled
I have a patch for enabling it by default here: https://bugs.webkit.org/show_bug.cgi?id=211736
We mainly need to test that measuring paint timing doesn't (badly) influence loading performance.

  

Cheers,
Keith

On Jul 13, 2020, at 3:02 AM, Noam Rosenthal <[hidden email]> wrote:



On Wed, May 27, 2020 at 12:04 PM Yoav Weiss <[hidden email]> wrote:
[hidden email] [hidden email] who were involved in the spec discussions.

On Wed, May 27, 2020 at 10:29 AM Noam Rosenthal <[hidden email]> wrote:


Following up on this.
FOn Tue, May 12, 2020 at 10:28 AM Maciej Stachowiak <[hidden email]> wrote:


On May 11, 2020, at 9:53 PM, Noam Rosenthal <[hidden email]> wrote:



On Tue, May 12, 2020 at 1:36 AM Maciej Stachowiak <[hidden email]> wrote:

I noticed from comments in one of the Radars that the patch may result in an additional “fake paint”, so it should probably be performance tested. Have you done any testing? 
I've tested it locally, I haven't noticed any significant side effect, because in complex situations the fake paint only happens once per page and bails early once contentfulness is detected. but I can run any additional test needed.
 
We’ll likely want to A/B some of Apple’s page load speed benchmarks.
A/B testing load speed sounds sensible. How do we go about doing that?

Unfortunately our page load speed benchmarks are not public because they incorporate captured page content, which we can’t freely redistribute.

So, can someone else from Apple review that the code is mature enough for this? Simon had reviewed the original patch. Maybe Zalan/Darin? 

A helpful person from Apple may be able to set up an A/B test for this patch.
What's required to ask for help from a helpful person at Apple? :) 
Hola
Pinging about this again :)
The code for paint timing API is sitting there in the repo, waiting either for internal Apple A/B tests, for an additional code maturity review, or for enabling it by default... I'm here if any changes in the code need to be made.

Trying to figure out how we can proceed with this... [hidden email]?
Cheers
_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev



_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|

Re: Suggesting to enable paint timing by default

Noam Rosenthal


On Thu, Jul 16, 2020 at 11:03 PM Keith Miller <[hidden email]> wrote:
Results appear to be neutral on the page load time benchmark, so you should be good on that front. I don’t know who the best person to vet the maturity of the code is though, sorry.

Thanks a lot Keith, I appreciate it!
[hidden email], what would be a good way to assert whether the code maturity is good enough to enable paint timing by default?
The original code was reviewed by smfr and initially by zalan. It's covered by over 30 tests, mostly WPT, and A/B tests show no effect on load times as per Keith's check.
Would asking for additional reviews be the next step? From whom?

Cheers,
Noam

_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev