[chromium-dev] Heads up: safely ignored regression on Linux Startup perf test

2009-11-17 Thread John Abd-El-Malek 
Heads up to explain the sudden jump on Linux Startup perf test.

I just submitted r32264 which makes opening and closing processes happen off
the UI thread.  Surprisingly enough, according to UMA stats these would take
an average of 1s on Linux for the first renderer, and 100ms on Windows.
 Subsequent launches were very quick on Linux, but on Windows averaged 50ms.
 When using session restore with many tabs, this would block the UI thread
for quite a bit.  Also, Linux had code which might sleep for up to 2s on the
UI thread waiting for the process to die.

Linux Warm 
Startupperf
test is showing a big regression (200ms->300ms).  With Elliott's
insight, I tracked this down to the fact that the UI thread is very busy at
startup handling GTK messages, so the posted task back to it to tell
BrowserRenderProcessHost that the handle is available is queued up.  This
parallelization is exactly what we want though, and the Linux New Tab
Coldtest
went from ~615ms to ~440ms.  It's hard to see a change on Linux
Warm 
Startup
because
of all the noise.

As for other platforms: Mac
Startup
(both
warm and cold) went from around 307ms to 290ms, while Mac New Tab Cold

went
from 720ms to 620ms.  Windows didn't have much change.

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Heads up: safely ignored regression on Linux Startup perf test

2009-11-18 Thread Darin Fisher 
This sounds like goodness.  Updating the reference builds is usually a good
thing to do in cases like this so that new changes are easier to notice.
-Darin

On Tue, Nov 17, 2009 at 8:52 PM, John Abd-El-Malek  wrote:

> Heads up to explain the sudden jump on Linux Startup perf test.
>
> I just submitted r32264 which makes opening and closing processes happen
> off the UI thread.  Surprisingly enough, according to UMA stats these would
> take an average of 1s on Linux for the first renderer, and 100ms on Windows.
>  Subsequent launches were very quick on Linux, but on Windows averaged 50ms.
>  When using session restore with many tabs, this would block the UI thread
> for quite a bit.  Also, Linux had code which might sleep for up to 2s on the
> UI thread waiting for the process to die.
>
> Linux Warm 
> Startupperf
>  test is showing a big regression (200ms->300ms).  With Elliott's
> insight, I tracked this down to the fact that the UI thread is very busy at
> startup handling GTK messages, so the posted task back to it to tell
> BrowserRenderProcessHost that the handle is available is queued up.  This
> parallelization is exactly what we want though, and the Linux New Tab 
> Coldtest
>  went from ~615ms to ~440ms.  It's hard to see a change on Linux
> Warm 
> Startup
>  because
> of all the noise.
>
> As for other platforms: Mac 
> Startup
>  (both
> warm and cold) went from around 307ms to 290ms, while Mac New Tab Cold
> 
>  went
> from 720ms to 620ms.  Windows didn't have much change.
>
> --
> Chromium Developers mailing list: chromium-dev@googlegroups.com
> View archives, change email options, or unsubscribe:
> http://groups.google.com/group/chromium-dev

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Heads up: safely ignored regression on Linux Startup perf test

