#At file:///home/dlenev/src/bzr/mysql-trunk-bug27480-2/ based on revid:dmitry.lenev@stripped
3534 Dmitry Lenev 2011-03-31
Yet another follow-up for the patch for Bug#11746602
27480: Extend CREATE TEMPORARY TABLES privilege to
allow temp table operations).
This patch tries to eliminate performance overhead which
was introduced by one of previous patches for this bug.
One of those patches added pre-opening of temporary tables as
a separate stage of statement execution, handled by a separate
call to open_temporary_tables(). As result we started to
construct table definition key twice per statement almost for
each table to be open by statement.
This patch addresses the issue by changing code to avoid
construction of keys and borrowing pre-constructed key from
TABLE_LIST::mdl_request::key instead, where it is possible.
@ sql/sql_admin.cc
Adjusted prepare_for_repair() code to use get_table_def_key()
instead of create_table_def_key() to get table definition
cache key (the former piggy-backs on pre-constructed
TABLE_LIST::mdl_request::key instead of constructing new
instance of key like the latter).
@ sql/sql_base.cc
- Introduced get_table_def_key() function which allows to
get table definition cache key for the table list element
cheaply, by piggy-backing on pre-constructed
TABLE_LIST::mdl_request::key.
Adjusted code to use this function instead of
create_table_def_key(), which constructs new instance of
key, where possible.
- Made create_table_def_key() private to sql_base.cc as it
is no longer used outside of this file. Also changed it to
accept database and table name instead of table list element
since now it is only used in cases when table list element
is not easily available.
Changed some code which specifically constructed temporary
TABLE_LIST objects to call create_table_def_key() to not
to do this.
- Changed find_temporary_table(TABLE_LIST) to use key borrowed
from TABLE_LIST::mdl_request::key instead of especially
constructed key. This change allows us to save on key
construction but adds a small overhead during key comparison.
Such a trade-off should be acceptable as overhead during key
comparison is only a few instructions.
- Signatures of get_table_share/get_table_share_with_discover()
and tdc_open_view() were changed to accept constant table
definition key to allow them to be used in cases when keys
were borrowed from MDL subsystem.
@ sql/sql_base.h
- Introduced get_table_def_key() function which allows to
get table definition cache key for the table list element
cheaply, by piggy-backing on pre-constructed
TABLE_LIST::mdl_request::key.
key, where possible.
- Made create_table_def_key() private to sql_base.cc as it
is no longer used outside of this file.
- Signatures of get_table_share/get_table_share_with_discover()
and tdc_open_view() were changed to accept constant table
definition key to allow them to be used in cases when keys
were borrowed from MDL subsystem.
@ sql/sql_show.cc
Adjusted fill_schema_table_from_frm() code to use
get_table_def_key() instead of create_table_def_key() to get
table definition cache key (the former piggy-backs on
pre-constructed TABLE_LIST::mdl_request::key instead of
constructing new instance of key like the latter).
@ sql/sql_table.cc
Changed two places in mysql_alter_table() code where we tried
to open tables using table list element objects which were not
fully/correctly initialized and which, therefore, were impossible
to use to get table definition cache key from them.
@ sql/sql_view.cc
Adjusted fill_defined_view_parts() code to use
get_table_def_key() instead of create_table_def_key() to
get table definition cache key (the former piggy-backs on
pre-constructed TABLE_LIST::mdl_request::key instead of
constructing new instance of key like the latter).
@ sql/table.cc
Changed signature of alloc_table_share() to accept constant
table definition key to allow it to be used in cases when
key is borrowed from MDL subsystem.
@ sql/table.h
Changed signature of alloc_table_share() to accept constant
table definition key to allow it to be used in cases when
key is borrowed from MDL subsystem.
modified:
sql/sql_admin.cc
sql/sql_base.cc
sql/sql_base.h
sql/sql_show.cc
sql/sql_table.cc
sql/sql_view.cc
sql/table.cc
sql/table.h
=== modified file 'sql/sql_admin.cc'
--- a/sql/sql_admin.cc 2011-03-09 12:24:36 +0000
+++ b/sql/sql_admin.cc 2011-03-31 06:39:38 +0000
@@ -65,7 +65,7 @@ static int prepare_for_repair(THD *thd,
if (!(table= table_list->table))
{
- char key[MAX_DBKEY_LENGTH];
+ const char *key;
uint key_length;
/*
If the table didn't exist, we have a shared metadata lock
@@ -81,7 +81,6 @@ static int prepare_for_repair(THD *thd,
*/
my_hash_value_type hash_value;
- key_length= create_table_def_key(thd, key, table_list, 0);
table_list->mdl_request.init(MDL_key::TABLE,
table_list->db, table_list->table_name,
MDL_EXCLUSIVE, MDL_TRANSACTION);
@@ -91,6 +90,8 @@ static int prepare_for_repair(THD *thd,
DBUG_RETURN(0);
has_mdl_lock= TRUE;
+ key_length= get_table_def_key(table_list, &key);
+
hash_value= my_calc_hash(&table_def_cache, (uchar*) key, key_length);
mysql_mutex_lock(&LOCK_open);
share= get_table_share(thd, table_list, key, key_length, 0,
=== modified file 'sql/sql_base.cc'
--- a/sql/sql_base.cc 2011-03-26 10:56:27 +0000
+++ b/sql/sql_base.cc 2011-03-31 06:39:38 +0000
@@ -222,17 +222,16 @@ static void check_unused(void)
#endif
-/*
+/**
Create a table cache key
- SYNOPSIS
- create_table_def_key()
- thd Thread handler
- key Create key here (must be of size MAX_DBKEY_LENGTH)
- table_list Table definition
- tmp_table Set if table is a tmp table
+ @param thd Thread context
+ @param key Create key here (must be of size MAX_DBKEY_LENGTH)
+ @param db Database name.
+ @param table_name Table name.
+ @param tmp_table Set if table is a tmp table.
- IMPLEMENTATION
+ @note
The table cache_key is created from:
db_name + \0
table_name + \0
@@ -243,16 +242,14 @@ static void check_unused(void)
4 bytes for master thread id
4 bytes pseudo thread id
- RETURN
- Length of key
+ @return Length of key.
*/
-uint create_table_def_key(THD *thd, char *key,
- const TABLE_LIST *table_list,
- bool tmp_table)
+static uint create_table_def_key(THD *thd, char *key,
+ const char *db, const char *table_name,
+ bool tmp_table)
{
- uint key_length= (uint) (strmov(strmov(key, table_list->db)+1,
- table_list->table_name)-key)+1;
+ uint key_length= (uint) (strmov(strmov(key, db)+1, table_name)-key)+1;
if (tmp_table)
{
int4store(key + key_length, thd->server_id);
@@ -263,6 +260,41 @@ uint create_table_def_key(THD *thd, char
}
+/**
+ Get table cache key for a table list element.
+
+ @param table_list[in] Table list element.
+ @param key[out] On return points to table cache key for the table.
+
+ @note Unlike create_table_def_key() call this function doesn't construct
+ key in a buffer provider by caller. Instead it relies on the fact
+ that table list element for which key is requested has properly
+ initialized MDL_request object and the fact that table defition
+ cache key is suffix of key used in MDL subsystem. So to get table
+ definition key it simply needs to return pointer to appropriate
+ part of MDL_key object nested in this table list element.
+ Indeed, this means that lifetime of key produced by this call is
+ limited by the lifetime of table list element which it got as
+ parameter.
+
+ @return Length of key.
+*/
+
+uint get_table_def_key(const TABLE_LIST *table_list, const char **key)
+{
+ /*
+ This call relies on the fact that TABLE_LIST::mdl_request::key object
+ is properly initialized, so table definition cache can be produced
+ from key used by MDL subsystem.
+ */
+ DBUG_ASSERT(!strcmp(table_list->db, table_list->mdl_request.key.db_name()) &&
+ !strcmp(table_list->table_name, table_list->mdl_request.key.name()));
+
+ *key= (const char*)table_list->mdl_request.key.ptr() + 1;
+ return table_list->mdl_request.key.length() - 1;
+}
+
+
/*****************************************************************************
Functions to handle table definition cach (TABLE_SHARE)
@@ -494,8 +526,9 @@ static void table_def_unuse_table(TABLE
# Share for table
*/
-TABLE_SHARE *get_table_share(THD *thd, TABLE_LIST *table_list, char *key,
- uint key_length, uint db_flags, int *error,
+TABLE_SHARE *get_table_share(THD *thd, TABLE_LIST *table_list,
+ const char *key, uint key_length,
+ uint db_flags, int *error,
my_hash_value_type hash_value)
{
TABLE_SHARE *share;
@@ -610,7 +643,7 @@ found:
static TABLE_SHARE *
get_table_share_with_discover(THD *thd, TABLE_LIST *table_list,
- char *key, uint key_length,
+ const char *key, uint key_length,
uint db_flags, int *error,
my_hash_value_type hash_value)
@@ -755,14 +788,11 @@ void release_table_share(TABLE_SHARE *sh
TABLE_SHARE *get_cached_table_share(const char *db, const char *table_name)
{
- char key[NAME_LEN*2+2];
- TABLE_LIST table_list;
+ char key[MAX_DBKEY_LENGTH];
uint key_length;
mysql_mutex_assert_owner(&LOCK_open);
- table_list.db= (char*) db;
- table_list.table_name= (char*) table_name;
- key_length= create_table_def_key((THD*) 0, key, &table_list, 0);
+ key_length= create_table_def_key((THD*) 0, key, db, table_name, 0);
return (TABLE_SHARE*) my_hash_search(&table_def_cache,
(uchar*) key, key_length);
}
@@ -1990,12 +2020,9 @@ void update_non_unique_table_error(TABLE
TABLE *find_temporary_table(THD *thd, const char *db, const char *table_name)
{
- TABLE_LIST tl;
-
- tl.db= (char*) db;
- tl.table_name= (char*) table_name;
-
- return find_temporary_table(thd, &tl);
+ char key[MAX_DBKEY_LENGTH];
+ uint key_length= create_table_def_key(thd, key, db, table_name, 1);
+ return find_temporary_table(thd, key, key_length);
}
@@ -2008,10 +2035,25 @@ TABLE *find_temporary_table(THD *thd, co
TABLE *find_temporary_table(THD *thd, const TABLE_LIST *tl)
{
- char key[MAX_DBKEY_LENGTH];
- uint key_length= create_table_def_key(thd, key, tl, 1);
+ const char *key;
+ uint key_length;
+ char key_suffix[TMP_TABLE_KEY_EXTRA];
+ TABLE *table;
- return find_temporary_table(thd, key, key_length);
+ key_length= get_table_def_key(tl, &key);
+
+ int4store(key_suffix, thd->server_id);
+ int4store(key_suffix + 4, thd->variables.pseudo_thread_id);
+
+ for (table= thd->temporary_tables; table; table= table->next)
+ {
+ if ((table->s->table_cache_key.length == key_length+TMP_TABLE_KEY_EXTRA) &&
+ !memcmp(table->s->table_cache_key.str, key, key_length) &&
+ !memcmp(table->s->table_cache_key.str + key_length, key_suffix,
+ TMP_TABLE_KEY_EXTRA))
+ return table;
+ }
+ return NULL;
}
@@ -2186,15 +2228,12 @@ bool rename_temporary_table(THD* thd, TA
char *key;
uint key_length;
TABLE_SHARE *share= table->s;
- TABLE_LIST table_list;
DBUG_ENTER("rename_temporary_table");
if (!(key=(char*) alloc_root(&share->mem_root, MAX_DBKEY_LENGTH)))
DBUG_RETURN(1); /* purecov: inspected */
- table_list.db= (char*) db;
- table_list.table_name= (char*) table_name;
- key_length= create_table_def_key(thd, key, &table_list, 1);
+ key_length= create_table_def_key(thd, key, db, table_name, 1);
share->set_table_cache_key(key, key_length);
DBUG_RETURN(0);
}
@@ -2600,8 +2639,8 @@ bool open_table(THD *thd, TABLE_LIST *ta
Open_table_context *ot_ctx)
{
reg1 TABLE *table;
- char key[MAX_DBKEY_LENGTH];
- uint key_length;
+ const char *key;
+ uint key_length;
char *alias= table_list->alias;
uint flags= ot_ctx->get_flags();
MDL_ticket *mdl_ticket;
@@ -2625,7 +2664,7 @@ bool open_table(THD *thd, TABLE_LIST *ta
if (thd->killed)
DBUG_RETURN(TRUE);
- key_length= create_table_def_key(thd, key, table_list, 0);
+ key_length= get_table_def_key(table_list, &key);
/*
If we're in pre-locked or LOCK TABLES mode, let's try to find the
@@ -3664,7 +3703,7 @@ check_and_update_routine_version(THD *th
*/
bool tdc_open_view(THD *thd, TABLE_LIST *table_list, const char *alias,
- char *cache_key, uint cache_key_length,
+ const char *cache_key, uint cache_key_length,
MEM_ROOT *mem_root, uint flags)
{
TABLE not_used;
@@ -3765,15 +3804,15 @@ static bool open_table_entry_fini(THD *t
static bool auto_repair_table(THD *thd, TABLE_LIST *table_list)
{
- char cache_key[MAX_DBKEY_LENGTH];
- uint cache_key_length;
+ const char *cache_key;
+ uint cache_key_length;
TABLE_SHARE *share;
TABLE *entry;
int not_used;
bool result= TRUE;
my_hash_value_type hash_value;
- cache_key_length= create_table_def_key(thd, cache_key, table_list, 0);
+ cache_key_length= get_table_def_key(table_list, &cache_key);
thd->clear_error();
@@ -5883,7 +5922,6 @@ TABLE *open_table_uncached(THD *thd, con
TABLE_SHARE *share;
char cache_key[MAX_DBKEY_LENGTH], *saved_cache_key, *tmp_path;
uint key_length;
- TABLE_LIST table_list;
DBUG_ENTER("open_table_uncached");
DBUG_PRINT("enter",
("table: '%s'.'%s' path: '%s' server_id: %u "
@@ -5891,10 +5929,8 @@ TABLE *open_table_uncached(THD *thd, con
db, table_name, path,
(uint) thd->server_id, (ulong) thd->variables.pseudo_thread_id));
- table_list.db= (char*) db;
- table_list.table_name= (char*) table_name;
/* Create the cache_key for temporary tables */
- key_length= create_table_def_key(thd, cache_key, &table_list, 1);
+ key_length= create_table_def_key(thd, cache_key, db, table_name, 1);
if (!(tmp_table= (TABLE*) my_malloc(sizeof(*tmp_table) + sizeof(*share) +
strlen(path)+1 + key_length,
=== modified file 'sql/sql_base.h'
--- a/sql/sql_base.h 2011-03-26 10:56:27 +0000
+++ b/sql/sql_base.h 2011-03-31 06:39:38 +0000
@@ -78,11 +78,10 @@ void table_def_start_shutdown(void);
void assign_new_table_id(TABLE_SHARE *share);
uint cached_open_tables(void);
uint cached_table_definitions(void);
-uint create_table_def_key(THD *thd, char *key,
- const TABLE_LIST *table_list,
- bool tmp_table);
-TABLE_SHARE *get_table_share(THD *thd, TABLE_LIST *table_list, char *key,
- uint key_length, uint db_flags, int *error,
+uint get_table_def_key(const TABLE_LIST *table_list, const char **key);
+TABLE_SHARE *get_table_share(THD *thd, TABLE_LIST *table_list,
+ const char *key, uint key_length,
+ uint db_flags, int *error,
my_hash_value_type hash_value);
void release_table_share(TABLE_SHARE *share);
TABLE_SHARE *get_cached_table_share(const char *db, const char *table_name);
@@ -294,7 +293,7 @@ void tdc_remove_table(THD *thd, enum_tdc
const char *db, const char *table_name,
bool has_lock);
bool tdc_open_view(THD *thd, TABLE_LIST *table_list, const char *alias,
- char *cache_key, uint cache_key_length,
+ const char *cache_key, uint cache_key_length,
MEM_ROOT *mem_root, uint flags);
void tdc_flush_unused_tables();
TABLE *find_table_for_mdl_upgrade(THD *thd, const char *db,
=== modified file 'sql/sql_show.cc'
--- a/sql/sql_show.cc 2011-03-09 12:24:36 +0000
+++ b/sql/sql_show.cc 2011-03-31 06:39:38 +0000
@@ -3272,7 +3272,7 @@ static int fill_schema_table_from_frm(TH
uint res= 0;
int not_used;
my_hash_value_type hash_value;
- char key[MAX_DBKEY_LENGTH];
+ const char *key;
uint key_length;
char db_name_buff[NAME_LEN + 1], table_name_buff[NAME_LEN + 1];
@@ -3345,7 +3345,7 @@ static int fill_schema_table_from_frm(TH
goto end;
}
- key_length= create_table_def_key(thd, key, &table_list, 0);
+ key_length= get_table_def_key(&table_list, &key);
hash_value= my_calc_hash(&table_def_cache, (uchar*) key, key_length);
mysql_mutex_lock(&LOCK_open);
share= get_table_share(thd, &table_list, key,
=== modified file 'sql/sql_table.cc'
--- a/sql/sql_table.cc 2011-03-26 10:56:27 +0000
+++ b/sql/sql_table.cc 2011-03-31 06:39:38 +0000
@@ -6592,9 +6592,9 @@ bool mysql_alter_table(THD *thd,char *ne
if (table->s->tmp_table)
{
TABLE_LIST tbl;
- bzero((void*) &tbl, sizeof(tbl));
- tbl.db= new_db;
- tbl.table_name= tbl.alias= tmp_name;
+ tbl.init_one_table(new_db, strlen(new_db),
+ tmp_name, strlen(tmp_name),
+ tmp_name, TL_READ_NO_INSERT);
/* Table is in thd->temporary_tables */
(void) open_temporary_table(thd, &tbl);
new_table= tbl.table;
@@ -6890,15 +6890,15 @@ bool mysql_alter_table(THD *thd,char *ne
NO need to tamper with MERGE tables. The real open is done later.
*/
Open_table_context ot_ctx(thd, MYSQL_OPEN_REOPEN);
- TABLE *t_table;
+ TABLE_LIST temp_table_list;
+ TABLE_LIST *t_table_list;
if (new_name != table_name || new_db != db)
{
- table_list->alias= new_name;
- table_list->table_name= new_name;
- table_list->table_name_length= strlen(new_name);
- table_list->db= new_db;
- table_list->db_length= strlen(new_db);
- table_list->mdl_request.ticket= target_mdl_request.ticket;
+ temp_table_list.init_one_table(new_db, strlen(new_db),
+ new_name, strlen(new_name),
+ new_name, TL_READ_NO_INSERT);
+ temp_table_list.mdl_request.ticket= target_mdl_request.ticket;
+ t_table_list= &temp_table_list;
}
else
{
@@ -6908,20 +6908,21 @@ bool mysql_alter_table(THD *thd,char *ne
to request the lock.
*/
table_list->mdl_request.ticket= mdl_ticket;
+ t_table_list= table_list;
}
- if (open_table(thd, table_list, thd->mem_root, &ot_ctx))
+ if (open_table(thd, t_table_list, thd->mem_root, &ot_ctx))
{
goto err_with_mdl;
}
- t_table= table_list->table;
/* Tell the handler that a new frm file is in place. */
- error= t_table->file->ha_create_handler_files(path, NULL, CHF_INDEX_FLAG,
- create_info);
+ error= t_table_list->table->file->ha_create_handler_files(path, NULL,
+ CHF_INDEX_FLAG,
+ create_info);
- DBUG_ASSERT(thd->open_tables == t_table);
+ DBUG_ASSERT(thd->open_tables == t_table_list->table);
close_thread_table(thd, &thd->open_tables);
- table_list->table= 0;
+ t_table_list->table= 0;
if (error)
goto err_with_mdl;
=== modified file 'sql/sql_view.cc'
--- a/sql/sql_view.cc 2011-03-09 12:24:36 +0000
+++ b/sql/sql_view.cc 2011-03-31 06:39:38 +0000
@@ -209,13 +209,14 @@ static void make_valid_column_names(List
static bool
fill_defined_view_parts (THD *thd, TABLE_LIST *view)
{
- char key[MAX_DBKEY_LENGTH];
+ const char *key;
uint key_length;
LEX *lex= thd->lex;
TABLE_LIST decoy;
memcpy (&decoy, view, sizeof (TABLE_LIST));
- key_length= create_table_def_key(thd, key, view, 0);
+
+ key_length= get_table_def_key(view, &key);
if (tdc_open_view(thd, &decoy, decoy.alias, key, key_length,
thd->mem_root, OPEN_VIEW_NO_PARSE))
=== modified file 'sql/table.cc'
--- a/sql/table.cc 2011-03-01 14:47:01 +0000
+++ b/sql/table.cc 2011-03-31 06:39:38 +0000
@@ -311,7 +311,7 @@ TABLE_CATEGORY get_table_category(const
# Share
*/
-TABLE_SHARE *alloc_table_share(TABLE_LIST *table_list, char *key,
+TABLE_SHARE *alloc_table_share(TABLE_LIST *table_list, const char *key,
uint key_length)
{
MEM_ROOT mem_root;
=== modified file 'sql/table.h'
--- a/sql/table.h 2011-02-02 22:02:29 +0000
+++ b/sql/table.h 2011-03-31 06:39:38 +0000
@@ -2150,7 +2150,7 @@ void init_mdl_requests(TABLE_LIST *table
int open_table_from_share(THD *thd, TABLE_SHARE *share, const char *alias,
uint db_stat, uint prgflag, uint ha_open_flags,
TABLE *outparam, bool is_create_table);
-TABLE_SHARE *alloc_table_share(TABLE_LIST *table_list, char *key,
+TABLE_SHARE *alloc_table_share(TABLE_LIST *table_list, const char *key,
uint key_length);
void init_tmp_table_share(THD *thd, TABLE_SHARE *share, const char *key,
uint key_length,
Attachment: [text/bzr-bundle] bzr/dmitry.lenev@oracle.com-20110331063938-bn2ypz2aumfq5aac.bundle
