Speedup net_unittests on Android by 30%.

Speedup net_unittests on Android by 30%.

The Chrome test server spawner (running on the host) was sleeping way too much.
Note that ideally we should move away from polling but this still sounds like a                                                                                                                                                                                                                                                
reasonable short term improvement.

R=bulach@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=199949

[bulach, 2013-05-07 15:09:28]

lgtm, thanks! just small suggestions below:

https://codereview.chromium.org/14767023/diff/5001/build/android/pylib/chrome...
File build/android/pylib/chrome_test_server_spawner.py (right):

https://codereview.chromium.org/14767023/diff/5001/build/android/pylib/chrome...
build/android/pylib/chrome_test_server_spawner.py:244: sleep_time_sec *= 2
perhaps move this out into a "_CheckDevicePortStatus()", similarly to
_CheckPortStatus() above?
so this would then become:

if device_port and _CheckDevicePortStatus(adb, device_port):
  self.is_ready = True
  self.forwarder_device_port = device_port


also, not sure how prevalent this functionality is, but maybe move to ports
itself?

https://codereview.chromium.org/14767023/diff/5001/build/android/pylib/chrome...
build/android/pylib/chrome_test_server_spawner.py:251: sleep_time_sec *= 2
should we limit this somehow up to say, 1 or two secs? since this is just on the
host side, if Stop() is called at X+1, this would take another whole X to
actually stop...

[Philippe, 2013-05-07 15:47:41]

Thanks Marcus! PTAL since I had to make other changes in order to factor out the
polling logic.

https://chromiumcodereview.appspot.com/14767023/diff/5001/build/android/pylib...
File build/android/pylib/chrome_test_server_spawner.py (right):

https://chromiumcodereview.appspot.com/14767023/diff/5001/build/android/pylib...
build/android/pylib/chrome_test_server_spawner.py:244: sleep_time_sec *= 2
On 2013/05/07 15:09:28, bulach wrote:
> perhaps move this out into a "_CheckDevicePortStatus()", similarly to
> _CheckPortStatus() above?
> so this would then become:
> 
> if device_port and _CheckDevicePortStatus(adb, device_port):
>   self.is_ready = True
>   self.forwarder_device_port = device_port
> 
> 
> also, not sure how prevalent this functionality is, but maybe move to ports
> itself?

Good idea. I would not encourage people to use this pattern so if you agree we
can keep this here.

https://chromiumcodereview.appspot.com/14767023/diff/5001/build/android/pylib...
build/android/pylib/chrome_test_server_spawner.py:251: sleep_time_sec *= 2
On 2013/05/07 15:09:28, bulach wrote:
> should we limit this somehow up to say, 1 or two secs? since this is just on
the
> host side, if Stop() is called at X+1, this would take another whole X to
> actually stop...

Yeah good point.

[pliard, 2013-05-07 15:50:20]

Please hold on, my last patch set makes the test suite fails.


On Tue, May 7, 2013 at 5:47 PM, <pliard@chromium.org> wrote:

