[Lldb-commits] [lldb] 7ff2de4 - Do not list adb devices when a device id is given

[Pavel Labath via lldb-commits]

Author: Emre Kultursay
Date: 2020-05-29T11:36:11+02:00
New Revision: 7ff2de4f0c60c5d13880440e85ef8edc78482a2f

URL: https://github.com/llvm/llvm-project/commit/7ff2de4f0c60c5d13880440e85ef8edc78482a2f
DIFF: https://github.com/llvm/llvm-project/commit/7ff2de4f0c60c5d13880440e85ef8edc78482a2f.diff

LOG: Do not list adb devices when a device id is given

Summary:
On Android, this method gets called twice: first when establishing
a host-server connection, then when attaching to a process id.

Each call takes several seconds to finish (especially slower on Windows)
and eliminating the call for the typical case improves latency significantly.

Reviewed By: labath

Differential Revision: https://reviews.llvm.org/D79586

Added: 
    lldb/unittests/Platform/Android/AdbClientTest.cpp
    lldb/unittests/Platform/Android/CMakeLists.txt

Modified: 
    lldb/source/Plugins/Platform/Android/AdbClient.cpp
    lldb/unittests/Platform/CMakeLists.txt

Removed: 
    


################################################################################
diff  --git a/lldb/source/Plugins/Platform/Android/AdbClient.cpp b/lldb/source/Plugins/Platform/Android/AdbClient.cpp
index 14d97ebe7c3c..17707118d9c9 100644
--- a/lldb/source/Plugins/Platform/Android/AdbClient.cpp
+++ b/lldb/source/Plugins/Platform/Android/AdbClient.cpp
@@ -94,11 +94,7 @@ Status ReadAllBytes(Connection &conn, void *buffer, size_t size) {
 
 Status AdbClient::CreateByDeviceID(const std::string &device_id,
                                    AdbClient &adb) {
-  DeviceIDList connect_devices;
-  auto error = adb.GetDevices(connect_devices);
-  if (error.Fail())
-    return error;
-
+  Status error;
   std::string android_serial;
   if (!device_id.empty())
     android_serial = device_id;
@@ -106,18 +102,18 @@ Status AdbClient::CreateByDeviceID(const std::string &device_id,
     android_serial = env_serial;
 
   if (android_serial.empty()) {
-    if (connect_devices.size() != 1)
+    DeviceIDList connected_devices;
+    error = adb.GetDevices(connected_devices);
+    if (error.Fail())
+      return error;
+
+    if (connected_devices.size() != 1)
       return Status("Expected a single connected device, got instead %zu - try "
                     "setting 'ANDROID_SERIAL'",
-                    connect_devices.size());
-    adb.SetDeviceID(connect_devices.front());
+                    connected_devices.size());
+    adb.SetDeviceID(connected_devices.front());
   } else {
-    auto find_it = std::find(connect_devices.begin(), connect_devices.end(),
-                             android_serial);
-    if (find_it == connect_devices.end())
-      return Status("Device \"%s\" not found", android_serial.c_str());
-
-    adb.SetDeviceID(*find_it);
+    adb.SetDeviceID(android_serial);
   }
   return error;
 }

diff  --git a/lldb/unittests/Platform/Android/AdbClientTest.cpp b/lldb/unittests/Platform/Android/AdbClientTest.cpp
new file mode 100644
index 000000000000..0808b96f69fc
--- /dev/null
+++ b/lldb/unittests/Platform/Android/AdbClientTest.cpp
@@ -0,0 +1,51 @@
+//===-- AdbClientTest.cpp -------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "gtest/gtest.h"
+#include "Plugins/Platform/Android/AdbClient.h"
+#include <cstdlib>
+
+static void set_env(const char *var, const char *value) {
+#ifdef _WIN32
+  _putenv_s(var, value);
+#else
+  setenv(var, value, true);
+#endif
+}
+
+using namespace lldb;
+using namespace lldb_private;
+
+namespace lldb_private {
+namespace platform_android {
+
+class AdbClientTest : public ::testing::Test {
+public:
+  void SetUp() override { set_env("ANDROID_SERIAL", ""); }
+
+  void TearDown() override { set_env("ANDROID_SERIAL", ""); }
+};
+
+TEST(AdbClientTest, CreateByDeviceId) {
+  AdbClient adb;
+  Status error = AdbClient::CreateByDeviceID("device1", adb);
+  EXPECT_TRUE(error.Success());
+  EXPECT_EQ("device1", adb.GetDeviceID());
+}
+
+TEST(AdbClientTest, CreateByDeviceId_ByEnvVar) {
+  set_env("ANDROID_SERIAL", "device2");
+
+  AdbClient adb;
+  Status error = AdbClient::CreateByDeviceID("", adb);
+  EXPECT_TRUE(error.Success());
+  EXPECT_EQ("device2", adb.GetDeviceID());
+}
+
+} // end namespace platform_android
+} // end namespace lldb_private

diff  --git a/lldb/unittests/Platform/Android/CMakeLists.txt b/lldb/unittests/Platform/Android/CMakeLists.txt
new file mode 100644
index 000000000000..3de2a2d12016
--- /dev/null
+++ b/lldb/unittests/Platform/Android/CMakeLists.txt
@@ -0,0 +1,8 @@
+include_directories(${LLDB_SOURCE_DIR}/source/Plugins/Platform/Android)
+
+add_lldb_unittest(AdbClientTest
+  AdbClientTest.cpp
+
+  LINK_LIBS
+    lldbPluginPlatformAndroid
+  )

diff  --git a/lldb/unittests/Platform/CMakeLists.txt b/lldb/unittests/Platform/CMakeLists.txt
index 3362ca08d60c..eb7f0a6ca3c4 100644
--- a/lldb/unittests/Platform/CMakeLists.txt
+++ b/lldb/unittests/Platform/CMakeLists.txt
@@ -6,3 +6,5 @@ add_lldb_unittest(LLDBPlatformTests
   LINK_COMPONENTS
     Support
   )
+
+add_subdirectory(Android)