[adb] Don't copy features set on each get()

The features are already cached in a static and don't change.
Let's return the cached set instead of copying it every time.

Bug: 150183149
Test: manual
Change-Id: Ifdca852cc3b32e09e50ea4771f7878987c46cce8
This commit is contained in:
Yurii Zubrytskyi 2020-03-26 18:19:28 -07:00
parent 83c7ad22a1
commit 6fc26dff7c
9 changed files with 71 additions and 78 deletions

View File

@ -1198,9 +1198,9 @@ HostRequestResult handle_host_request(std::string_view service, TransportType ty
FeatureSet features = supported_features();
// Abuse features to report libusb status.
if (should_use_libusb()) {
features.insert(kFeatureLibusb);
features.emplace_back(kFeatureLibusb);
}
features.insert(kFeaturePushSync);
features.emplace_back(kFeaturePushSync);
SendOkay(reply_fd, FeatureSetToString(features));
return HostRequestResult::Handled;
}

View File

@ -37,6 +37,7 @@
#include <vector>
#include <android-base/file.h>
#include <android-base/no_destructor.h>
#include <android-base/stringprintf.h>
#include <android-base/strings.h>
#include <android-base/thread_annotations.h>
@ -415,18 +416,14 @@ std::string format_host_command(const char* command) {
return android::base::StringPrintf("%s:%s", prefix, command);
}
bool adb_get_feature_set(FeatureSet* feature_set, std::string* error) {
static FeatureSet* features = nullptr;
if (!features) {
const FeatureSet& adb_get_feature_set() {
static const android::base::NoDestructor<FeatureSet> features([] {
std::string result;
if (adb_query(format_host_command("features"), &result, error)) {
features = new FeatureSet(StringToFeatureSet(result));
if (!adb_query(format_host_command("features"), &result, &result)) {
fprintf(stderr, "failed to get feature set: %s\n", result.c_str());
return FeatureSet{};
}
}
if (features) {
*feature_set = *features;
return true;
}
feature_set->clear();
return false;
return StringToFeatureSet(result);
}());
return *features;
}

View File

@ -76,7 +76,7 @@ bool adb_status(borrowed_fd fd, std::string* _Nonnull error);
std::string format_host_command(const char* _Nonnull command);
// Get the feature set of the current preferred transport.
bool adb_get_feature_set(FeatureSet* _Nonnull feature_set, std::string* _Nonnull error);
const FeatureSet& adb_get_feature_set();
#if defined(__linux__)
// Get the path of a file containing the path to the server executable, if the socket spec set via

View File

@ -57,10 +57,8 @@ enum class CmdlineOption { None, Enable, Disable };
}
static bool can_use_feature(const char* feature) {
FeatureSet features;
std::string error;
if (!adb_get_feature_set(&features, &error)) {
fprintf(stderr, "error: %s\n", error.c_str());
auto&& features = adb_get_feature_set();
if (features.empty()) {
return false;
}
return CanUseFeature(features, feature);

View File

@ -672,10 +672,8 @@ static int RemoteShell(bool use_shell_protocol, const std::string& type_arg, cha
}
static int adb_shell(int argc, const char** argv) {
FeatureSet features;
std::string error_message;
if (!adb_get_feature_set(&features, &error_message)) {
fprintf(stderr, "error: %s\n", error_message.c_str());
auto&& features = adb_get_feature_set();
if (features.empty()) {
return 1;
}
@ -779,13 +777,10 @@ static int adb_shell(int argc, const char** argv) {
}
static int adb_abb(int argc, const char** argv) {
FeatureSet features;
std::string error_message;
if (!adb_get_feature_set(&features, &error_message)) {
fprintf(stderr, "error: %s\n", error_message.c_str());
auto&& features = adb_get_feature_set();
if (features.empty()) {
return 1;
}
if (!CanUseFeature(features, kFeatureAbb)) {
error_exit("abb is not supported by the device");
}
@ -1164,9 +1159,8 @@ int send_shell_command(const std::string& command, bool disable_shell_protocol,
// Use shell protocol if it's supported and the caller doesn't explicitly
// disable it.
if (!disable_shell_protocol) {
FeatureSet features;
std::string error;
if (adb_get_feature_set(&features, &error)) {
auto&& features = adb_get_feature_set();
if (!features.empty()) {
use_shell_protocol = CanUseFeature(features, kFeatureShell2);
} else {
// Device was unreachable.
@ -1816,10 +1810,8 @@ int adb_commandline(int argc, const char** argv) {
}
return adb_connect_command(android::base::StringPrintf("tcpip:%d", port));
} else if (!strcmp(argv[0], "remount")) {
FeatureSet features;
std::string error;
if (!adb_get_feature_set(&features, &error)) {
fprintf(stderr, "error: %s\n", error.c_str());
auto&& features = adb_get_feature_set();
if (features.empty()) {
return 1;
}
@ -2042,10 +2034,8 @@ int adb_commandline(int argc, const char** argv) {
} else if (!strcmp(argv[0], "track-jdwp")) {
return adb_connect_command("track-jdwp");
} else if (!strcmp(argv[0], "track-app")) {
FeatureSet features;
std::string error;
if (!adb_get_feature_set(&features, &error)) {
fprintf(stderr, "error: %s\n", error.c_str());
auto&& features = adb_get_feature_set();
if (features.empty()) {
return 1;
}
if (!CanUseFeature(features, kFeatureTrackApp)) {
@ -2074,10 +2064,8 @@ int adb_commandline(int argc, const char** argv) {
return 0;
} else if (!strcmp(argv[0], "features")) {
// Only list the features common to both the adb client and the device.
FeatureSet features;
std::string error;
if (!adb_get_feature_set(&features, &error)) {
fprintf(stderr, "error: %s\n", error.c_str());
auto&& features = adb_get_feature_set();
if (features.empty()) {
return 1;
}

View File

@ -225,13 +225,14 @@ struct TransferLedger {
class SyncConnection {
public:
SyncConnection() : acknowledgement_buffer_(sizeof(sync_status) + SYNC_DATA_MAX) {
SyncConnection()
: acknowledgement_buffer_(sizeof(sync_status) + SYNC_DATA_MAX),
features_(adb_get_feature_set()) {
acknowledgement_buffer_.resize(0);
max = SYNC_DATA_MAX; // TODO: decide at runtime.
std::string error;
if (!adb_get_feature_set(&features_, &error)) {
Error("failed to get feature set: %s", error.c_str());
if (features_.empty()) {
Error("failed to get feature set");
} else {
have_stat_v2_ = CanUseFeature(features_, kFeatureStat2);
have_ls_v2_ = CanUseFeature(features_, kFeatureLs2);
@ -239,6 +240,7 @@ class SyncConnection {
have_sendrecv_v2_brotli_ = CanUseFeature(features_, kFeatureSendRecv2Brotli);
have_sendrecv_v2_lz4_ = CanUseFeature(features_, kFeatureSendRecv2LZ4);
have_sendrecv_v2_dry_run_send_ = CanUseFeature(features_, kFeatureSendRecv2DryRunSend);
std::string error;
fd.reset(adb_connect("sync:", &error));
if (fd < 0) {
Error("connect failed: %s", error.c_str());
@ -919,7 +921,7 @@ class SyncConnection {
private:
std::deque<std::pair<std::string, std::string>> deferred_acknowledgements_;
Block acknowledgement_buffer_;
FeatureSet features_;
const FeatureSet& features_;
bool have_stat_v2_;
bool have_ls_v2_;
bool have_sendrecv_v2_;

View File

@ -29,7 +29,6 @@
#include <unistd.h>
#include <algorithm>
#include <deque>
#include <list>
#include <memory>
#include <mutex>
@ -40,6 +39,7 @@
#include <adb/crypto/x509_generator.h>
#include <adb/tls/tls_connection.h>
#include <android-base/logging.h>
#include <android-base/no_destructor.h>
#include <android-base/parsenetaddress.h>
#include <android-base/stringprintf.h>
#include <android-base/strings.h>
@ -1170,28 +1170,29 @@ size_t atransport::get_max_payload() const {
}
const FeatureSet& supported_features() {
// Local static allocation to avoid global non-POD variables.
static const FeatureSet* features = new FeatureSet{
kFeatureShell2,
kFeatureCmd,
kFeatureStat2,
kFeatureLs2,
kFeatureFixedPushMkdir,
kFeatureApex,
kFeatureAbb,
kFeatureFixedPushSymlinkTimestamp,
kFeatureAbbExec,
kFeatureRemountShell,
kFeatureTrackApp,
kFeatureSendRecv2,
kFeatureSendRecv2Brotli,
kFeatureSendRecv2LZ4,
kFeatureSendRecv2DryRunSend,
// Increment ADB_SERVER_VERSION when adding a feature that adbd needs
// to know about. Otherwise, the client can be stuck running an old
// version of the server even after upgrading their copy of adb.
// (http://b/24370690)
};
static const android::base::NoDestructor<FeatureSet> features([] {
return FeatureSet{
kFeatureShell2,
kFeatureCmd,
kFeatureStat2,
kFeatureLs2,
kFeatureFixedPushMkdir,
kFeatureApex,
kFeatureAbb,
kFeatureFixedPushSymlinkTimestamp,
kFeatureAbbExec,
kFeatureRemountShell,
kFeatureTrackApp,
kFeatureSendRecv2,
kFeatureSendRecv2Brotli,
kFeatureSendRecv2LZ4,
kFeatureSendRecv2DryRunSend,
// Increment ADB_SERVER_VERSION when adding a feature that adbd needs
// to know about. Otherwise, the client can be stuck running an old
// version of the server even after upgrading their copy of adb.
// (http://b/24370690)
};
}());
return *features;
}
@ -1205,16 +1206,20 @@ FeatureSet StringToFeatureSet(const std::string& features_string) {
return FeatureSet();
}
auto names = android::base::Split(features_string, ",");
return FeatureSet(names.begin(), names.end());
return android::base::Split(features_string, ",");
}
template <class Range, class Value>
static bool contains(const Range& r, const Value& v) {
return std::find(std::begin(r), std::end(r), v) != std::end(r);
}
bool CanUseFeature(const FeatureSet& feature_set, const std::string& feature) {
return feature_set.count(feature) > 0 && supported_features().count(feature) > 0;
return contains(feature_set, feature) && contains(supported_features(), feature);
}
bool atransport::has_feature(const std::string& feature) const {
return features_.count(feature) > 0;
return contains(features_, feature);
}
void atransport::SetFeatures(const std::string& features_string) {

View File

@ -30,7 +30,7 @@
#include <string>
#include <string_view>
#include <thread>
#include <unordered_set>
#include <vector>
#include <android-base/macros.h>
#include <android-base/thread_annotations.h>
@ -40,7 +40,10 @@
#include "adb_unique_fd.h"
#include "types.h"
typedef std::unordered_set<std::string> FeatureSet;
// Even though the feature set is used as a set, we only have a dozen or two
// of available features at any moment. Vector works much better in terms of
// both memory usage and performance for these sizes.
using FeatureSet = std::vector<std::string>;
namespace adb {
namespace tls {

View File

@ -66,7 +66,7 @@ TEST_F(TransportTest, SetFeatures) {
ASSERT_TRUE(t.has_feature("bar"));
t.SetFeatures(FeatureSetToString(FeatureSet{"foo", "bar", "foo"}));
ASSERT_EQ(2U, t.features().size());
ASSERT_LE(2U, t.features().size());
ASSERT_TRUE(t.has_feature("foo"));
ASSERT_TRUE(t.has_feature("bar"));