> Thanks Marcus! PTAL since I had to make other changes in order to factor
> out the
> polling logic.
>
>
> https://chromiumcodereview.**appspot.com/14767023/diff/**
>
5001/build/android/pylib/**chrome_test_server_spawner.py<https://chromiumcodereview.appspot.com/14767023/diff/5001/build/android/pylib/chrome_test_server_spawner.py>
> File build/android/pylib/chrome_**test_server_spawner.py (right):
>
> https://chromiumcodereview.**appspot.com/14767023/diff/**
>
5001/build/android/pylib/**chrome_test_server_spawner.py#**newcode244<https://chromiumcodereview.appspot.com/14767023/diff/5001/build/android/pylib/chrome_test_server_spawner.py#newcode244>
>
> build/android/pylib/chrome_**test_server_spawner.py:244: sleep_time_sec *=
> 2
> On 2013/05/07 15:09:28, bulach wrote:
>
>> perhaps move this out into a "_CheckDevicePortStatus()", similarly to
>> _CheckPortStatus() above?
>> so this would then become:
>>
>
>  if device_port and _CheckDevicePortStatus(adb, device_port):
>>    self.is_ready = True
>>    self.forwarder_device_port = device_port
>>
>
>
>  also, not sure how prevalent this functionality is, but maybe move to
>>
> ports
>
>> itself?
>>
>
> Good idea. I would not encourage people to use this pattern so if you
> agree we can keep this here.
>
> https://chromiumcodereview.**appspot.com/14767023/diff/**
>
5001/build/android/pylib/**chrome_test_server_spawner.py#**newcode251<https://chromiumcodereview.appspot.com/14767023/diff/5001/build/android/pylib/chrome_test_server_spawner.py#newcode251>
>
> build/android/pylib/chrome_**test_server_spawner.py:251: sleep_time_sec *=
> 2
> On 2013/05/07 15:09:28, bulach wrote:
>
>> should we limit this somehow up to say, 1 or two secs? since this is
>>
> just on the
>
>> host side, if Stop() is called at X+1, this would take another whole X
>>
> to
>
>> actually stop...
>>
>
> Yeah good point.
>
>
https://chromiumcodereview.**appspot.com/14767023/<https://chromiumcodereview...
>

[pliard, 2013-05-07 15:55:27]

I fixed the issue with patch set #3. Feel free to take another look.


On Tue, May 7, 2013 at 5:50 PM, Philippe Liard <pliard@google.com> wrote:

> Please hold on, my last patch set makes the test suite fails.
>
>
> On Tue, May 7, 2013 at 5:47 PM, <pliard@chromium.org> wrote:
>
>> Thanks Marcus! PTAL since I had to make other changes in order to factor
>> out the
>> polling logic.
>>
>>
>> https://chromiumcodereview.**appspot.com/14767023/diff/**
>>
5001/build/android/pylib/**chrome_test_server_spawner.py<https://chromiumcodereview.appspot.com/14767023/diff/5001/build/android/pylib/chrome_test_server_spawner.py>
>> File build/android/pylib/chrome_**test_server_spawner.py (right):
>>
>> https://chromiumcodereview.**appspot.com/14767023/diff/**
>>
5001/build/android/pylib/**chrome_test_server_spawner.py#**newcode244<https://chromiumcodereview.appspot.com/14767023/diff/5001/build/android/pylib/chrome_test_server_spawner.py#newcode244>
>>
>> build/android/pylib/chrome_**test_server_spawner.py:244: sleep_time_sec
>> *=
>> 2
>> On 2013/05/07 15:09:28, bulach wrote:
>>
>>> perhaps move this out into a "_CheckDevicePortStatus()", similarly to
>>> _CheckPortStatus() above?
>>> so this would then become:
>>>
>>
>>  if device_port and _CheckDevicePortStatus(adb, device_port):
>>>    self.is_ready = True
>>>    self.forwarder_device_port = device_port
>>>
>>
>>
>>  also, not sure how prevalent this functionality is, but maybe move to
>>>
>> ports
>>
>>> itself?
>>>
>>
>> Good idea. I would not encourage people to use this pattern so if you
>> agree we can keep this here.
>>
>> https://chromiumcodereview.**appspot.com/14767023/diff/**
>>
5001/build/android/pylib/**chrome_test_server_spawner.py#**newcode251<https://chromiumcodereview.appspot.com/14767023/diff/5001/build/android/pylib/chrome_test_server_spawner.py#newcode251>
>>
>> build/android/pylib/chrome_**test_server_spawner.py:251: sleep_time_sec
>> *=
>> 2
>> On 2013/05/07 15:09:28, bulach wrote:
>>
>>> should we limit this somehow up to say, 1 or two secs? since this is
>>>
>> just on the
>>
>>> host side, if Stop() is called at X+1, this would take another whole X
>>>
>> to
>>
>>> actually stop...
>>>
>>
>> Yeah good point.
>>
>>
https://chromiumcodereview.**appspot.com/14767023/<https://chromiumcodereview...
>>
>
>

[bulach, 2013-05-07 16:03:10]

lgtm, thanks! yeah, agree, let's keep this behavior private if nothing else uses
it..

[commit-bot: I haz the power, 2013-05-13 14:05:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pliard@chromium.org/14767023/11001

[commit-bot: I haz the power, 2013-05-13 20:56:48]

Retried try job too often on win_rel for step(s) content_browsertests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[Philippe, 2013-05-14 08:47:13]

Committed patchset #3 manually as r199949 (presubmit successful).