2009-11-18 Thread John Abd-El-Malek 
Yep, that was my plan.  I'm planning on doing the same thing for the rest of
the child processes, and if I see any significant changes on the perf test
(which I don't expect), I'll update the reference builds again.

On Wed, Nov 18, 2009 at 6:46 AM, Brett Wilson  wrote:

> On Tue, Nov 17, 2009 at 10:57 PM, Darin Fisher  wrote:
> > This sounds like goodness.  Updating the reference builds is usually a
> good
> > thing to do in cases like this so that new changes are easier to notice.
>
> We'll be doing this soon anyway. Al has a patch for the IPC message
> types running out which will break the reference build.
>
> Brett
>

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Heads up: safely ignored regression on Linux Startup perf test

2009-11-18 Thread John Abd-El-Malek 
I don't have an answer to that.  The t_ref line didn't move either.

On Wed, Nov 18, 2009 at 11:42 AM, Tony Chang  wrote:

> Why didn't the black line on the linux warm perf bot change?  It says that
> that is the extension_toolstrip50 test, which I would expect to run slower
> than the extension_toolstrip1 test.  Maybe the graph is pulling the wrong
> numbers?
>
>
> http://build.chromium.org/buildbot/perf/linux-release-hardy/startup/report.html?history=150&graph=warm
>
> On Wed, Nov 18, 2009 at 9:53 AM, John Abd-El-Malek wrote:
>
>> Yep, that was my plan.  I'm planning on doing the same thing for the rest
>> of the child processes, and if I see any significant changes on the perf
>> test (which I don't expect), I'll update the reference builds again.
>>
>> On Wed, Nov 18, 2009 at 6:46 AM, Brett Wilson  wrote:
>>
>>>  On Tue, Nov 17, 2009 at 10:57 PM, Darin Fisher 
>>> wrote:
>>> > This sounds like goodness.  Updating the reference builds is usually a
>>> good
>>> > thing to do in cases like this so that new changes are easier to
>>> notice.
>>>
>>> We'll be doing this soon anyway. Al has a patch for the IPC message
>>> types running out which will break the reference build.
>>>
>>> Brett
>>>
>>
>>  --
>> Chromium Developers mailing list: chromium-dev@googlegroups.com
>> View archives, change email options, or unsubscribe:
>> http://groups.google.com/group/chromium-dev
>>
>
>

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Heads up: safely ignored regression on Linux Startup perf test

2009-11-19 Thread Tony Chang 
For reasons unknown to me, this line jumped back up. It seems it's because
of Matt's revert:
http://src.chromium.org/viewvc/chrome?view=rev&revision=32524

This is a startup test, so it basically times how long it takes for
LaunchApp to return.  Maybe the methodology here is a bit off?

On Wed, Nov 18, 2009 at 6:02 PM, Chase Phillips  wrote:

> t_ref shouldn't move, though, since it was isolated from your change.
>
> Tony, I don't think there's a problem with the graph pulling the wrong
> numbers.  I see the same difference between extension_toolstrip50 and
> extension_toolstrip1 when comparing the linux release hardy's graph values,
> the .dat file the graph code uses, and the output of the startup test
> itself.  I thought maybe extension_toolstrip50 could be using the reference
> build on accident, so I verified startup_test.cc runs extension_toolstrip50
> on the current build instead of the reference build (it does).
>
> Things look fine on Windows (the perf graph is what I'd expect, and running
> the test locally results in toolstrip50 results greater than toolstrip1).
>  These tests don't run on Mac.  We should run the tests on Linux to verify
> things look sane locally, too.  No explanation for the odd results yet.
>
> Chase
>
> On Wed, Nov 18, 2009 at 3:08 PM, John Abd-El-Malek wrote:
>
>> I don't have an answer to that.  The t_ref line didn't move either.
>>
>> On Wed, Nov 18, 2009 at 11:42 AM, Tony Chang  wrote:
>>
>>> Why didn't the black line on the linux warm perf bot change?  It says
>>> that that is the extension_toolstrip50 test, which I would expect to run
>>> slower than the extension_toolstrip1 test.  Maybe the graph is pulling the
>>> wrong numbers?
>>>
>>>
>>> http://build.chromium.org/buildbot/perf/linux-release-hardy/startup/report.html?history=150&graph=warm
>>>
>>> On Wed, Nov 18, 2009 at 9:53 AM, John Abd-El-Malek wrote:
>>>
 Yep, that was my plan.  I'm planning on doing the same thing for the
 rest of the child processes, and if I see any significant changes on the
 perf test (which I don't expect), I'll update the reference builds again.

 On Wed, Nov 18, 2009 at 6:46 AM, Brett Wilson wrote:

>  On Tue, Nov 17, 2009 at 10:57 PM, Darin Fisher 
> wrote:
> > This sounds like goodness.  Updating the reference builds is usually
> a good
> > thing to do in cases like this so that new changes are easier to
> notice.
>
> We'll be doing this soon anyway. Al has a patch for the IPC message
> types running out which will break the reference build.
>
> Brett
>

  --
 Chromium Developers mailing list: chromium-dev@googlegroups.com
 View archives, change email options, or unsubscribe:
 http://groups.google.com/group/chromium-dev

>>>
>>>
>>  --
>> Chromium Developers mailing list: chromium-dev@googlegroups.com
>> View archives, change email options, or unsubscribe:
>> http://groups.google.com/group/chromium-dev
>>
>
>

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Heads up: safely ignored regression on Linux Startup perf test

2009-11-19 Thread John Abd-El-Malek 
On Thu, Nov 19, 2009 at 1:57 PM, Tony Chang  wrote:

> For reasons unknown to me, this line jumped back up. It seems it's because
> of Matt's revert:
> http://src.chromium.org/viewvc/chrome?view=rev&revision=32524
>
> This is a startup test, so it basically times how long it takes for
> LaunchApp to return.  Maybe the methodology here is a bit off?
>

Yeah, I think New Tab tests are more important, since they measure what the
user sees, not when internal APIs are called. In the case of the above
change by Matt, New Tab Cold hasn't changed so I think it's fine to ignore
the rise.
http://build.chromium.org/buildbot/perf/linux-release-hardy/new-tab-ui-cold/report.html?history=150


> On Wed, Nov 18, 2009 at 6:02 PM, Chase Phillips  wrote:
>
>> t_ref shouldn't move, though, since it was isolated from your change.
>>
>> Tony, I don't think there's a problem with the graph pulling the wrong
>> numbers.  I see the same difference between extension_toolstrip50 and
>> extension_toolstrip1 when comparing the linux release hardy's graph values,
>> the .dat file the graph code uses, and the output of the startup test
>> itself.  I thought maybe extension_toolstrip50 could be using the reference
>> build on accident, so I verified startup_test.cc runs extension_toolstrip50
>> on the current build instead of the reference build (it does).
>>
>> Things look fine on Windows (the perf graph is what I'd expect, and
>> running the test locally results in toolstrip50 results greater than
>> toolstrip1).  These tests don't run on Mac.  We should run the tests on
>> Linux to verify things look sane locally, too.  No explanation for the odd
>> results yet.
>>
>> Chase
>>
>> On Wed, Nov 18, 2009 at 3:08 PM, John Abd-El-Malek wrote:
>>
>>> I don't have an answer to that.  The t_ref line didn't move either.
>>>
>>> On Wed, Nov 18, 2009 at 11:42 AM, Tony Chang  wrote:
>>>
 Why didn't the black line on the linux warm perf bot change?  It says
 that that is the extension_toolstrip50 test, which I would expect to run
 slower than the extension_toolstrip1 test.  Maybe the graph is pulling the
 wrong numbers?


 http://build.chromium.org/buildbot/perf/linux-release-hardy/startup/report.html?history=150&graph=warm

 On Wed, Nov 18, 2009 at 9:53 AM, John Abd-El-Malek 
 wrote:

> Yep, that was my plan.  I'm planning on doing the same thing for the
> rest of the child processes, and if I see any significant changes on the
> perf test (which I don't expect), I'll update the reference builds again.
>
> On Wed, Nov 18, 2009 at 6:46 AM, Brett Wilson wrote:
>
>>  On Tue, Nov 17, 2009 at 10:57 PM, Darin Fisher 
>> wrote:
>> > This sounds like goodness.  Updating the reference builds is usually
>> a good
>> > thing to do in cases like this so that new changes are easier to
>> notice.
>>
>> We'll be doing this soon anyway. Al has a patch for the IPC message
>> types running out which will break the reference build.
>>
>> Brett
>>
>
>  --
> Chromium Developers mailing list: chromium-dev@googlegroups.com
> View archives, change email options, or unsubscribe:
> http://groups.google.com/group/chromium-dev
>


>>>  --
>>> Chromium Developers mailing list: chromium-dev@googlegroups.com
>>> View archives, change email options, or unsubscribe:
>>> http://groups.google.com/group/chromium-dev
>>>
>>
>>
>  --
> Chromium Developers mailing list: chromium-dev@googlegroups.com
> View archives, change email options, or unsubscribe:
> http://groups.google.com/group/chromium-dev
>

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Heads up: safely ignored regression on Linux Startup perf test

2009-11-19 Thread Matt Perry 
Yeah, I'm sure it's my revert. We're now starting the extension processes in
parallel with the UI loop (on the launcher thread), rather than always
deferred until the next pass through the UI loop. It probably won't make
much difference in practice, so I'm torn on whether to undo that change.

On Thu, Nov 19, 2009 at 1:57 PM, Tony Chang  wrote:

> For reasons unknown to me, this line jumped back up. It seems it's because
> of Matt's revert:
> http://src.chromium.org/viewvc/chrome?view=rev&revision=32524
>
> This is a startup test, so it basically times how long it takes for
> LaunchApp to return.  Maybe the methodology here is a bit off?
>
> On Wed, Nov 18, 2009 at 6:02 PM, Chase Phillips  wrote:
>
>> t_ref shouldn't move, though, since it was isolated from your change.
>>
>> Tony, I don't think there's a problem with the graph pulling the wrong
>> numbers.  I see the same difference between extension_toolstrip50 and
>> extension_toolstrip1 when comparing the linux release hardy's graph values,
>> the .dat file the graph code uses, and the output of the startup test
>> itself.  I thought maybe extension_toolstrip50 could be using the reference
>> build on accident, so I verified startup_test.cc runs extension_toolstrip50
>> on the current build instead of the reference build (it does).
>>
>> Things look fine on Windows (the perf graph is what I'd expect, and
>> running the test locally results in toolstrip50 results greater than
>> toolstrip1).  These tests don't run on Mac.  We should run the tests on
>> Linux to verify things look sane locally, too.  No explanation for the odd
>> results yet.
>>
>> Chase
>>
>> On Wed, Nov 18, 2009 at 3:08 PM, John Abd-El-Malek wrote:
>>
>>> I don't have an answer to that.  The t_ref line didn't move either.
>>>
>>> On Wed, Nov 18, 2009 at 11:42 AM, Tony Chang  wrote:
>>>
 Why didn't the black line on the linux warm perf bot change?  It says
 that that is the extension_toolstrip50 test, which I would expect to run
 slower than the extension_toolstrip1 test.  Maybe the graph is pulling the
 wrong numbers?


 http://build.chromium.org/buildbot/perf/linux-release-hardy/startup/report.html?history=150&graph=warm

 On Wed, Nov 18, 2009 at 9:53 AM, John Abd-El-Malek 
 wrote:

> Yep, that was my plan.  I'm planning on doing the same thing for the
> rest of the child processes, and if I see any significant changes on the
> perf test (which I don't expect), I'll update the reference builds again.
>
> On Wed, Nov 18, 2009 at 6:46 AM, Brett Wilson wrote:
>
>>  On Tue, Nov 17, 2009 at 10:57 PM, Darin Fisher 
>> wrote:
>> > This sounds like goodness.  Updating the reference builds is usually
>> a good
>> > thing to do in cases like this so that new changes are easier to
>> notice.
>>
>> We'll be doing this soon anyway. Al has a patch for the IPC message
>> types running out which will break the reference build.
>>
>> Brett
>>
>
>  --
> Chromium Developers mailing list: chromium-dev@googlegroups.com
> View archives, change email options, or unsubscribe:
> http://groups.google.com/group/chromium-dev
>


>>>  --
>>> Chromium Developers mailing list: chromium-dev@googlegroups.com
>>> View archives, change email options, or unsubscribe:
>>> http://groups.google.com/group/chromium-dev
>>>
>>
>>
>  --
> Chromium Developers mailing list: chromium-dev@googlegroups.com
> View archives, change email options, or unsubscribe:
> http://groups.google.com/group/chromium-dev
>

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Heads up: safely ignored regression on Linux Startup perf test

2009-11-19 Thread Matt Perry 
On Thu, Nov 19, 2009 at 2:14 PM, John Abd-El-Malek  wrote:

>
>
> On Thu, Nov 19, 2009 at 1:57 PM, Tony Chang  wrote:
>
>> For reasons unknown to me, this line jumped back up. It seems it's because
>> of Matt's revert:
>> http://src.chromium.org/viewvc/chrome?view=rev&revision=32524
>>
>> This is a startup test, so it basically times how long it takes for
>> LaunchApp to return.  Maybe the methodology here is a bit off?
>>
>
> Yeah, I think New Tab tests are more important, since they measure what the
> user sees, not when internal APIs are called. In the case of the above
> change by Matt, New Tab Cold hasn't changed so I think it's fine to ignore
> the rise.
> http://build.chromium.org/buildbot/perf/linux-release-hardy/new-tab-ui-cold/report.html?history=150
>

New Tab hasn't changed because none of those tests load any extensions.
Maybe we should add some.


>
>> On Wed, Nov 18, 2009 at 6:02 PM, Chase Phillips  wrote:
>>
>>> t_ref shouldn't move, though, since it was isolated from your change.
>>>
>>> Tony, I don't think there's a problem with the graph pulling the wrong
>>> numbers.  I see the same difference between extension_toolstrip50 and
>>> extension_toolstrip1 when comparing the linux release hardy's graph values,
>>> the .dat file the graph code uses, and the output of the startup test
>>> itself.  I thought maybe extension_toolstrip50 could be using the reference
>>> build on accident, so I verified startup_test.cc runs extension_toolstrip50
>>> on the current build instead of the reference build (it does).
>>>
>>> Things look fine on Windows (the perf graph is what I'd expect, and
>>> running the test locally results in toolstrip50 results greater than
>>> toolstrip1).  These tests don't run on Mac.  We should run the tests on
>>> Linux to verify things look sane locally, too.  No explanation for the odd
>>> results yet.
>>>
>>> Chase
>>>
>>> On Wed, Nov 18, 2009 at 3:08 PM, John Abd-El-Malek wrote:
>>>
 I don't have an answer to that.  The t_ref line didn't move either.

 On Wed, Nov 18, 2009 at 11:42 AM, Tony Chang  wrote:

> Why didn't the black line on the linux warm perf bot change?  It says
> that that is the extension_toolstrip50 test, which I would expect to run
> slower than the extension_toolstrip1 test.  Maybe the graph is pulling the
> wrong numbers?
>
>
> http://build.chromium.org/buildbot/perf/linux-release-hardy/startup/report.html?history=150&graph=warm
>
> On Wed, Nov 18, 2009 at 9:53 AM, John Abd-El-Malek 
> wrote:
>
>> Yep, that was my plan.  I'm planning on doing the same thing for the
>> rest of the child processes, and if I see any significant changes on the
>> perf test (which I don't expect), I'll update the reference builds again.
>>
>> On Wed, Nov 18, 2009 at 6:46 AM, Brett Wilson wrote:
>>
>>>  On Tue, Nov 17, 2009 at 10:57 PM, Darin Fisher 
>>> wrote:
>>> > This sounds like goodness.  Updating the reference builds is
>>> usually a good
>>> > thing to do in cases like this so that new changes are easier to
>>> notice.
>>>
>>> We'll be doing this soon anyway. Al has a patch for the IPC message
>>> types running out which will break the reference build.
>>>
>>> Brett
>>>
>>
>>  --
>> Chromium Developers mailing list: chromium-dev@googlegroups.com
>> View archives, change email options, or unsubscribe:
>> http://groups.google.com/group/chromium-dev
>>
>
>
  --
 Chromium Developers mailing list: chromium-dev@googlegroups.com
 View archives, change email options, or unsubscribe:
 http://groups.google.com/group/chromium-dev

>>>
>>>
>>  --
>> Chromium Developers mailing list: chromium-dev@googlegroups.com
>> View archives, change email options, or unsubscribe:
>> http://groups.google.com/group/chromium-dev
>>
>
>

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Heads up: safely ignored regression on Linux Startup perf test

2009-11-19 Thread John Abd-El-Malek 
On Thu, Nov 19, 2009 at 2:17 PM, Matt Perry  wrote:

> On Thu, Nov 19, 2009 at 2:14 PM, John Abd-El-Malek wrote:
>
>>
>>
>> On Thu, Nov 19, 2009 at 1:57 PM, Tony Chang  wrote:
>>
>>> For reasons unknown to me, this line jumped back up. It seems it's
>>> because of Matt's revert:
>>> http://src.chromium.org/viewvc/chrome?view=rev&revision=32524
>>>
>>> This is a startup test, so it basically times how long it takes for
>>> LaunchApp to return.  Maybe the methodology here is a bit off?
>>>
>>
>> Yeah, I think New Tab tests are more important, since they measure what
>> the user sees, not when internal APIs are called. In the case of the above
>> change by Matt, New Tab Cold hasn't changed so I think it's fine to ignore
>> the rise.
>> http://build.chromium.org/buildbot/perf/linux-release-hardy/new-tab-ui-cold/report.html?history=150
>>
>
> New Tab hasn't changed because none of those tests load any extensions.
> Maybe we should add some.
>

good point, thanks for the explanation.

I think your change will improve the user-visible response time even if this
particular test shows a drop in perf.  If I'm reading your change correctly,
things have to be faster since the process creation on the launcher thread
is kicked off earlier (instead of after a PostTask to the UI thread).


>
>>
>>> On Wed, Nov 18, 2009 at 6:02 PM, Chase Phillips  wrote:
>>>
 t_ref shouldn't move, though, since it was isolated from your change.

 Tony, I don't think there's a problem with the graph pulling the wrong
 numbers.  I see the same difference between extension_toolstrip50 and
 extension_toolstrip1 when comparing the linux release hardy's graph values,
 the .dat file the graph code uses, and the output of the startup test
 itself.  I thought maybe extension_toolstrip50 could be using the reference
 build on accident, so I verified startup_test.cc runs extension_toolstrip50
 on the current build instead of the reference build (it does).

 Things look fine on Windows (the perf graph is what I'd expect, and
 running the test locally results in toolstrip50 results greater than
 toolstrip1).  These tests don't run on Mac.  We should run the tests on
 Linux to verify things look sane locally, too.  No explanation for the odd
 results yet.

 Chase

 On Wed, Nov 18, 2009 at 3:08 PM, John Abd-El-Malek 
 wrote:

> I don't have an answer to that.  The t_ref line didn't move either.
>
> On Wed, Nov 18, 2009 at 11:42 AM, Tony Chang  wrote:
>
>> Why didn't the black line on the linux warm perf bot change?  It says
>> that that is the extension_toolstrip50 test, which I would expect to run
>> slower than the extension_toolstrip1 test.  Maybe the graph is pulling 
>> the
>> wrong numbers?
>>
>>
>> http://build.chromium.org/buildbot/perf/linux-release-hardy/startup/report.html?history=150&graph=warm
>>
>> On Wed, Nov 18, 2009 at 9:53 AM, John Abd-El-Malek 
>> wrote:
>>
>>> Yep, that was my plan.  I'm planning on doing the same thing for the
>>> rest of the child processes, and if I see any significant changes on the
>>> perf test (which I don't expect), I'll update the reference builds 
>>> again.
>>>
>>> On Wed, Nov 18, 2009 at 6:46 AM, Brett Wilson wrote:
>>>
  On Tue, Nov 17, 2009 at 10:57 PM, Darin Fisher 
 wrote:
 > This sounds like goodness.  Updating the reference builds is
 usually a good
 > thing to do in cases like this so that new changes are easier to
 notice.

 We'll be doing this soon anyway. Al has a patch for the IPC message
 types running out which will break the reference build.

 Brett

>>>
>>>  --
>>> Chromium Developers mailing list: chromium-dev@googlegroups.com
>>> View archives, change email options, or unsubscribe:
>>> http://groups.google.com/group/chromium-dev
>>>
>>
>>
>  --
> Chromium Developers mailing list: chromium-dev@googlegroups.com
> View archives, change email options, or unsubscribe:
> http://groups.google.com/group/chromium-dev
>


>>>  --
>>> Chromium Developers mailing list: chromium-dev@googlegroups.com
>>> View archives, change email options, or unsubscribe:
>>> http://groups.google.com/group/chromium-dev
>>>
>>
>>
>

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Heads up: safely ignored regression on Linux Startup perf test

2009-11-19 Thread Matt Perry 
On Thu, Nov 19, 2009 at 2:20 PM, John Abd-El-Malek  wrote:

>
>
> On Thu, Nov 19, 2009 at 2:17 PM, Matt Perry wrote:
>
>> On Thu, Nov 19, 2009 at 2:14 PM, John Abd-El-Malek wrote:
>>
>>>
>>>
>>> On Thu, Nov 19, 2009 at 1:57 PM, Tony Chang  wrote:
>>>
 For reasons unknown to me, this line jumped back up. It seems it's
 because of Matt's revert:
 http://src.chromium.org/viewvc/chrome?view=rev&revision=32524

 This is a startup test, so it basically times how long it takes for
 LaunchApp to return.  Maybe the methodology here is a bit off?

>>>
>>> Yeah, I think New Tab tests are more important, since they measure what
>>> the user sees, not when internal APIs are called. In the case of the above
>>> change by Matt, New Tab Cold hasn't changed so I think it's fine to ignore
>>> the rise.
>>> http://build.chromium.org/buildbot/perf/linux-release-hardy/new-tab-ui-cold/report.html?history=150
>>>
>>
>> New Tab hasn't changed because none of those tests load any extensions.
>> Maybe we should add some.
>>
>
> good point, thanks for the explanation.
>
> I think your change will improve the user-visible response time even if
> this particular test shows a drop in perf.  If I'm reading your change
> correctly, things have to be faster since the process creation on the
> launcher thread is kicked off earlier (instead of after a PostTask to the UI
> thread).
>

Yes, but the point of the PostTask in the first place was to defer extension
load - they're not essential processes, so they don't need to slow down
startup. If postponing them until after the UI thread settles down improves
user perceived startup time, then we should do that.

That said, I think I'm going to revert my change. If nothing else, it'll
bring the extension startup test time back to normal so we can see
regressions better.


>
>
>>
>>>
 On Wed, Nov 18, 2009 at 6:02 PM, Chase Phillips  wrote:

> t_ref shouldn't move, though, since it was isolated from your change.
>
> Tony, I don't think there's a problem with the graph pulling the wrong
> numbers.  I see the same difference between extension_toolstrip50 and
> extension_toolstrip1 when comparing the linux release hardy's graph 
> values,
> the .dat file the graph code uses, and the output of the startup test
> itself.  I thought maybe extension_toolstrip50 could be using the 
> reference
> build on accident, so I verified startup_test.cc runs 
> extension_toolstrip50
> on the current build instead of the reference build (it does).
>
> Things look fine on Windows (the perf graph is what I'd expect, and
> running the test locally results in toolstrip50 results greater than
> toolstrip1).  These tests don't run on Mac.  We should run the tests on
> Linux to verify things look sane locally, too.  No explanation for the odd
> results yet.
>
> Chase
>
> On Wed, Nov 18, 2009 at 3:08 PM, John Abd-El-Malek 
> wrote:
>
>> I don't have an answer to that.  The t_ref line didn't move either.
>>
>> On Wed, Nov 18, 2009 at 11:42 AM, Tony Chang  wrote:
>>
>>> Why didn't the black line on the linux warm perf bot change?  It says
>>> that that is the extension_toolstrip50 test, which I would expect to run
>>> slower than the extension_toolstrip1 test.  Maybe the graph is pulling 
>>> the
>>> wrong numbers?
>>>
>>>
>>> http://build.chromium.org/buildbot/perf/linux-release-hardy/startup/report.html?history=150&graph=warm
>>>
>>> On Wed, Nov 18, 2009 at 9:53 AM, John Abd-El-Malek >> > wrote:
>>>
 Yep, that was my plan.  I'm planning on doing the same thing for the
 rest of the child processes, and if I see any significant changes on 
 the
 perf test (which I don't expect), I'll update the reference builds 
 again.

 On Wed, Nov 18, 2009 at 6:46 AM, Brett Wilson wrote:

>  On Tue, Nov 17, 2009 at 10:57 PM, Darin Fisher <
> da...@chromium.org> wrote:
> > This sounds like goodness.  Updating the reference builds is
> usually a good
> > thing to do in cases like this so that new changes are easier to
> notice.
>
> We'll be doing this soon anyway. Al has a patch for the IPC message
> types running out which will break the reference build.
>
> Brett
>

  --
 Chromium Developers mailing list: chromium-dev@googlegroups.com
 View archives, change email options, or unsubscribe:
 http://groups.google.com/group/chromium-dev

>>>
>>>
>>  --
>> Chromium Developers mailing list: chromium-dev@googlegroups.com
>> View archives, change email options, or unsubscribe:
>> http://groups.google.com/group/chromium-dev
>>
>
>
  --
 Chromium Developers mailing list: chromium-dev@googlegroups.com
 View a