Re-write sonic-db-cli with c++ for sonic startup performance issue#10491
Re-write sonic-db-cli with c++ for sonic startup performance issue#10491liuh-80 wants to merge 8 commits intosonic-net:masterfrom
Conversation
liuh-80
changed the title
| AUTOMAKE_OPTIONS = subdir-objects | ||
| SUBDIRS = tests | ||
|
|
||
| bin_PROGRAMS = sonic-db-cli |
There was a problem hiding this comment.
I will close this PR and send another PR for this.
| bool use_unix_socket) | ||
| { | ||
| std::unique_ptr<swss::SonicV2Connector_Native> dbconn; | ||
| if (name_space.compare("None") == 0) { |
There was a problem hiding this comment.
Will fix this in another PR.
| { | ||
| std::unique_ptr<swss::SonicV2Connector_Native> dbconn; | ||
| if (name_space.compare("None") == 0) { | ||
| dbconn = std::make_unique<swss::SonicV2Connector_Native>(use_unix_socket, empty_str); |
| bool use_unix_socket) | ||
| { | ||
| std::unique_ptr<swss::SonicV2Connector_Native> dbconn; | ||
| if (name_space.compare("None") == 0) { |
There was a problem hiding this comment.
Will fix in another PR.
| auto& client = dbconn->get_redis_client(db_name); | ||
|
|
||
| size_t argc = commands.size(); | ||
| const char** argv = new const char*[argc]; |
There was a problem hiding this comment.
Will fix in another PR.
|
|
||
| size_t argc = commands.size(); | ||
| const char** argv = new const char*[argc]; | ||
| size_t* argvc = new size_t[argc]; |
There was a problem hiding this comment.
Will fix in another PR.
| size_t* argvc = new size_t[argc]; | ||
| for (size_t i = 0; i < argc; i++) | ||
| { | ||
| argv[i] = strdup(commands[i].c_str()); |
There was a problem hiding this comment.
I will rewrite this method in another PR, because there are so many memory leak.
| switch(reply->type) | ||
| { | ||
| case REDIS_REPLY_INTEGER: | ||
| std::cout << reply->integer; |
There was a problem hiding this comment.
cout is little bit faster than printf: https://stackoverflow.com/questions/896654/cout-or-printf-which-of-the-two-has-a-faster-execution-speed-c
| } | ||
|
|
||
| void handleAllInstances( | ||
| const std::string& name_space, |
There was a problem hiding this comment.
Fixed, will submit in another PR, because this file will check-in to sonic-swss-common repo.
| void handleAllInstances( | ||
| const std::string& name_space, | ||
| const std::string& operation, | ||
| bool use_unix_socket) |
There was a problem hiding this comment.
If this is a pure-C++ application, not to used by SWIG/python at all, we prefer camelCaseName, and try follow existing code as much as possible.
For this parameter, please check isTcpConn.
This is a general comment applied to all the source code files, all variable names, all functions names.
There was a problem hiding this comment.
Fixed, will update in another PR.
| @@ -0,0 +1,15 @@ | |||
| #INCLUDES = -I $(top_srcdir) | |||
|
|
|||
| bin_PROGRAMS = tests | |||
There was a problem hiding this comment.
Discussed offline. Let's move testcase into sonic-swss-common repo.
You can use real redis instances in unit test.
You can use either googletest or pytest frameworks.
You can use sonic-db-cli as a process.
I am thinking about reuse the same set of testcases in order to test the behavior of old python version of sonic-db-cli. The co-test could last for a while until the python version is completely deprecated and removed from all applications.
There was a problem hiding this comment.
WIll also move sests to sonic-swss-common and submit in another PR.
|
Close this PR and create another PR in sonic-swss-common repo: sonic-net/sonic-swss-common#607 |
2 participants
Why I did it
How I did it
How to verify it
Which release branch to backport (provide reason below if selected)
Description for the changelog
Link to config_db schema for YANG module changes
A picture of a cute animal (not mandatory but encouraged)