New issue
Advanced search Search tips

Issue 872561 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 10
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

Tast should start adb server in parallel to avoid server startup cost

Project Member Reported by nya@chromium.org, Aug 9

Issue description

In ARC integration tests we use adb installed on the device to communicate with Android.

We today use dev-util/android-tools-5.1.1_p13, but its initialization code has inefficient sleep(3) in start-server, which uniformly adds 3 seconds to all ARC tests.

The issue was fixed upstream in Android O (8.0):
https://android-review.googlesource.com/c/platform/system/core/+/388558

We'll want to uprev android-tools to get the fix.

 
I tried getting a newer package from portage:
cros_portage_upgrade --upgrade --unstable-ok --board=caroline dev-util/android-tools

However the build fails on cmake configure. vapier@, do you have any idea why it fails? (I saw you actually uploaded the package in the upstream)


CMake Error at CMakeLists.txt:233 (elseif):
  given arguments:

    "STREQUAL" "x86_64"

  Unknown arguments specified


see issue 792957.  the last upgrade broke hwtests and i gave up.  that was version 6 at least.
Thanks for the pointer! Yes, it's possible adb uprev may broke ARC tests. But I still can't build adb... I tried your patch locally but it also fails on build. Maybe it's caused by difference of toolchains?

In file included from core/adb/adb.cpp:19:
core/adb/sysdeps.h:505:12: error: cannot initialize return object of type 'char *' with an rvalue of type 'const char *'
    return strchr(path, '/');
           ^~~~~~~~~~~~~~~~~
core/adb/sysdeps.h:510:12: error: cannot initialize return object of type 'char *' with an rvalue of type 'const char *'
    return strrchr(path, '/');
           ^~~~~~~~~~~~~~~~~~
core/adb/adb.cpp:729:38: error: assigning to 'char *' from incompatible type 'const char *'
        local = strchr(service, ':') + 1;
                ~~~~~~~~~~~~~~~~~~~~~^~~
3 errors generated.


Re: android-tools 8, error occurred at:

   if (OPENSSL_NO_ASM)
     add_definitions(-DOPENSSL_NO_ASM)
     set(ARCH "generic")
>> elseif (${CMAKE_SYSTEM_PROCESSOR} STREQUAL "x86_64")
     set(ARCH "x86_64")
   elseif (${CMAKE_SYSTEM_PROCESSOR} STREQUAL "amd64")


Also I found following log in CMakeOutput.log:

The target system is: Linux -  - 
The host system is: Linux - 4.9.0-6-amd64 - x86_64

Looks target system is not correctly passed to cmake?

hmm, i saw those cmake failures too.  i thought they were resolved by upgrading cmake eclass & cmake package ...
OK, I needed this patch:
https://gitweb.gentoo.org/repo/gentoo.git/commit/eclass/cmake-utils.eclass?id=d9b6edbeec026d8f63ed278854526eebd5fb30e5

Now compile passes, I'll take a look at adb failures.

Summary: Tast should start adb server in parallel to avoid server startup cost (was: adb in dev-util/android-tools-5.1.1_p13 has inefficient sleep(3) on start-server)
Good news: that adb 8 worked just fine (even with Android N).

Bad news: adb 8 did not help speeding up adb server startup. It seems like adb server is taking time anyway to scan USB devices, and I'm not sure how to disable USB device scan. For Tast, I'll just start adb server in parallel.

Project Member

Comment 8 by bugdroid1@chromium.org, Aug 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/tast-tests/+/9cb5b69026958274bb1b543d8990a97db16a3995

commit 9cb5b69026958274bb1b543d8990a97db16a3995
Author: Shuhei Takahashi <nya@chromium.org>
Date: Fri Aug 10 08:44:34 2018

arc: Start ADB local server in parallel.

Starting an ADB local server takes a few seconds. We can do this
in parallel to Android boot to speed up ARC tests.

This change improves iteration time of arc.BootForever from
~28s to ~23s on caroline.

BUG= chromium:872561 
TEST=tast -verbose run DUT arc.BootForever

Change-Id: I80da65f347e793ef4fa3dfccc82dc4ac4835f529
Reviewed-on: https://chromium-review.googlesource.com/1168718
Commit-Ready: Shuhei Takahashi <nya@chromium.org>
Tested-by: Shuhei Takahashi <nya@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>

[modify] https://crrev.com/9cb5b69026958274bb1b543d8990a97db16a3995/src/chromiumos/tast/local/arc/arc.go
[modify] https://crrev.com/9cb5b69026958274bb1b543d8990a97db16a3995/src/chromiumos/tast/local/arc/adb.go

Status: Fixed (was: Assigned)
This issue has been fixed. I may still try to uprev adb, but it would be updated in issue 792957.

Sign in to add a comment