| Bug #49501 | Inefficient information_schema check (system collation) | ||
|---|---|---|---|
| Submitted: | 7 Dec 2009 10:53 | Modified: | 12 Mar 2010 16:35 |
| Reporter: | Domas Mituzas | Email Updates: | |
| Status: | Closed | Impact on me: | |
| Category: | MySQL Server: Charsets | Severity: | S5 (Performance) |
| Version: | 5.1.43-bzr | OS: | Any |
| Assigned to: | Sergei Glukhov | CPU Architecture: | Any |
[7 Dec 2009 10:53]
Domas Mituzas
[8 Dec 2009 17:18]
Valeriy Kravchuk
Thank you for the problem report.
[11 Dec 2009 8:28]
Alexander Barkov
This piece of code can be optimized by a new condition, using length:
if (new_db_name->length == INFORMATION_SCHEMA_NAME.length &&
my_strcasecmp(system_charset_info, new_db_name->str,
INFORMATION_SCHEMA_NAME.str) == 0)
so in most cases my_strcasecmp() will even not be executed.
Quick search using "grep my_stecasecmp | grep INFORMATION_SCHEMA"
discovers three pieces where this optimization can be done:
mysql_priv.h:
#define is_schema_db(X) \
!my_strcasecmp(system_charset_info, INFORMATION_SCHEMA_NAME.str, (X))
sql_db.cc: if (!my_strcasecmp(system_charset_info, db, INFORMATION_SCHEMA_NAME.str))
sql_show.cc: if (!my_strcasecmp(system_charset_info, INFORMATION_SCHEMA_NAME.str,
[15 Dec 2009 11:12]
Bugs System
A patch for this bug has been committed. After review, it may be pushed to the relevant source trees for release in the next version. You can access the patch from: http://lists.mysql.com/commits/94104 3273 Sergey Glukhov 2009-12-15 Bug#49501 Inefficient information_schema check (system collation) added check_length optimization for I_S_NAME comparison @ sql/event_data_objects.cc added check_length optimization for I_S_NAME comparison @ sql/events.cc added check_length optimization for I_S_NAME comparison @ sql/mysql_priv.h added check_length optimization for I_S_NAME comparison @ sql/repl_failsafe.cc added check_length optimization for I_S_NAME comparison @ sql/sql_db.cc added check_length optimization for I_S_NAME comparison @ sql/sql_parse.cc added check_length optimization for I_S_NAME comparison @ sql/sql_show.cc added check_length optimization for I_S_NAME comparison @ sql/sql_view.cc added check_length optimization for I_S_NAME comparison @ sql/table.cc added check_length optimization for I_S_NAME comparison
[15 Dec 2009 11:33]
Alexander Barkov
http://lists.mysql.com/commits/94104 is Ok to push. But I'd also perhaps add an overloaded function: inline bool is_schema_db(const LEX_STR *str); because it is exactly LEX_STR which is tested in most of the cases.
[15 Dec 2009 11:34]
Alexander Barkov
or even: inline bool is_schema_db(const LEX_STR &);
[15 Dec 2009 15:50]
Mark Callaghan
Can we continue the discussion on this topic?
"!my_strcasecmp" can be found throughout the code. Will all of those calls be faster if there is a check for string lengths prior to doing the comparison? Alas, some people might use 'my_strcasecmp(...) == 0' making the search harder. I wish that style was not used.
A similar optimization opportunity exists in sql_string.cc in the stringcmp function. This is used in several places where the caller checks for equality not for order. If a stringeq method where added it could optimize for this check by first checking string length.
int stringcmp(const String *s,const String *t)
{
uint32 s_len=s->length(),t_len=t->length(),len=min(s_len,t_len);
int cmp= memcmp(s->ptr(), t->ptr(), len);
return (cmp) ? cmp : (int) (s_len - t_len);
}
callers to stringcmp that really want equality checks, from cscope for 5.0.84
item.cc eq 874 return !stringcmp(&str_value, &item->str_value);
item.cc eq 3117 return !stringcmp(&str_value, &item->str_value);
item.cc eq 5045 return !stringcmp(&str_value, &arg->str_value);
item.cc field_is_equal_to_item
6486 return !stringcmp(&field_tmp,item_result);
[19 Jan 2010 8:13]
Alexander Barkov
Hi Mark, thank you very much for your suggestion about the further improvements way. We cannot do all these improvements in MySQL-5.1 under the terms of this bug report. So we created a new WorkLog task: WL#5230 Replace strcasecmp() used for checking equality to a more efficient function It should appear soon under this location: http://forge.mysql.com/worklog/task.php?id=5230 Thanks!
[19 Jan 2010 9:04]
Bugs System
A patch for this bug has been committed. After review, it may be pushed to the relevant source trees for release in the next version. You can access the patch from: http://lists.mysql.com/commits/97331 3320 Sergey Glukhov 2010-01-19 Bug#49501 Inefficient information_schema check (system collation) added check_length optimization for I_S_NAME comparison @ sql/event_data_objects.cc added check_length optimization for I_S_NAME comparison @ sql/events.cc added check_length optimization for I_S_NAME comparison @ sql/mysql_priv.h added check_length optimization for I_S_NAME comparison @ sql/repl_failsafe.cc added check_length optimization for I_S_NAME comparison @ sql/sql_db.cc added check_length optimization for I_S_NAME comparison @ sql/sql_parse.cc added check_length optimization for I_S_NAME comparison @ sql/sql_show.cc added check_length optimization for I_S_NAME comparison @ sql/sql_view.cc added check_length optimization for I_S_NAME comparison @ sql/table.cc added check_length optimization for I_S_NAME comparison
[19 Jan 2010 17:21]
Mark Callaghan
Did you confirm that this change improves performance? The change made here is much more substantial than the change suggested by Domas and may result in many more calls to strlen. The change suggested by Domas required no additional calls to strlen. The problem reported by Domas requires changes to mysql_change_db() and add_table_to_list(). In both cases, the length of both strings is known in the check for equality with "information_schema".
[21 Jan 2010 8:17]
Bugs System
A patch for this bug has been committed. After review, it may be pushed to the relevant source trees for release in the next version. You can access the patch from: http://lists.mysql.com/commits/97684 3329 Sergey Glukhov 2010-01-21 Bug#49501 Inefficient information_schema check (system collation), addon removed wrongly introduced strlen calls @ sql/events.cc removed wrongly introduced strlen calls @ sql/mysql_priv.h removed wrongly introduced strlen calls @ sql/repl_failsafe.cc removed wrongly introduced strlen calls @ sql/sql_db.cc removed wrongly introduced strlen calls @ sql/sql_parse.cc removed wrongly introduced strlen calls @ sql/sql_show.cc removed wrongly introduced strlen calls
[22 Jan 2010 10:01]
Alexander Barkov
The patch http://lists.mysql.com/commits/97684 removing newly introduced strlen() calls looks ok. So now as a result we have two overloaded functions: is_schema_db(const char *name); (for the cases when length is not know) and is_schema_db(const char *name, size_t length) (for the case when length is known). The second version is about 3 times faster. In the future we will do refactoring (as Sinisa suggests): "WL#2894 change strlen() to LEX_STRING or String" to make length always available and thus get rid of the slower version.
[22 Jan 2010 10:03]
Alexander Barkov
Mark,
> Did you confirm that this change improves performance?
Yes, according to our tests:
is_schema_db("twiki", 5);
is 3 times faster than
is_schema_db("twiki");
Thank you very much for helping us!
[22 Jan 2010 10:59]
Bugs System
A patch for this bug has been committed. After review, it may be pushed to the relevant source trees for release in the next version. You can access the patch from: http://lists.mysql.com/commits/97841 3331 Sergey Glukhov 2010-01-22 Bug#49501 Inefficient information_schema check (system collation), addon removed wrongly introduced strlen calls @ sql/events.cc removed wrongly introduced strlen calls @ sql/mysql_priv.h removed wrongly introduced strlen calls @ sql/repl_failsafe.cc removed wrongly introduced strlen calls @ sql/sql_db.cc removed wrongly introduced strlen calls @ sql/sql_parse.cc removed wrongly introduced strlen calls @ sql/sql_show.cc removed wrongly introduced strlen calls
[4 Feb 2010 10:19]
Bugs System
Pushed into 5.1.44 (revid:joro@sun.com-20100204101444-2j32mhqroo0iiio6) (version source revid:dao-gang.qu@sun.com-20100125025505-zqa9v2mgdcfza0v6) (merge vers: 5.1.43) (pib:16)
[5 Feb 2010 11:48]
Bugs System
Pushed into mysql-next-mr (revid:alik@sun.com-20100204063540-9czpdmpixi3iw2yb) (version source revid:alik@sun.com-20100130192433-9fckdf4o5xc37flw) (pib:16)
[5 Feb 2010 11:55]
Bugs System
Pushed into 6.0.14-alpha (revid:alik@sun.com-20100205113942-oqovjy0eoqbarn7i) (version source revid:alik@sun.com-20100204064210-ljwanqvrjs83s1gq) (merge vers: 6.0.14-alpha) (pib:16)
[5 Feb 2010 12:00]
Bugs System
Pushed into 5.5.2-m2 (revid:alik@sun.com-20100203172258-1n5dsotny40yufxw) (version source revid:alexey.kopytov@sun.com-20100123210923-lx4o1ettww9fdkqk) (merge vers: 5.5.2-m2) (pib:16)
[11 Feb 2010 13:49]
Paul DuBois
Noted in 5.1.44, 5.5.2, 6.0.14 changelogs. The method for comparing INFORMATION_SCHEMA names and database names was nonoptimal and an improvement was made: When the database name length is already known, a length check is made first and content comparison skipped if the lengths are unequal. Setting report to Need Merge pending push to Celosia.
[12 Mar 2010 14:08]
Bugs System
Pushed into 5.1.44-ndb-7.0.14 (revid:jonas@mysql.com-20100312135944-t0z8s1da2orvl66x) (version source revid:jonas@mysql.com-20100312115609-woou0te4a6s4ae9y) (merge vers: 5.1.44-ndb-7.0.14) (pib:16)
[12 Mar 2010 14:24]
Bugs System
Pushed into 5.1.44-ndb-6.2.19 (revid:jonas@mysql.com-20100312134846-tuqhd9w3tv4xgl3d) (version source revid:jonas@mysql.com-20100312060623-mx6407w2vx76h3by) (merge vers: 5.1.44-ndb-6.2.19) (pib:16)
[12 Mar 2010 14:38]
Bugs System
Pushed into 5.1.44-ndb-6.3.33 (revid:jonas@mysql.com-20100312135724-xcw8vw2lu3mijrhn) (version source revid:jonas@mysql.com-20100312103652-snkltsd197l7q2yg) (merge vers: 5.1.44-ndb-6.3.33) (pib:16)
[12 Mar 2010 16:35]
Paul DuBois
Fixed in earlier 5.1.x, 5.5.x.
