MySQL Bugs: #97471: Function passing std::vector<> by value could be sightly improved

Bug #97471	Function passing std::vector<> by value could be sightly improved
Submitted:	4 Nov 2019 13:53	Modified:	7 Nov 2019 14:41
Reporter:	Xing Ai (OCA)	Email Updates:
Status:	Closed	Impact on me:	None
Category:	MySQL Server: Compiling	Severity:	S3 (Non-critical)
Version:	8.0	OS:	Any
Assigned to:		CPU Architecture:	Any

Description:
This problem is found through code review instead of testing.

Basically most of the server code functions pass "std::vector<>" by pointer or const reference. However the codes have a few cases that passing std::vector<> as value which might have slight performance problem.

How to repeat:
Below is the list of the codes that currently pass "std::vector<>" as value: 

https://github.com/mysql/mysql-server/blob/8.0/client/base/abstract_program.h
	Line 89:   virtual int execute(std::vector<std::string> positional_options) = 0;

https://github.com/mysql/mysql-server/blob/8.0/client/check/mysqlcheck.h
	Line 55:                         std::vector<std::string> arguments,
	Line 73:   int check_databases(MYSQL *connection, std::vector<std::string> databases);
	Line 81:   int upgrade_databases(MYSQL *connection, std::vector<std::string> databases);
	Line 133:   int execute(std::vector<std::string> positional_options);

https://github.com/mysql/mysql-server/blob/8.0/client/check/mysqlcheck_core.cc
	Line 63: static int process_databases(vector<string> db_names);
	Line 64: static int process_selected_tables(string db, vector<string> table_names);
	Line 91: static int process_databases(vector<string> db_names) {
	Line 100: static int process_selected_tables(string db, vector<string> table_names) {

https://github.com/mysql/mysql-server/blob/8.0/client/dump/mysqldump_tool_chain_maker_opti...
	Line 50:    void process_positional_options(std::vector<std::string> positional_options);

https://github.com/mysql/mysql-server/blob/8.0/client/dump/program.cc
	Line 142: int Program::execute(std::vector<std::string> positional_options) {

https://github.com/mysql/mysql-server/blob/8.0/client/dump/program.h
	Line 50:   int execute(std::vector<std::string> positional_options);

https://github.com/mysql/mysql-server/blob/8.0/client/upgrade/program.cc
	Line 88:   int execute(vector<string> positional_options MY_ATTRIBUTE((unused))) {

https://github.com/mysql/mysql-server/blob/8.0/plugin/x/client/xsession_impl.cc
	Line 967   Session_impl::validate_and_adjust_auth_methods(std::vector<Auth> auth_methods,

https://github.com/mysql/mysql-server/blob/8.0/plugin/x/client/xsession_impl.h
	Line 135:   std::vector<Auth> auth_methods, const bool can_use_plain);

https://github.com/mysql/mysql-server/blob/8.0/sql/auth/auth_common.h
	Line 944:                                      const std::vector<std::string> privs);
	Line 955:   Drop_temporary_dynamic_privileges(const std::vector<std::string> privs)

https://github.com/mysql/mysql-server/blob/8.0/sql/auth/sql_authorization.cc
	Line 7190:     const THD *thd, const std::vector<std::string> privs)
	
https://github.com/mysql/mysql-server/blob/8.0/storage/ndb/plugin/ndb_util_table.cc
	Line 171:     bool Ndb_util_table::check_primary_key(
					const std::vector<const char *> columns) const {

https://github.com/mysql/mysql-server/blob/8.0/storage/ndb/plugin/ndb_util_table.h
	Line 75:   bool check_primary_key(const std::vector<const char *> columns) const;

Suggested fix:
The list above might have chances to pass "std::vector<>" by const reference in functions, if not all of them is applicable.

Hi Mr. Ai,

Thank you for your bug report.

I think that you have a point here, but we need more info. Since you quote line numbers, we need the exact release that you are referring to. Next, can you check whether 8.0.18 has the same small deficiency ????

Next, I do not think that client side is critical, since, it is basically single threaded.

Lastly, could you point to any method in the server that does similar function as any of those server functions, which uses a pass-by-reference instead of pass-by-value ???

Hi and thanks for the bug report!

I have looked at the code and there are indeed some room for improvements here.

Thanks Sinisa and Jon for looking into the bug.

Sorry I am not sure what is the latest code tree of MySQL, but the links I gave should have the path and line information for those listed files even if you look at them in the latest code tree.

Most of the codes have good coding styles for passing std::vector<> as const reference or pointer for performance reason, but there are still a few cases left behind.

If you scan and review the server codes (Not including the extra or third parties codes), you will find many similar cases that pass "std::string" as value, which may need improve as well.

It is a minor effort to provide a full release of the server where these objects have been passed by value.

However, it would save us some time and would lead to the better and faster handling of this report.

Posted by developer:
 
Fixed in 8.0.19.

Code cleanup. No changelog entry required.

Thank you, Paul ......