Re: [webkit-dev] Suggesting to enable paint timing by default

[Noam Rosenthal]

On Thu, 20 Aug 2020 at 6:26 Maciej Stachowiak  wrote:

>
>
> On Jul 17, 2020, at 12:12 AM, Noam Rosenthal  wrote:
>
>
>
> On Thu, Jul 16, 2020 at 11:03 PM Keith Miller 
> 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!
> @Maciej Stachowiak , 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?
>
>
> At this point, if a reviewer approves a patch to enable it by default on
> trunk, I think you are good to go.
>
> As a courtesy to Apple, I’d ask you to hold off on landing until
> mid-September, but that is optional.
>

Great
The patch has already been reviewed,
I will re-land it in mid September.
Thank you!

>
>  - Maciej
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Suggesting to enable paint timing by default

[Maciej Stachowiak]

> On Jul 17, 2020, at 12:12 AM, Noam Rosenthal  wrote:
> 
> 
> 
> On Thu, Jul 16, 2020 at 11:03 PM Keith Miller  > 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!
> @Maciej Stachowiak , 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?

At this point, if a reviewer approves a patch to enable it by default on trunk, 
I think you are good to go.

As a courtesy to Apple, I’d ask you to hold off on landing until mid-September, 
but that is optional.

 - Maciej___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Suggesting to enable paint timing by default

[Noam Rosenthal]

On Thu, Jul 16, 2020 at 11:03 PM Keith Miller 
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!
@Maciej Stachowiak , 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
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] 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  wrote:
> 
> 
> 
> On Mon, Jul 13, 2020 at 9:15 PM Keith Miller  > 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 > > wrote:
>> 
>> 
>> 
>> On Wed, May 27, 2020 at 12:04 PM Yoav Weiss > > wrote:
>> +Ryosuke Niwa  +Alex Christensen 
>>  who were involved in the spec discussions.
>> 
>> On Wed, May 27, 2020 at 10:29 AM Noam Rosenthal > > wrote:
>> 
>> 
>> Following up on this.
>> FOn Tue, May 12, 2020 at 10:28 AM Maciej Stachowiak > > wrote:
>> 
>> 
>>> On May 11, 2020, at 9:53 PM, Noam Rosenthal >> > wrote:
>>> 
>>> 
>>> 
>>> On Tue, May 12, 2020 at 1:36 AM Maciej Stachowiak >> > 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... @Maciej Stachowiak 
>> ?
>> Cheers
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org 
>> https://lists.webkit.org/mailman/listinfo/webkit-dev 
>> 

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Suggesting to enable paint timing by default

[Noam Rosenthal]

On Mon, Jul 13, 2020 at 9:15 PM Keith Miller  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  wrote:
>
>
>
> On Wed, May 27, 2020 at 12:04 PM Yoav Weiss  wrote:
>
>> +Ryosuke Niwa  +Alex Christensen
>>  who were involved in the spec discussions.
>>
>> On Wed, May 27, 2020 at 10:29 AM Noam Rosenthal  wrote:
>>
>>>
>>>
>>> Following up on this.
>>>
 FOn Tue, May 12, 2020 at 10:28 AM Maciej Stachowiak 
 wrote:

>
>
> On May 11, 2020, at 9:53 PM, Noam Rosenthal  wrote:
>
>
>
> On Tue, May 12, 2020 at 1:36 AM Maciej Stachowiak 
> 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... @Maciej Stachowiak
> ?
> Cheers
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev
>
>
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] 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  wrote:
> 
> 
> 
> On Wed, May 27, 2020 at 12:04 PM Yoav Weiss  > wrote:
> +Ryosuke Niwa  +Alex Christensen 
>  who were involved in the spec discussions.
> 
> On Wed, May 27, 2020 at 10:29 AM Noam Rosenthal  > wrote:
> 
> 
> Following up on this.
> FOn Tue, May 12, 2020 at 10:28 AM Maciej Stachowiak  > wrote:
> 
> 
>> On May 11, 2020, at 9:53 PM, Noam Rosenthal > > wrote:
>> 
>> 
>> 
>> On Tue, May 12, 2020 at 1:36 AM Maciej Stachowiak > > 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... @Maciej Stachowiak 
> ?
> Cheers
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org 
> https://lists.webkit.org/mailman/listinfo/webkit-dev 
> 
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Suggesting to enable paint timing by default

[Noam Rosenthal]

On Wed, May 27, 2020 at 12:04 PM Yoav Weiss  wrote:

> +Ryosuke Niwa  +Alex Christensen
>  who were involved in the spec discussions.
>
> On Wed, May 27, 2020 at 10:29 AM Noam Rosenthal  wrote:
>
>>
>>
>> Following up on this.
>>
>>> FOn Tue, May 12, 2020 at 10:28 AM Maciej Stachowiak 
>>> wrote:
>>>


 On May 11, 2020, at 9:53 PM, Noam Rosenthal  wrote:



 On Tue, May 12, 2020 at 1:36 AM Maciej Stachowiak 
 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... @Maciej Stachowiak
?
Cheers
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Suggesting to enable paint timing by default

[Yoav Weiss]

+Ryosuke Niwa  +Alex Christensen  who
were involved in the spec discussions.

On Wed, May 27, 2020 at 10:29 AM Noam Rosenthal  wrote:

>
>
> Following up on this.
>
>> FOn Tue, May 12, 2020 at 10:28 AM Maciej Stachowiak 
>> wrote:
>>
>>>
>>>
>>> On May 11, 2020, at 9:53 PM, Noam Rosenthal  wrote:
>>>
>>>
>>>
>>> On Tue, May 12, 2020 at 1:36 AM Maciej Stachowiak  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
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Suggesting to enable paint timing by default

[Noam Rosenthal]

Following up on this.

> FOn Tue, May 12, 2020 at 10:28 AM Maciej Stachowiak  wrote:
>
>>
>>
>> On May 11, 2020, at 9:53 PM, Noam Rosenthal  wrote:
>>
>>
>>
>> On Tue, May 12, 2020 at 1:36 AM Maciej Stachowiak  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
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Suggesting to enable paint timing by default

[Noam Rosenthal]

On Tue, May 12, 2020 at 10:28 AM Maciej Stachowiak  wrote:

>
>
> On May 11, 2020, at 9:53 PM, Noam Rosenthal  wrote:
>
>
>
> On Tue, May 12, 2020 at 1:36 AM Maciej Stachowiak  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
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Suggesting to enable paint timing by default

[Maciej Stachowiak]

> On May 11, 2020, at 9:53 PM, Noam Rosenthal  wrote:
> 
> 
> 
> On Tue, May 12, 2020 at 1:36 AM Maciej Stachowiak  > 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
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Suggesting to enable paint timing by default

[Noam Rosenthal]

On Tue, May 12, 2020 at 1:36 AM Maciej Stachowiak  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  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.
>
> https://bugs.webkit.org/show_bug.cgi?id=211736
>
> Any objections? Other thoughts?
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev
>
>
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] 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  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. 
> 
> https://bugs.webkit.org/show_bug.cgi?id=211736 
> 
> 
> Any objections? Other thoughts?
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

[webkit-dev] 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.

https://bugs.webkit.org/show_bug.cgi?id=211736

Any objections? Other thoughts?
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev