Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Minor code optimizations found with the profiler #4442

Open
wants to merge 8 commits into
base: v2.x
Choose a base branch
from
Open

Minor code optimizations found with the profiler #4442

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5703802
Minor optimizations found with profiler
renecannao Jan 25, 2024
78fd49e
Minor code optimization
renecannao Jan 25, 2024
1d11701
Minor code optimizations
renecannao Jan 25, 2024
725027b
Fixed optimization in previous commit
renecannao Jan 25, 2024
b757c97
Removed several assert() from MySQL_Variables in non-debug build
renecannao Jan 25, 2024
eba540a
Remove redundant code from MySQL_Session about charsets check
renecannao Jan 25, 2024
d6690e8
Code optimizations in verify_set_names()
renecannao Jan 25, 2024
6b8a542
Removed file created by mistake
renecannao Jan 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions include/MySQL_Protocol.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ extern MySQL_Variables mysql_variables;

class MySQL_ResultSet {
private:
int nTrx;
bool deprecate_eof_active;
public:
bool transfer_started;
Expand Down
1 change: 1 addition & 0 deletions include/MySQL_Session.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,7 @@ class MySQL_Session
* maintenance thread. These values will be used to release the retained connections in the specific
* hostgroups in housekeeping operations, before client packet processing. Currently 'housekeeping_before_pkts'.
*/
size_t hgs_expired_conns_cnt = 0;
std::vector<int32_t> hgs_expired_conns {};
char * default_schema;
char * user_attributes;
Expand Down
11 changes: 9 additions & 2 deletions lib/MySQL_Protocol.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2631,6 +2631,7 @@ bool MySQL_Protocol::generate_COM_QUERY_from_COM_FIELD_LIST(PtrSize_t *pkt) {

MySQL_ResultSet::MySQL_ResultSet() {
buffer = NULL;
nTrx = -1;
//reset_pid = true;
}

Expand All @@ -2645,6 +2646,7 @@ void MySQL_ResultSet::buffer_init(MySQL_Protocol* myproto) {

void MySQL_ResultSet::init(MySQL_Protocol *_myprot, MYSQL_RES *_res, MYSQL *_my, MYSQL_STMT *_stmt) {
PROXY_TRACE2();
nTrx = -1;
transfer_started=false;
resultset_completed=false;
myprot=_myprot;
Expand Down Expand Up @@ -2707,7 +2709,9 @@ void MySQL_ResultSet::init(MySQL_Protocol *_myprot, MYSQL_RES *_res, MYSQL *_my,
deprecate_eof_active = c_myds->myconn && (c_myds->myconn->options.client_flag & CLIENT_DEPRECATE_EOF);

// first EOF
unsigned int nTrx=myds->sess->NumActiveTransactions();
if (nTrx == -1) { // not initialized yet
nTrx=myds->sess->NumActiveTransactions();
}
uint16_t setStatus = (nTrx ? SERVER_STATUS_IN_TRANS : 0 );
if (myds->sess->autocommit) setStatus += SERVER_STATUS_AUTOCOMMIT;
setStatus |= ( mysql->server_status & ~SERVER_STATUS_AUTOCOMMIT ); // get flags from server_status but ignore autocommit
Expand Down Expand Up @@ -2754,6 +2758,7 @@ void MySQL_ResultSet::remove_last_eof() {
void MySQL_ResultSet::init_with_stmt(MySQL_Connection *myconn) {
PROXY_TRACE2();
assert(stmt);
nTrx = -1;
MYSQL_STMT *_stmt = stmt;
MySQL_Data_Stream * c_myds = *(myprot->myds);
buffer_to_PSarrayOut();
Expand Down Expand Up @@ -2993,7 +2998,9 @@ unsigned int MySQL_ResultSet::add_row2(MYSQL_ROWS *row, unsigned char *offset) {

void MySQL_ResultSet::add_eof(bool suppress_warning_count) {
if (myprot) {
unsigned int nTrx=myds->sess->NumActiveTransactions();
if (nTrx == -1) { // not initialized yet
nTrx=myds->sess->NumActiveTransactions();
}
uint16_t setStatus = (nTrx ? SERVER_STATUS_IN_TRANS : 0 );
if (myds->sess->autocommit) setStatus += SERVER_STATUS_AUTOCOMMIT;
setStatus |= ( mysql->server_status & ~SERVER_STATUS_AUTOCOMMIT ); // get flags from server_status but ignore autocommit
Expand Down
54 changes: 42 additions & 12 deletions lib/MySQL_Session.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -770,6 +770,7 @@ void MySQL_Session::update_expired_conns(const vector<function<bool(MySQL_Connec
}
}
}
hgs_expired_conns_cnt = hgs_expired_conns.size();
}

MySQL_Backend * MySQL_Session::create_backend(int hostgroup_id, MySQL_Data_Stream *_myds) {
Expand Down Expand Up @@ -4837,6 +4838,7 @@ void MySQL_Session::housekeeping_before_pkts() {
if (hgs_expired_conns.empty() == false) {
hgs_expired_conns.clear();
}
hgs_expired_conns_cnt = hgs_expired_conns.size();
}
}

Expand Down Expand Up @@ -4884,7 +4886,9 @@ int MySQL_Session::handler() {
}
}

housekeeping_before_pkts();
if (unlikely(hgs_expired_conns_cnt != 0)) {
housekeeping_before_pkts();
}
handler_ret = get_pkts_from_client(wrong_pass, pkt);
if (handler_ret != 0) {
return handler_ret;
Expand Down Expand Up @@ -5001,19 +5005,24 @@ int MySQL_Session::handler() {
MySQL_Data_Stream *myds=mybe->server_myds;
MySQL_Connection *myconn=myds->myconn;
mybe->server_myds->max_connect_time=0;
MySQL_Connection *client_myconn = client_myds->myconn;
// we insert it in mypolls only if not already there
if (myds->mypolls==NULL) {
thread->mypolls.add(POLLIN|POLLOUT, mybe->server_myds->fd, mybe->server_myds, thread->curtime);
}
if (default_hostgroup>=0) {
if (handler_again___verify_backend_user_schema()) {
goto handler_again;
if (mybe->server_myds->DSS != STATE_MARIADB_QUERY) { // probably this should be: if (mybe->server_myds->DSS == STATE_READY || mybe->server_myds->DSS == STATE_MARIADB_GENERIC) {
if (handler_again___verify_backend_user_schema()) {
goto handler_again;
}
}
if (mirror==false) { // do not care about autocommit and charset if mirror
proxy_debug(PROXY_DEBUG_MYSQL_CONNECTION, 5, "Session %p , default_HG=%d server_myds DSS=%d , locked_on_HG=%d\n", this, default_hostgroup, mybe->server_myds->DSS, locked_on_hostgroup);
if (mybe->server_myds->DSS == STATE_READY || mybe->server_myds->DSS == STATE_MARIADB_GENERIC) {
if (handler_again___verify_init_connect()) {
goto handler_again;
if (unlikely(myconn->options.init_connect_sent==false)) { // micro-optimization. Perform this check outside handler_again___verify_init_connect()
if (handler_again___verify_init_connect()) {
goto handler_again;
}
}
if (use_ldap_auth) {
if (handler_again___verify_ldap_user_variable()) {
Expand All @@ -5025,8 +5034,10 @@ int MySQL_Session::handler() {
}
if (locked_on_hostgroup == -1 || locked_on_hostgroup_and_all_variables_set == false ) {

if (handler_again___verify_backend_multi_statement()) {
goto handler_again;
if (unlikely((client_myconn->options.client_flag & CLIENT_MULTI_STATEMENTS) != (myconn->options.client_flag & CLIENT_MULTI_STATEMENTS))) { // micro-optimization. Perform this check outside handler_again___verify_backend_multi_statement
if (handler_again___verify_backend_multi_statement()) {
goto handler_again;
}
}

if (handler_again___verify_backend_session_track_gtids()) {
Expand All @@ -5038,8 +5049,26 @@ int MySQL_Session::handler() {
goto handler_again;
}

/*
// Since all variables below SQL_NAME_LAST_LOW_WM
// are related to charset, this section of the code is
// redundant because verify_set_names() takes care of it.
bool SQL_NAME_LAST_LOW_WM_matches = true;
// this is a "fast" version of the extensive check that follows
for (auto i = 0; i < SQL_NAME_LAST_LOW_WM && SQL_NAME_LAST_LOW_WM_matches == true; i++) {
if (i == SQL_CHARACTER_ACTION) {
if (client_myconn->var_hash[i] != myconn->var_hash[i]) {
SQL_NAME_LAST_LOW_WM_matches = false;
}
}
}
if (SQL_NAME_LAST_LOW_WM_matches == false) {
for (auto i = 0; i < SQL_NAME_LAST_LOW_WM; i++) {
auto client_hash = client_myds->myconn->var_hash[i];
auto client_hash = client_myconn->var_hash[i];
if (i == SQL_CHARACTER_ACTION) {
// SQL_CHARACTER_ACTION has no meaning for the backend
continue;
}
#ifdef DEBUG
if (GloVars.global.gdbg) {
switch (i) {
Expand All @@ -5064,11 +5093,12 @@ int MySQL_Session::handler() {
}
}
}
MySQL_Connection *c_con = client_myds->myconn;
vector<uint32_t>::const_iterator it_c = c_con->dynamic_variables_idx.begin(); // client connection iterator
for ( ; it_c != c_con->dynamic_variables_idx.end() ; it_c++) {
}
*/
vector<uint32_t>::const_iterator it_c = client_myconn->dynamic_variables_idx.begin(); // client connection iterator
for ( ; it_c != client_myconn->dynamic_variables_idx.end() ; it_c++) {
auto i = *it_c;
auto client_hash = c_con->var_hash[i];
auto client_hash = client_myconn->var_hash[i];
auto server_hash = myconn->var_hash[i];
if (client_hash != server_hash) {
if(
Expand Down
64 changes: 55 additions & 9 deletions lib/MySQL_Variables.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,24 +212,30 @@ bool MySQL_Variables::client_set_value(MySQL_Session* session, int idx, const st
}

const char* MySQL_Variables::client_get_value(MySQL_Session* session, int idx) const {
#ifdef DEBUG
assert(session);
assert(session->client_myds);
assert(session->client_myds->myconn);
#endif // DEBUG
return session->client_myds->myconn->variables[idx].value;
}

uint32_t MySQL_Variables::client_get_hash(MySQL_Session* session, int idx) const {
#ifdef DEBUG
assert(session);
assert(session->client_myds);
assert(session->client_myds->myconn);
#endif // DEBUG
return session->client_myds->myconn->var_hash[idx];
}

void MySQL_Variables::server_set_value(MySQL_Session* session, int idx, const char* value) {
#ifdef DEBUG
assert(session);
assert(session->mybe);
assert(session->mybe->server_myds);
assert(session->mybe->server_myds->myconn);
#endif // DEBUG
if (!value) return; // FIXME: I am not sure about this implementation . If value == NULL , show the variable be reset?
session->mybe->server_myds->myconn->var_hash[idx] = SpookyHash::Hash32(value,strlen(value),10);

Expand All @@ -242,10 +248,12 @@ void MySQL_Variables::server_set_value(MySQL_Session* session, int idx, const ch
}

void MySQL_Variables::server_reset_value(MySQL_Session* session, int idx) {
#ifdef DEBUG
assert(session);
assert(session->mybe);
assert(session->mybe->server_myds);
assert(session->mybe->server_myds->myconn);
#endif // DEBUG

MySQL_Connection *backend_conn = session->mybe->server_myds->myconn;

Expand All @@ -261,18 +269,22 @@ void MySQL_Variables::server_reset_value(MySQL_Session* session, int idx) {
}

const char* MySQL_Variables::server_get_value(MySQL_Session* session, int idx) const {
#ifdef DEBUG
assert(session);
assert(session->mybe);
assert(session->mybe->server_myds);
assert(session->mybe->server_myds->myconn);
#endif // DEBUG
return session->mybe->server_myds->myconn->variables[idx].value;
}

uint32_t MySQL_Variables::server_get_hash(MySQL_Session* session, int idx) const {
#ifdef DEBUG
assert(session);
assert(session->mybe);
assert(session->mybe->server_myds);
assert(session->mybe->server_myds->myconn);
#endif // DEBUG
return session->mybe->server_myds->myconn->var_hash[idx];
}

Expand Down Expand Up @@ -470,27 +482,61 @@ bool update_server_variable(MySQL_Session* session, int idx, int &_rc) {
}

bool verify_set_names(MySQL_Session* session) {
uint32_t client_charset_hash = mysql_variables.client_get_hash(session, SQL_CHARACTER_SET_CLIENT);
MySQL_Connection *client_myconn = session->client_myds->myconn;
uint32_t client_charset_hash = client_myconn->var_hash[SQL_CHARACTER_SET_CLIENT];
#ifdef DEBUG
// client_get_hash() should return the same result
uint32_t client_charset_hash_2 = mysql_variables.client_get_hash(session, SQL_CHARACTER_SET_CLIENT);
assert(client_charset_hash == client_charset_hash_2);
#endif // DEBUG
if (client_charset_hash == 0)
return false;

uint32_t results_charset_hash = mysql_variables.client_get_hash(session, SQL_CHARACTER_SET_RESULTS);
uint32_t results_charset_hash = client_myconn->var_hash[SQL_CHARACTER_SET_RESULTS];
#ifdef DEBUG
// client_get_hash() should return the same result
uint32_t results_charset_hash_2 = mysql_variables.client_get_hash(session, SQL_CHARACTER_SET_RESULTS);
assert(results_charset_hash == results_charset_hash_2);
#endif // DEBUG
if (client_charset_hash != results_charset_hash)
return false;

uint32_t connection_charset_hash = mysql_variables.client_get_hash(session, SQL_CHARACTER_SET_CONNECTION);
uint32_t connection_charset_hash = client_myconn->var_hash[SQL_CHARACTER_SET_CONNECTION];
#ifdef DEBUG
// client_get_hash() should return the same result
uint32_t connection_charset_hash_2 = mysql_variables.client_get_hash(session, SQL_CHARACTER_SET_CONNECTION);
assert(connection_charset_hash == connection_charset_hash_2);
#endif // DEBUG
if (client_charset_hash != connection_charset_hash)
return false;

uint32_t collation_hash = mysql_variables.client_get_hash(session, SQL_COLLATION_CONNECTION);
uint32_t collation_hash = client_myconn->var_hash[SQL_COLLATION_CONNECTION];
#ifdef DEBUG
// client_get_hash() should return the same result
uint32_t collation_hash_2 = mysql_variables.client_get_hash(session, SQL_COLLATION_CONNECTION);
assert(collation_hash == collation_hash_2);
#endif // DEBUG
if (client_charset_hash != collation_hash)
return false;

if (client_charset_hash != mysql_variables.server_get_hash(session, SQL_CHARACTER_SET_CLIENT) ||
results_charset_hash != mysql_variables.server_get_hash(session, SQL_CHARACTER_SET_RESULTS) ||
connection_charset_hash != mysql_variables.server_get_hash(session, SQL_CHARACTER_SET_CONNECTION) ||
collation_hash != mysql_variables.server_get_hash(session, SQL_COLLATION_CONNECTION)) {

bool variables_match = true;
MySQL_Connection *server_myconn = session->mybe->server_myds->myconn;
if (
client_charset_hash != server_myconn->var_hash[SQL_CHARACTER_SET_CLIENT] ||
results_charset_hash != server_myconn->var_hash[SQL_CHARACTER_SET_RESULTS] ||
connection_charset_hash != server_myconn->var_hash[SQL_CHARACTER_SET_CONNECTION] ||
collation_hash != server_myconn->var_hash[SQL_COLLATION_CONNECTION]
) {
variables_match = false;
}
#ifdef DEBUG
// server_get_hash() should return the same result
assert(mysql_variables.server_get_hash(session, SQL_CHARACTER_SET_CLIENT) == server_myconn->var_hash[SQL_CHARACTER_SET_CLIENT]);
assert(mysql_variables.server_get_hash(session, SQL_CHARACTER_SET_RESULTS) == server_myconn->var_hash[SQL_CHARACTER_SET_RESULTS]);
assert(mysql_variables.server_get_hash(session, SQL_CHARACTER_SET_CONNECTION) == server_myconn->var_hash[SQL_CHARACTER_SET_CONNECTION]);
assert(mysql_variables.server_get_hash(session, SQL_COLLATION_CONNECTION) == server_myconn->var_hash[SQL_COLLATION_CONNECTION]);
#endif // DEBUG
if (variables_match == false) {
switch(session->status) { // this switch can be replaced with a simple previous_status.push(status), but it is here for readibility
case PROCESSING_QUERY:
session->previous_status.push(PROCESSING_QUERY);
Expand Down
Loading