From 4ea001c2d3dc3f5c8421a5e86098272b295b213e Mon Sep 17 00:00:00 2001 From: Wei Wang Date: Mon, 16 Apr 2018 20:30:36 -0700 Subject: libperfmgr: change to use Value directly in config Using Value directly is more clear and intuitive than using an index of Values. Bug: 77282526 Test: libperfmgr_test and boot wahoo Change-Id: I4414a2bb04be8bf3fc01c20695ebd6ac3c19df79 --- libperfmgr/HintManager.cc | 44 ++++++++++++-------------------- libperfmgr/Node.cc | 10 +++----- libperfmgr/config_schema.json | 12 ++++----- libperfmgr/include/perfmgr/Node.h | 4 +-- libperfmgr/tests/HintManagerTest.cc | 17 +++++++----- libperfmgr/tests/NodeLooperThreadTest.cc | 3 ++- libperfmgr/tests/NodeTest.cc | 7 ++--- libperfmgr/tools/ConfigVerifier.cc | 2 +- 8 files changed, 46 insertions(+), 53 deletions(-) diff --git a/libperfmgr/HintManager.cc b/libperfmgr/HintManager.cc index ce3d125c..691bd41b 100644 --- a/libperfmgr/HintManager.cc +++ b/libperfmgr/HintManager.cc @@ -82,11 +82,12 @@ std::vector HintManager::GetHints() const { } void HintManager::DumpToFd(int fd) { - std::string header("========== Begin perfmgr nodes ==========\n" - "Node Name\t" - "Node Path\t" - "Current Index\t" - "Current Value\n"); + std::string header( + "========== Begin perfmgr nodes ==========\n" + "Node Name\t" + "Node Path\t" + "Current Index\t" + "Current Value\n"); if (!android::base::WriteStringToFd(header, fd)) { LOG(ERROR) << "Failed to dump fd: " << fd; } @@ -158,7 +159,7 @@ std::vector> HintManager::ParseNodes( LOG(ERROR) << "Duplicate Node[" << i << "]'s Name"; nodes_parsed.clear(); return nodes_parsed; - }; + } std::string path = nodes[i]["Path"].asString(); LOG(VERBOSE) << "Node[" << i << "]'s Path: " << path; @@ -284,27 +285,18 @@ std::map> HintManager::ParseActions( } node_index = nodes_index[node_name]; - Json::UInt64 value_index = 0; - if (actions[i]["ValueIndex"].empty() || - !actions[i]["ValueIndex"].isUInt64()) { - LOG(ERROR) << "Failed to read Action" << i << "'s ValueIndex"; - actions_parsed.clear(); - return actions_parsed; - } else { - value_index = actions[i]["ValueIndex"].asUInt64(); - } - std::size_t max_value_index = nodes[node_index]->GetValues().size() - 1; - if (value_index > max_value_index) { - LOG(ERROR) << "Failed to read Action" << i << "'s Duration"; - LOG(ERROR) << "Action[" << i - << "]'s ValueIndex out of bound, value index: " - << value_index << ", max: " << max_value_index; + std::string value_name = actions[i]["Value"].asString(); + LOG(VERBOSE) << "Action[" << i << "]'s Value: " << value_name; + std::size_t value_index = 0; + + if (!nodes[node_index]->GetValueIndex(value_name, &value_index)) { + LOG(ERROR) << "Failed to read Action" << i << "'s Value"; + LOG(ERROR) << "Action[" << i << "]'s Value " << value_name + << " is not defined in Node[" << node_name; actions_parsed.clear(); return actions_parsed; } LOG(VERBOSE) << "Action[" << i << "]'s ValueIndex: " << value_index; - LOG(VERBOSE) << "Action[" << i << "]'s Node Value: " - << nodes[node_index]->GetValues()[value_index]; Json::UInt64 duration = 0; if (actions[i]["Duration"].empty() || @@ -319,8 +311,7 @@ std::map> HintManager::ParseActions( if (actions_parsed.find(hint_type) == actions_parsed.end()) { actions_parsed[hint_type] = std::vector{ - {node_index, static_cast(value_index), - std::chrono::milliseconds(duration)}}; + {node_index, value_index, std::chrono::milliseconds(duration)}}; } else { for (const auto& action : actions_parsed[hint_type]) { if (action.node_index == node_index) { @@ -332,8 +323,7 @@ std::map> HintManager::ParseActions( } } actions_parsed[hint_type].emplace_back( - node_index, static_cast(value_index), - std::chrono::milliseconds(duration)); + node_index, value_index, std::chrono::milliseconds(duration)); } ++total_parsed; diff --git a/libperfmgr/Node.cc b/libperfmgr/Node.cc index 53677700..63cf8aaa 100644 --- a/libperfmgr/Node.cc +++ b/libperfmgr/Node.cc @@ -18,8 +18,8 @@ #include #include -#include #include +#include #include "perfmgr/Node.h" @@ -154,11 +154,9 @@ void Node::DumpToFd(int fd) { LOG(ERROR) << "Failed to read node path: " << node_path_; } node_value = android::base::Trim(node_value); - std::string buf(android::base::StringPrintf("%s\t%s\t%zu\t%s\n", - name_.c_str(), - node_path_.c_str(), - current_val_index_, - node_value.c_str())); + std::string buf(android::base::StringPrintf( + "%s\t%s\t%zu\t%s\n", name_.c_str(), node_path_.c_str(), + current_val_index_, node_value.c_str())); if (!android::base::WriteStringToFd(buf, fd)) { LOG(ERROR) << "Failed to dump fd: " << fd; } diff --git a/libperfmgr/config_schema.json b/libperfmgr/config_schema.json index 0fffe509..b74bacbe 100644 --- a/libperfmgr/config_schema.json +++ b/libperfmgr/config_schema.json @@ -97,15 +97,15 @@ "type": "string", "id": "/properties/Actions/items/properties/Node", "title": "The Node Schema.", - "description": "The Node name of the action.", + "description": "The Node name of the action, which is defined in Nodes.", "minLength": 1 }, - "ValueIndex": { - "type": "integer", - "id": "/properties/Actions/items/properties/ValueIndex", + "Value": { + "type": "string", + "id": "/properties/Actions/items/properties/Value", "title": "The Value Index Schema.", - "description": "The value index of the action.", - "minimum": 0 + "description": "The value of action, which is defined in Nodes.", + "minLength": 1 }, "Duration": { "type": "integer", diff --git a/libperfmgr/include/perfmgr/Node.h b/libperfmgr/include/perfmgr/Node.h index f31b0128..ea4c3e34 100644 --- a/libperfmgr/include/perfmgr/Node.h +++ b/libperfmgr/include/perfmgr/Node.h @@ -42,8 +42,8 @@ namespace perfmgr { // value. For each value, there may be multiple requests because different // powerhints may request the same value, and the requests may have different // expiration times. All of the in-progress powerhints for a given value are -// collected in a RequestGroup. Node class is not thread safe so it need protection -// from caller e.g. NodeLooperThread. +// collected in a RequestGroup. Node class is not thread safe so it needs +// protection from caller e.g. NodeLooperThread. class Node { public: Node(std::string name, std::string node_path, diff --git a/libperfmgr/tests/HintManagerTest.cc b/libperfmgr/tests/HintManagerTest.cc index 0630fe7f..2355b8ff 100644 --- a/libperfmgr/tests/HintManagerTest.cc +++ b/libperfmgr/tests/HintManagerTest.cc @@ -61,19 +61,19 @@ constexpr auto kSLEEP_TOLERANCE_MS = 50ms; // { // "PowerHint": "INTERACTION", // "Node": "CPUCluster1MinFreq", -// "ValueIndex": 1, +// "Value": "1134000", // "Duration": 800 // }, // { // "PowerHint": "LAUNCH", // "Node": "CPUCluster0MinFreq", -// "ValueIndex": 1, +// "Value": "1134000", // "Duration": 500 // }, // { // "PowerHint": "LAUNCH", // "Node": "CPUCluster1MinFreq", -// "ValueIndex": 0, +// "Value": "1512000", // "Duration": 2000 // } // ] @@ -86,10 +86,12 @@ constexpr char kJSON_RAW[] = "\"Path\":\"/sys/devices/system/cpu/cpu4/cpufreq/" "scaling_min_freq\",\"Values\":[\"1512000\",\"1134000\",\"384000\"]," "\"HoldFd\":true}],\"Actions\":[{\"PowerHint\":\"INTERACTION\",\"Node\":" - "\"CPUCluster1MinFreq\",\"ValueIndex\":1,\"Duration\":800},{\"PowerHint\":" - "\"LAUNCH\",\"Node\":\"CPUCluster0MinFreq\",\"ValueIndex\":1,\"Duration\":" + "\"CPUCluster1MinFreq\",\"Value\":\"1134000\",\"Duration\":800},{" + "\"PowerHint\":" + "\"LAUNCH\",\"Node\":\"CPUCluster0MinFreq\",\"Value\":\"1134000\"," + "\"Duration\":" "500},{\"PowerHint\":\"LAUNCH\",\"Node\":\"CPUCluster1MinFreq\"," - "\"ValueIndex\":0,\"Duration\":2000}]}"; + "\"Value\":\"1512000\",\"Duration\":2000}]}"; class HintManagerTest : public ::testing::Test, public HintManager { protected: @@ -150,7 +152,8 @@ class HintManagerTest : public ::testing::Test, public HintManager { std::string json_doc_; }; -static inline void _VerifyPathValue(const std::string& path, const std::string& value) { +static inline void _VerifyPathValue(const std::string& path, + const std::string& value) { std::string s; EXPECT_TRUE(android::base::ReadFileToString(path, &s)) << strerror(errno); EXPECT_EQ(value, s); diff --git a/libperfmgr/tests/NodeLooperThreadTest.cc b/libperfmgr/tests/NodeLooperThreadTest.cc index 6f96cf8f..24659cf7 100644 --- a/libperfmgr/tests/NodeLooperThreadTest.cc +++ b/libperfmgr/tests/NodeLooperThreadTest.cc @@ -54,7 +54,8 @@ class NodeLooperThreadTest : public ::testing::Test { std::vector> files_; }; -static inline void _VerifyPathValue(const std::string& path, const std::string& value) { +static inline void _VerifyPathValue(const std::string& path, + const std::string& value) { std::string s; EXPECT_TRUE(android::base::ReadFileToString(path, &s)) << strerror(errno); EXPECT_EQ(value, s); diff --git a/libperfmgr/tests/NodeTest.cc b/libperfmgr/tests/NodeTest.cc index 2e37a4d5..02429ccd 100644 --- a/libperfmgr/tests/NodeTest.cc +++ b/libperfmgr/tests/NodeTest.cc @@ -33,7 +33,8 @@ using namespace std::chrono_literals; constexpr double kTIMING_TOLERANCE_MS = std::chrono::milliseconds(25).count(); constexpr auto kSLEEP_TOLERANCE_MS = 2ms; -static inline void _VerifyPathValue(const std::string& path, const std::string& value) { +static inline void _VerifyPathValue(const std::string& path, + const std::string& value) { std::string s; EXPECT_TRUE(android::base::ReadFileToString(path, &s)) << strerror(errno); EXPECT_EQ(value, s); @@ -63,8 +64,8 @@ TEST(NodeTest, DumpToFdTest) { TemporaryFile dumptf; t.DumpToFd(dumptf.fd); fsync(dumptf.fd); - std::string buf(android::base::StringPrintf("test_dump\t%s\t1\tvalue1\n", - tf.path)); + std::string buf( + android::base::StringPrintf("test_dump\t%s\t1\tvalue1\n", tf.path)); _VerifyPathValue(dumptf.path, buf); } diff --git a/libperfmgr/tools/ConfigVerifier.cc b/libperfmgr/tools/ConfigVerifier.cc index 9fadd7d8..3b771a91 100644 --- a/libperfmgr/tools/ConfigVerifier.cc +++ b/libperfmgr/tools/ConfigVerifier.cc @@ -79,7 +79,7 @@ static void printUsage(const char* exec_name) { usage + " is a command-line tool to verify Nodes in Json config are writable.\n" "Usages:\n" - " [su system]" + + " [su system] " + exec_name + " [options]\n" "\n" -- cgit v1.2.3